rspec-mocks icon indicating copy to clipboard operation
rspec-mocks copied to clipboard

`any_instance_of` does not restore method on JRuby

Open marcotc opened this issue 4 years ago • 7 comments

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.

marcotc avatar Aug 20 '20 21:08 marcotc

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?

pirj avatar Aug 20 '20 21:08 pirj

@marcotc ping

pirj avatar Aug 24 '20 17:08 pirj

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:

  1. A test calls allow_any_instance_of(clazz).to receive(:foo).
  2. Subsequent test calls expect_any_instance_of(clazz).to_not receive(:foo).
  3. 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.

marcotc avatar Aug 24 '20 18:08 marcotc

- 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:

  1. test on earlier rubies available in RVM, check if the above change works on all
  2. open a ticket for JRuby
  3. add a spec and a note to the above change mentioning JRuby ticket
  4. send a pull request with the change

pirj avatar Aug 25 '20 13:08 pirj

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?

JonRowe avatar Aug 26 '20 04:08 JonRowe

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.

pirj avatar Aug 26 '20 11:08 pirj

Care to add an example to our build that fails as a draft as a first step?

JonRowe avatar Aug 26 '20 12:08 JonRowe

Closing due to inactivity / lack of a reproduction during the monorepo migration.

JonRowe avatar Nov 27 '24 21:11 JonRowe