freerange / mocha

A mocking and stubbing library for Ruby
https://mocha.jamesmead.org
Other
1.21k stars 157 forks source link

can't dump anonymous class #<Mocha::StubbedMethod::PrependedModule:0x...> #529

Closed khiav223577 closed 2 years ago

khiav223577 commented 2 years ago

Reproduction Steps

  1. Add a test case
    
    require 'rails_helper'

describe User do it do user = User.new user.stubs(:id)

Rails.cache.fetch('test'){ user }

end end


2. Run it: `rspec spec/models/user_spec.rb`
3. Get failure:

Failures:

1) User Failure/Error: Rails.cache.fetch('test'){ user }

 TypeError:
   can't dump anonymous class #<Mocha::StubbedMethod::PrependedModule:0x00007fc2620f7400>
floehopper commented 2 years ago

@khiav223577

Hi. Sorry for the slow reply.

I think this is effectively a duplicate of #285 which in turn is related to #81. I did some work on the latter a while back, but in the end I decided Mocha should fail fast if anyone attempts to serialize to YAML (i.e. #318), although that hasn't been implemented. I've now added an equivalent issue for failing fast when serializing using Marshal.dump which is I think what you're asking about. However, obviously even if that was addressed, it wouldn't help in your current situation...

Instead I would recommend that you avoid dumping the mock (or partial mock) object. Is your example test actually reflective of what you're trying to do? It doesn't seem to have any assertions. If you can tell me what specific behaviour you are trying to test, I can suggest an alternative approach that avoids the exception.

khiav223577 commented 2 years ago

We have a ActiveRecord-like model which uses active_hash to load data from files. Let it be Country. In one test file (Test File A), we mock one of the country's attribute and test other method that are not defined in Country. In other test file (Test File B), we want to test supported_countries method. Since we have large data source, we want to cache the result in the cache.

If the test order is Test File B -> Test File A, it works fine. But if the test order is Test File A -> Test File B, it will raise can't dump anonymous class.

It seems that if we mock an object, it will permanently change it. And the object loads from active_hash are like global objects, it will not be reloaded between test cases. Therefore any changes to it will affect the following test cases.

class Country < ActiveHash::Base
  self.data = [
    { id: 1, key: 'US', supported: true },
    { id: 2, key: 'Canada', supported: false },
  ]

  class << self
    def supported_countries
      Rails.cache.fetch('supported_countries'){ all.select(&:supported) }
    end
  end
end

require 'rails_helper'

# Test File A
describe do
  it do
    Country.find_by(key: 'US').stubs(:supported).returns(false)
    # test something
  end
end

# Test File B
describe Country do
  it do
    expect(supported_countries.map(&:key)).to eq %w[US]
  end
end
floehopper commented 2 years ago

@khiav223577

I haven't come across active_hash before - looks interesting!

I would generally advise against stubbing methods on objects that are shared between tests. In fact a more general point is that sharing any state between tests is worth avoiding as much as possible!

Mocha was designed on the assumption that all stubbed methods are set up and torn down within a single test. If you really do need to stub methods on an object shared between tests, then I would try to explicitly set up the stubs within each test, even if the object is created outside the test. That way, the stubbed method should be torn down at the end of each test. However, given that it sounds as if active_hash is persisting the state of the object, even doing that might not solve your specific problem.

Have you considered stubbing Country.data to return a smaller set of objects? That might mean you could avoid needing to stub methods on the instances of Country. This seems like the best option I can see, but I might have missed something.

I'm unclear whether it's active_hash or Rails.cache which is triggering the object to be dumped. If it's the latter, perhaps you could use a different cache store type in your tests which doesn't attempt to dump objects, but just stores them in memory...?

The only other thought I've had is that you could try to explicitly un-stub a method using Mocha::Mock#unstub (for mock objects) or Mocha::ObjectMethods#unstub (for partial mocks). It's a bit of a hack and I'm not convinced it will actually work, but it might be worth a try...?

I hope some of that helps. As I said in my previous comment, it doesn't makes sense to me to support the dumping of mocks or partial mocks, and so the only change I'm likely to make is to fail fast if such an attempt is made. On that basis I'm going to close this issue for now, but do get back in touch if you have more questions.