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

Rails 7.1 breaks receive().with #1530

Closed
palkan opened this issue Mar 6, 2023 · 6 comments · Fixed by healthify/fake_idp#70
Closed

Rails 7.1 breaks receive().with #1530

palkan opened this issue Mar 6, 2023 · 6 comments · Fixed by healthify/fake_idp#70

Comments

@palkan
Copy link

palkan commented Mar 6, 2023

Your environment

  • Ruby version: 3.2
  • rspec-mocks version: 3.12.2
  • Rails version: main.

Steps to reproduce

In a Rails application (i.e., with Rails loaded), define a method expectation:

mock = instance_double("Hash")
allow(mock).to receive(:key?).with(:x) { 1 }

expect(mock.key?(:x)).to eq 1

Expected behavior

Pass.

Actual behavior

ArgumentError:
       wrong number of arguments (given 1, expected 0)
     # ../rails/activesupport/lib/active_support/core_ext/object/with.rb:24:in `with'

What happened?

Rails monkey-patched Object#with rails/rails#46681 /cc @byroot

RSpec receive matcher defines chain methods only for undefined methods:

MessageExpectation.public_instance_methods(false).each do |method|
next if method_defined?(method)

Since #with is defined on Object, the with modifier is no longer available.

@nobu
Copy link
Contributor

nobu commented Mar 7, 2023

Why does MessageExpectation need to b e a subclass of Object, not BasicObject?

@byroot
Copy link
Contributor

byroot commented Mar 7, 2023

Even then, why would it not redefine existing methods? I'll git blame to see if I can find the reason for this.

@byroot
Copy link
Contributor

byroot commented Mar 7, 2023

Ah, I think the intent is to not redefine methods directly defined in Receive, since we're copying all the matchers. method_defined?(name, false) would fix it I think.

casperisfine pushed a commit to casperisfine/rspec-mocks that referenced this issue Mar 7, 2023
Fix: rspec#1530

My understanding of this `next if method_defined?` check that there since
this class was added in 08ec2e8
is that it's to prevent accidentally overriding Matcher or own methods.

If my understanding is correct, then we should ignore methods inherited from
`Object`.
@byroot
Copy link
Contributor

byroot commented Mar 7, 2023

I think I have a fix for it, but GitHub seems to be having issues, I get an error message when trying to create the PR....

Pull request creation failed. Validation failed: You can't perform that action at this time.

https:/rspec/rspec-mocks/compare/rspec:main...casperisfine:receiver-with?expand=1

@pirj
Copy link
Member

pirj commented Mar 7, 2023

@byroot I’ve opened one, does it work for you?

@byroot
Copy link
Contributor

byroot commented Mar 7, 2023

Sure, thank you.

mshibuya added a commit to carrierwaveuploader/carrierwave that referenced this issue Mar 12, 2023
jdelStrother added a commit to jdelStrother/thinking-sphinx that referenced this issue Oct 11, 2023
rspec to fix Object#with - rspec/rspec-mocks#1530

database_cleaner to fix "undefined method `table_name' for ActiveRecord::SchemaMigration:Class"
Earlopain added a commit to Earlopain/mailgun-ruby that referenced this issue Jan 2, 2024
Fixes test failures with Rails 7.1
rspec/rspec-mocks#1530
chrislo added a commit to RaspberryPiFoundation/editor-api that referenced this issue Jun 11, 2024
We ran

   bundle update --conservative rspec-mocks

We need to update rspec-mocks here since Rails 7.1 introduces a method
`with` that clashes with the `with` method provided by
rspec-mocks[1]. We're using the latter in the corp_middleware_spec.

[1] rspec/rspec-mocks#1530

Co-authored-by: Chris Roos <[email protected]>
chrislo added a commit to RaspberryPiFoundation/editor-api that referenced this issue Jun 11, 2024
We ran

   bundle update --conservative rspec-mocks

We need to update rspec-mocks here since Rails 7.1 introduces a method
`with` that clashes with the `with` method provided by
rspec-mocks[1]. We're using the latter in the corp_middleware_spec.

[1] rspec/rspec-mocks#1530

Co-authored-by: Chris Roos <[email protected]>
chrislo added a commit to RaspberryPiFoundation/editor-api that referenced this issue Jun 12, 2024
We ran

   bundle update --conservative rspec-mocks

We need to update rspec-mocks here since Rails 7.1 introduces a method
`with` that clashes with the `with` method provided by
rspec-mocks[1]. We're using the latter in the corp_middleware_spec.

[1] rspec/rspec-mocks#1530

Co-authored-by: Chris Roos <[email protected]>
jdelStrother added a commit to jdelStrother/thinking-sphinx that referenced this issue Jun 15, 2024
rspec to fix Object#with - rspec/rspec-mocks#1530

database_cleaner to fix "undefined method `table_name' for ActiveRecord::SchemaMigration:Class"
jdelStrother added a commit to jdelStrother/thinking-sphinx that referenced this issue Jun 16, 2024
rspec to fix Object#with - rspec/rspec-mocks#1530

database_cleaner to fix "undefined method `table_name' for ActiveRecord::SchemaMigration:Class"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants