Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with RSpec::Matchers.define_negated_matcher and active_support/core_ext/object/with.rb #49958

Closed
vimalloc opened this issue Nov 7, 2023 · 8 comments

Comments

@vimalloc
Copy link

vimalloc commented Nov 7, 2023

Hello!

We are running into an issue that was introduced when upgrading from rails 7.0.8 to 7.1.1, which is triggered by the newly added Object#with method.

Steps to reproduce

  • Define a negated matcher that can acceptswith (ex: enqueue_job.with('foo'))
  • Attempt to run a test using both the negated matcher and with
  • Notice we receive a wrong number of arguments error, and that the stacktrace indicates that it's trying to call Object#with, as defined in activesupport-7.1.1/lib/active_support/core_ext/object/with.rb
# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem "sqlite3"
  gem 'rails', '~> 7.1'
  gem 'rspec-rails'
end

require "rails/all"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)
ActiveJob::Base.queue_adapter = :test

require 'rspec/rails'
require 'rspec/autorun'

# This is the thing that is eventually leading to an ArgumentError being raised when it shouldn't
RSpec::Matchers.define_negated_matcher(:not_enqueue_job, :enqueue_job)

RSpec.describe 'downcase' do
  it 'works' do
    expect('Foo'.downcase).to eq('foo')
  end

  it 'does not enqueue a job' do
    expect { 'Foo'.downcase }.to not_enqueue_job.with('Bar')
  end
end

Expected behavior

The test passes.

Actual behavior

  1) downcase does not enqueue a job
     Failure/Error: expect { 'Foo'.downcase }.to not_enqueue_job.with('Bar')

     ArgumentError:
       wrong number of arguments (given 1, expected 0)
     # ./example_spec.rb:30:in `block (2 levels) in <top (required)>'

Notes:

This is happening with a few different matchers we use, not just the not_enqueue_job one. We are using these negated matchers because we have a lot of specs that assert a couple different things using .and <negated_matcher>.

Rspec-mocks ran into a similar issue as this: rspec/rspec-mocks#1530.

System configuration

Rails version:
7.1.1

Ruby version:
3.2.2

@byroot
Copy link
Member

byroot commented Nov 7, 2023

I'm trying to run your repro script, but it doesn't execute the tests, how do you do that?

@byroot
Copy link
Member

byroot commented Nov 7, 2023

Nevermind, it's just missing require "rspec/autorun"

@fatkodima
Copy link
Member

I think, Object#with comes with this require

require "active_support/core_ext/object"

Should we do only require "active_support/core_ext/object/deep_dup" here?

@vimalloc
Copy link
Author

vimalloc commented Nov 7, 2023

Nevermind, it's just missing require "rspec/autorun"

Doh, thank you. Sorry, I was running it using nvim-test, which just did the right thing for me 😅

@byroot
Copy link
Member

byroot commented Nov 7, 2023

@fatkodima that's not the issue. rspec-matchers should work even in the presence of Object#with.

@byroot
Copy link
Member

byroot commented Nov 7, 2023

Ok. I found the issue. not_enqueue_job returns a Rspec::Matchers::AliasedNegatedMatcher. That class relies on method_missing, so now that Object#with was introduced it no longer intercept .with.

The clean fix would be for it to inherit from BasicObject, I'll try to work on a patch for that.

As a quick workaround you can add:

Rspec::Matchers::AliasedNegatedMatcher.undef_method(:with)

In you spec_helper or something.

@vimalloc
Copy link
Author

vimalloc commented Nov 7, 2023

Awesome, that unblocked me! Thank you for looking at this! <3

@byroot
Copy link
Member

byroot commented Nov 7, 2023

Not quite production ready, but the clean fix will look something like that: rspec/rspec-expectations#1434

I'll try to polish it tomorrow.

Now if you don't mind, given this isn't a Rails bug, I'll close.

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

No branches or pull requests

3 participants