rspec-mocks
rspec-mocks copied to clipboard
`any_instance_of` does not restore method on JRuby
Subject of the issue
A special combination of any_instance_of
calls can prevent rspec-mocks
from properly restoring the originally mocked method method during it cleanup step. I'm only able to observe this on JRuby.
Your environment
- Ruby version: jruby 9.2.11.1
- rspec-mocks version: 3.9.1
Steps to reproduce
# frozen_string_literal: true
begin
require "bundler/inline"
rescue LoadError => e
$stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
raise e
end
gemfile(true) do
source 'https://rubygems.org'
gem 'rspec', '3.9.0'
gem 'rspec-mocks', '3.9.1'
end
require 'rspec/autorun'
puts "Ruby version is: #{RUBY_VERSION}"
STDERR.puts 'You need JRuby to reproduce this issue!' unless RSpec::Support::Ruby.jruby?
RSpec.describe do
it do
clazz = Class.new do
prepend Module.new
def foo
end
end
allow_any_instance_of(clazz).to receive(:foo)
RSpec::Mocks.space.reset_all
expect_any_instance_of(clazz).to_not receive(:foo)
RSpec::Mocks.space.reset_all
clazz.new.foo
end
end
Expected behavior
No failures:
1 example, 0 failures
Actual behavior
Failures:
1)
Failure/Error: clazz.new.foo
NoMethodError:
undefined method `foo' for #<#<Class:0x1d008e61>:0x29db008c>
Did you mean? fork
# rspec-mock-issue.rb:39:in `block in rspec-mock-issue.rb'
Finished in 0.02769 seconds (files took 0.15631 seconds to load)
1 example, 1 failure
Preliminary investigation
My semi-educated guess is that the issue lies in this code block: https://github.com/rspec/rspec-mocks/blob/v3.9.1/lib/rspec/mocks/any_instance/recorder.rb#L202-L222
Forcing alias_method method_name, alias_method_name
to when running on JRuby in that code block "solves" the problem from my example. I unfortunately wasn't able process how the comment included in that snippet linked is applying to our case, so I'm not 100% confident that this is the correct solution.
Nice find, thanks for reporting.
Would it make sense to add JRuby check to RUBY_VERSION < "2.3"
?
Is there a more real-life spec that we could add to prevent regression?
Does it make sense to open a JRuby ticket to eventually address this?
@marcotc ping
For context, I spent around 2 tries trying to fix this issue, but I wasn't able to figure out how the internals of rspec-mocks
should work around the involved the area. My goal was to open a PR to fix the issue, and the only reason I'm opening an issue instead because I wasn't able to come up with a solution.
Would it make sense to add JRuby check to RUBY_VERSION < "2.3"?
It would this specific issue, but I don't have enough confidence to say if that's the right solution.
Is there a more real-life spec that we could add to prevent regression?
I tried my best to reduce the test to its minimal form, and I wasn't able to simplify it any further than the reported version above.
In our use-case it boiled down to:
- A test calls
allow_any_instance_of(clazz).to receive(:foo)
. - Subsequent test calls
expect_any_instance_of(clazz).to_not receive(:foo)
. - After that,
clazz
does not respond to:foo
anymore.
Trying to simplify/remove any of these steps did not allow me to reproduce the issue anymore.
Does it make sense to open a JRuby ticket to eventually address this?
I don't have the confidence to know if the issue lies in this repo or JRuby. I noticed there's an issue linked from rspec-mocks
about an MRI but that I thought was affecting JRuby, but JRuby actually behaves as expected in that scenario, so it's not that same issue.
- if RUBY_VERSION < "2.3" || backed_up_method_owner[method_name.to_sym] == self
+ if RUBY_VERSION < "2.3" || RSpec::Support::Ruby.jruby? || backed_up_method_owner[method_name.to_sym] == self
made your spec green on jruby-head
(jruby 9.3.0.0-SNAPSHOT (2.6.5) 2020-08-25 2f0c49000a OpenJDK 64-Bit Server VM 14.0.1+14 on 14.0.1+14 +jit [darwin-x86_64])
I propose the following:
- test on earlier rubies available in RVM, check if the above change works on all
- open a ticket for JRuby
- add a spec and a note to the above change mentioning JRuby ticket
- send a pull request with the change
Ah JRuby, I haven't missed your quirks, is this something thats happening only on the current head? Why haven't our builds caught it @pirj?
The example https://github.com/rspec/rspec-mocks/blob/018d0d54a0ecaed259fd4eac5f151c51abb57b1d/spec/rspec/mocks/any_instance_spec.rb#L204 is a bit different. Here, there's no even a parent class involved. But there is:
- prepend
- double stubbing
- double reset
without any one of those the issue doesn't reproduce.
Care to add an example to our build that fails as a draft as a first step?
Closing due to inactivity / lack of a reproduction during the monorepo migration.