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
stub_const
doesn't clear Class#subclasses
#1568
Comments
It seems that doing a garbage collection after
|
I tried on a Rails project and doing a garbage collection is not enough. Defined classes without |
What do you mean "correctly removed"? describe 'Test', order: :defined do
context 'with A' do
before(:each) do
class A < Something; end
end
it 'something' do
pp Something.subclasses # => [A]
end
end
context 'presumably without A' do
it 'something else' do
pp Something.subclasses # => [A] 💥
end
end
end
|
For |
I mean,
Note that I see than everytime we use
I understand that as As we see in this example,
I don't know how to be sure the class than is removed. |
Defining the class in a global variable fix the issue :
But I suppose it will create some other errors. |
@pirj can you confirm this is a bug or, at least, something that is supposed to be handled in rspec-mocks ? |
Something feels off, but the essence of the problem evades me so far. The What we know is that unreferenced classes are garbage-collected. But why subclasses show that there are two bound to consts with identical names? I’m still not convienced that our observation about |
It doesn't surprise me that much. Every
The error is not actually raised when I do
I did those tests (none fix the issue) # This does not raise an error
after(:each) do
RSpec::Mocks.space.reset_all
end
# This does not raise an error
after(:each) do
RSpec::Mocks.space.instance_variable_get(:@constant_mutators).map do |mutator|
mutator.idempotently_reset
end
end
# This raise constant Object::B not defined
after(:each) do
Object.send(:remove_const, :B)
end
# This raise constant Object::B not defined
after(:each) do
RSpec::Mocks.space.instance_variable_get(:@constant_mutators).map do |mutator|
mutator.instance_variable_get(:@parent).send(:remove_const, mutator.instance_variable_get(:@const_name))
end
end
# This raise constant Object::B not defined
after(:each) do
RSpec::Mocks.space.instance_variable_get(:@constant_mutators).map do |mutator|
mutator.reset
end
end |
Sorry, I never meant you should call remove_const from the spec code. And if I did, I was meaning something else. So we’re now at a point where we see that tge GC is involved, and yet-to-be-collected classes still appear through Do you think RSpec is the one responsible? What is the original problem you are dealing with? Do you use |
I do think this is a bug but globals is not the answer, if the issue is Ruby is retaining the class after we have removed it, I'm not sure of the solution to that... |
Relying on garbage collection can be tricky as the class can be kept in memory elsewhere, and I'm pretty sure it is with Rails.
No, I don't.
Yes, I do. We have something like :
I'm not sure if this is the best implementation but that's not the point. Rails overrides
It's pseudo code, it doesn't work. I don't like to override |
That means that we’ll keep references to stubbed classes in between examples, effectively preventing from GC’ing them. Also, the classes stubbed in an example should be returned by So, we’ll need an extra step and WeakRef. Even though this would fix your case, I have no certainty we should include this for everyone and what the performance penalty is. |
Yes, that's it. The method name should probably be |
I succeeded to fix my issue with :
I don't like to do override a Ruby method but see no other solution. I will create a PR with this code. |
Subject of the issue
When
stub_const
is used with a class that is a subclass of another, the class is still listed in subclasses of this parent after each spec.Your environment
Steps to reproduce
Expected behavior
Something.subclasses
always have to return[B, A]
and be the same for each spec.Actual behavior
Something.subclasses
still return every stubbed classes.The text was updated successfully, but these errors were encountered: