fix: remove hard requirements on active_support and webmock#403
Conversation
active_support was never declared as a runtime dependency but pact used blank?/present? in 70+ places, forcing every consumer to pre-require it. Replace with a minimal built-in polyfill (lib/pact/support/blank.rb) that is skipped automatically if active_support (or anything else) has already defined these methods. webmock was missing a `return` before the early yield in WebmockHelpers.turned_off, causing it to fall through to WebMock::Config even when WebMock was not loaded. Adding the return fixes the crash, making webmock a truly optional integration dependency. Document both as optional in the gemspec. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
ci checks didn't kick off for some reason |
active_support's deep_dup is used in 8 places in the gem (matchers, consumer interaction builder). Add a minimal Object/Array/Hash implementation to lib/pact/support/blank.rb so active_support is not required for this either. Prompted by review feedback from @YOU54F. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
a point of note, the code was originally designed by authors primarily with rails in mind, and for non-rails users like ourselves with the pact_broker and ruby client libraries, they currently require combustion to be loaded on both the consumer and provider side. Some of the changes in this PR are moving towards us not requiring on the consumer side. With the changes recommended below, we eliminate it entirely from the consumer side, but the provider side needs more investigation. For consumer side, https://github.com/pact-foundation/pact-ruby/blob/master/example/zoo-app/Gemfile#L10 presence is missing
resolved by requiring We can remove the requirement for https://github.com/pact-foundation/pact-ruby/blob/master/example/zoo-app/spec/rails_helper.rb For the provider side https://github.com/pact-foundation/pact-ruby/blob/master/example/animal-service/Gemfile changes for webmock working fine now, so webmock is no longer mandatory to be required in the users Gemfile when doing pact verifications. Without combustion loaded in the provider side project
we get the following error Loading https://github.com/pact-foundation/pact-ruby/blob/master/example/animal-service/spec/rails_helper.rb |
The file now covers both blank?/present? and deep_dup polyfills, so core_ext.rb better reflects its scope (mirrors active_support/core_ext convention). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Object#blank? was `!self`, ignoring objects that respond to empty?. Should be `respond_to?(:empty?) ? !!empty? : !self` to match active_support. Object#present? was `!!self`, which is always true for any non-nil/non-false object regardless of blank? — inconsistent with classes that override blank?. Should be `!blank?`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
YOU54F
left a comment
There was a problem hiding this comment.
Looks good to me now, thanks!
This PR fixes 1 & 3
Please add a note here to state if testing a non-rails application, to require the combustion gem, add the following helper
require "combustion"
begin
Combustion.initialize! :action_controller
rescue => e
warn "💥 Failed to load the pact-ruby: #{e.message}\n#{e.backtrace.join("\n")}"
exit(1)
endWe should look at sorting out issue 2, as well as addressing #399
Thanks!
Summary
active_supportwas never a declared runtime dependency but pact usedblank?/present?in 70+ places, forcing every consumer to pre-require it as a workaround. This PR adds a minimal built-in polyfill (lib/pact/support/blank.rb) loaded early inpact.rb— it is skipped automatically ifactive_support(or anything else) has already defined these methods, so existing users are unaffected.webmockhad a missingreturninWebmockHelpers.turned_off, causing it to fall through toWebMock::Configeven when WebMock was not loaded, crashing with aNameError. Adding thereturnmakes webmock a truly optional integration dependency again.pact.gemspecnow documents both as optional with a comment explaining the intent.Motivation
Consumers of this gem were forced to add defensive
requirestatements with inline comments explaining the workarounds. These workarounds have no "remove when X is fixed" marker, so they accumulate silently in downstream codebases. Fixing at the source removes the need for any workaround in consuming code.Test plan
bundle exec rspec)WebmockHelpers.turned_off { }no longer raises whenwebmockis not loadedblank?/present?behave correctly withoutactive_supportloadedactive_supportis loaded first, its definitions take precedence (polyfill is skipped)🤖 Generated with Claude Code