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

Fix === for multiple mocks of the same model #61

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

bquorning
Copy link
Contributor

@bquorning bquorning commented Sep 27, 2024

When the same model is mocked more than once in the same context, comparing with the model class using #=== would only return true for the first of the mocks. All following mocks would return false when compared to the original model class using #===.

For #=== to work correctly, we register each mock in a "mock_store" on the RSpec::ActiveModel::Mocks::Mocks module. The store is implemented as an RSpec stub, so it is reset after each spec example.

Fixes #60

I realize that I’m adding some complex code into some already-complex code, so please let me know if you’d rather have it implemented in some other way.

@bquorning bquorning mentioned this pull request Sep 27, 2024
@bquorning bquorning force-pushed the fix-===-for-multiple-stubs branch 2 times, most recently from 500deb0 to 641e55a Compare September 27, 2024 21:49
@bquorning bquorning marked this pull request as ready for review September 27, 2024 22:14
@JonRowe
Copy link
Member

JonRowe commented Oct 1, 2024

Can you rebase this please

Copy link
Member

@JonRowe JonRowe left a comment

Choose a reason for hiding this comment

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

I see why you used a stub to store this data (so its automatically cleared), but rather than a global, I suggest stubing it on the model itself, its slightly neater

lib/rspec/active_model/mocks/mocks.rb Outdated Show resolved Hide resolved
lib/rspec/active_model/mocks/mocks.rb Outdated Show resolved Hide resolved
lib/rspec/active_model/mocks/mocks.rb Outdated Show resolved Hide resolved
lib/rspec/active_model/mocks/mocks.rb Outdated Show resolved Hide resolved
@bquorning bquorning force-pushed the fix-===-for-multiple-stubs branch from 641e55a to 97ce3df Compare October 1, 2024 12:09
@bquorning
Copy link
Contributor Author

I went back and forth on stubbing globally or on the model itself, but chose global because 1) we don’t accidentally override a method on the model itself, and 2) one spec fails on Rails 7.1 when I stub on the model itself.

However, I think you are correct, it is better to stub on the model. I will look into why the spec is failing.

@bquorning bquorning force-pushed the fix-===-for-multiple-stubs branch from 97ce3df to 5d640fe Compare October 1, 2024 13:01
@bquorning
Copy link
Contributor Author

However, I think you are correct, it is better to stub on the model. I will look into why the spec is failing.

Sorry, I changed it back to the global stub store again. It works, and it seems to get properly cleared after each example is run.

@bquorning bquorning force-pushed the fix-===-for-multiple-stubs branch from 5d640fe to fdcc72d Compare October 1, 2024 14:54
When the same model is mocked more than once in the same context,
comparing with the model class using `#===` would only return true for
the first of the mocks. All following mocks would return false when
compared to the original model class using `#===`.

For `#===` to work correctly, we register each mock in a "mock_store"
on the `RSpec::ActiveModel::Mocks::Mocks` module. The store is
implemented as an RSpec stub, so it is reset after each spec example.

Fixes rspec#60
@bquorning bquorning force-pushed the fix-===-for-multiple-stubs branch from fdcc72d to 265ac5e Compare October 1, 2024 15:12
@JonRowe JonRowe merged commit 4142ad1 into rspec:main Oct 2, 2024
51 checks passed
@bquorning bquorning deleted the fix-===-for-multiple-stubs branch October 2, 2024 20:05
@JonRowe
Copy link
Member

JonRowe commented Oct 2, 2024

Released in 1.2.1, thanks!

@bquorning
Copy link
Contributor Author

Thank you Jon 👍🏼

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.

Rails 7.1: mock_model is not always === the class it mocks
2 participants