search

rspec / rspec-mocks

RSpec's 'test double' framework, with support for stubbing and mocking
https://rspec.info
MIT License
1.16k stars 357 forks source link

Receive: only avoid overriding own methods #1531

Closed pirj closed 1 year ago

pirj commented 1 year ago

Fix: https://github.com/rspec/rspec-mocks/issues/1530

My understanding of this next if method_defined? check that there since this class was added in 08ec2e80325a5959ac13b113a0175d28aca352a8 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 commented 1 year ago

The JRuby failure seem unrelated.

byroot commented 1 year ago

Do you think it would be possible to write a test this case when some of the method that Receive pulls from MessageExpectation are still defined if someone monkey patches Object with the same ones?

Hum, not sure how we could do this, that code is only executed once, so it's very hard to unit test it, and even if we refactored it out we'd have to patch object temporarily to simulate the issue.

I get the want for a test case, but it seems very impractical here.

byroot commented 1 year ago

Any blocker on this? I don't use RSpec myself, but I assume it my be quite blocking for RSpec users who track Rails edge.

JonRowe commented 1 year ago

It needs a test, I've added one in #1534 but some of the builds aren't passing yet

byroot commented 1 year ago

Thanks, I didn't notice that other PR.

JonRowe commented 1 year ago

It was intended to be quick addition then merge but the build gods had other ideas, I'm hoping to get it merged this weekend sometime and then released. Thanks for the work on this.

JonRowe commented 1 year ago

Merged in 7c10871

JonRowe commented 1 year ago

Released in 3.12.4 thank you!