Skip to content

fix: remove hard requirements on active_support and webmock#403

Open
Saup21 wants to merge 7 commits into
masterfrom
fix/remove-active-support-webmock-hard-requirements
Open

fix: remove hard requirements on active_support and webmock#403
Saup21 wants to merge 7 commits into
masterfrom
fix/remove-active-support-webmock-hard-requirements

Conversation

@Saup21
Copy link
Copy Markdown
Contributor

@Saup21 Saup21 commented May 26, 2026

Summary

  • active_support was never a declared runtime dependency but pact used blank?/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 in pact.rb — it is skipped automatically if active_support (or anything else) has already defined these methods, so existing users are unaffected.

  • webmock had a missing return in WebmockHelpers.turned_off, causing it to fall through to WebMock::Config even when WebMock was not loaded, crashing with a NameError. Adding the return makes webmock a truly optional integration dependency again.

  • pact.gemspec now documents both as optional with a comment explaining the intent.

Motivation

Consumers of this gem were forced to add defensive require statements 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

  • Existing RSpec suite passes (bundle exec rspec)
  • WebmockHelpers.turned_off { } no longer raises when webmock is not loaded
  • blank?/present? behave correctly without active_support loaded
  • When active_support is loaded first, its definitions take precedence (polyfill is skipped)

🤖 Generated with Claude Code

Saup21 and others added 2 commits May 26, 2026 16:39
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>
@Saup21 Saup21 requested a review from YOU54F May 26, 2026 11:51
@YOU54F YOU54F closed this May 27, 2026
@YOU54F YOU54F reopened this May 27, 2026
@YOU54F
Copy link
Copy Markdown
Member

YOU54F commented May 27, 2026

ci checks didn't kick off for some reason

Comment thread lib/pact.rb Outdated
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>
@Saup21 Saup21 requested a review from YOU54F May 27, 2026 17:51
@YOU54F
Copy link
Copy Markdown
Member

YOU54F commented May 28, 2026

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

X_PACT_DEVELOPMENT=true bundle exec rake pact:spec

     NoMethodError:
       undefined method 'presence' for nil
     # /Users/yousaf.nabi/dev/pact-foundation/pact-ruby/lib/pact/matchers/v3/each.rb:13:in 'Pact::Matchers::V3::Each#initialize'

resolved by requiring

require 'active_support/core_ext/object/blank'

We can remove the requirement for combustion gem for non-rails apps

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

X_PACT_DEVELOPMENT=true bundle exec rake pact:verify:foobar

we get the following error

NameError:
  uninitialized constant Pact::Provider::ProviderServerRunner::Logger
# /Users/yousaf.nabi/dev/pact-foundation/pact-ruby/lib/pact/provider/provider_server_runner.rb:18:in 'Pact::Provider::ProviderServerRunner#initialize'

Loading combustion gem, with the following helper resolves

https://github.com/pact-foundation/pact-ruby/blob/master/example/animal-service/spec/rails_helper.rb

Saup21 and others added 3 commits May 28, 2026 17:25
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>
Comment thread lib/pact/support/core_ext.rb
Copy link
Copy Markdown
Member

@YOU54F YOU54F left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
end

We should look at sorting out issue 2, as well as addressing #399

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants