Skip to content
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

Exclude stubbed classes from subclasses after teardown #1570

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions lib/rspec/mocks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,21 +113,22 @@ class << self

# To speed up boot time a bit, delay loading optional or rarely
# used features until their first use.
autoload :AnyInstance, "rspec/mocks/any_instance"
autoload :ExpectChain, "rspec/mocks/message_chain"
autoload :StubChain, "rspec/mocks/message_chain"
autoload :AnyInstance, "rspec/mocks/any_instance"
autoload :ExpectChain, "rspec/mocks/message_chain"
autoload :StubChain, "rspec/mocks/message_chain"
autoload :MarshalExtension, "rspec/mocks/marshal_extension"
pirj marked this conversation as resolved.
Show resolved Hide resolved
autoload :ExcludeStubbedClassesFromSubclasses, "rspec/mocks/exclude_stubbed_classes_from_subclasses"

# Namespace for mock-related matchers.
module Matchers
# @private
# just a "tag" for rspec-mock matchers detection
module Matcher; end

autoload :HaveReceived, "rspec/mocks/matchers/have_received"
autoload :Receive, "rspec/mocks/matchers/receive"
autoload :HaveReceived, "rspec/mocks/matchers/have_received"
autoload :Receive, "rspec/mocks/matchers/receive"
autoload :ReceiveMessageChain, "rspec/mocks/matchers/receive_message_chain"
autoload :ReceiveMessages, "rspec/mocks/matchers/receive_messages"
autoload :ReceiveMessages, "rspec/mocks/matchers/receive_messages"
end
end
end
17 changes: 17 additions & 0 deletions lib/rspec/mocks/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ def initialize
@verify_partial_doubles = false
@temporarily_suppress_partial_double_verification = false
@color = false
@exclude_stubbed_classes_from_subclasses = false
end

# Sets whether RSpec will warn, ignore, or fail a test when
Expand Down Expand Up @@ -178,6 +179,22 @@ def color?
end
end

def exclude_stubbed_classes_from_subclasses?
@exclude_stubbed_classes_from_subclasses
end

# When this is set to true, stubbed classes are excluded from the list of
# subclasses of the parent class after each spec.
def exclude_stubbed_classes_from_subclasses=(val)
@exclude_stubbed_classes_from_subclasses = !!val

if val
RSpec::Mocks::ExcludeStubbedClassesFromSubclasses.enable!
else
RSpec::Mocks::ExcludeStubbedClassesFromSubclasses.disable!
end
end

# Monkey-patch `Marshal.dump` to enable dumping of mocked or stubbed
# objects. By default this will not work since RSpec mocks works by
# adding singleton methods that cannot be serialized. This patch removes
Expand Down
56 changes: 56 additions & 0 deletions lib/rspec/mocks/exclude_stubbed_classes_from_subclasses.rb
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took RSpec::Mocks::MarshalExtension for example.

Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
module RSpec
module Mocks
# Support for `exclude_stubbed_classes_from_subclasses` configuration.
#
# @private
class ExcludeStubbedClassesFromSubclasses
class << self
def enable!
return unless RUBY_VERSION >= "3.1"
return if Class.respond_to?(:subclasses_with_rspec_mocks)

Class.class_eval do
def subclasses_with_rspec_mocks
subclasses_without_rspec_mocks - RSpec::Mocks::ExcludeStubbedClassesFromSubclasses.excluded_subclasses
end

alias subclasses_without_rspec_mocks subclasses
alias subclasses subclasses_with_rspec_mocks
end
end

def disable!
@excluded_subclasses = []

if Class.respond_to?(:subclasses_with_rspec_mocks)
Class.class_eval do
undef subclasses_with_rspec_mocks
alias subclasses subclasses_without_rspec_mocks
undef subclasses_without_rspec_mocks
end
end
end

def excluded_subclasses
require 'weakref' unless defined?(::WeakRef)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok to move this line into enable!?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it's a problem but now we can do RSpec::Mocks::ExcludeStubbedClassesFromSubclasses.excluded_subclasses without ExcludeStubbedClassesFromSubclasses.enable! first. If we move this line to enable!, it will raise an error.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you wish, I can do :

diff --git a/lib/rspec/mocks/exclude_stubbed_classes_from_subclasses.rb b/lib/rspec/mocks/exclude_stubbed_classes_from_subclasses.rb
index bb72101e..129ff0d2 100644
--- a/lib/rspec/mocks/exclude_stubbed_classes_from_subclasses.rb
+++ b/lib/rspec/mocks/exclude_stubbed_classes_from_subclasses.rb
@@ -8,6 +8,8 @@ module RSpec
         def enable!
           return unless RUBY_VERSION >= "3.1"
           return if Class.respond_to?(:subclasses_with_rspec_mocks)
+          @excluded_subclasses = []
+          require 'weakref' unless defined?(::WeakRef)
 
           Class.class_eval do
             def subclasses_with_rspec_mocks
@@ -32,9 +34,6 @@ module RSpec
         end
 
         def excluded_subclasses
-          require 'weakref' unless defined?(::WeakRef)
-
-          @excluded_subclasses ||= []
           @excluded_subclasses.select(&:weakref_alive?).map do |ref|
             begin
               ref.__getobj__
@@ -45,9 +44,6 @@ module RSpec
         end
 
         def exclude_subclass(constant)
-          require 'weakref' unless defined?(::WeakRef)
-
-          @excluded_subclasses ||= []
           @excluded_subclasses << ::WeakRef.new(constant)
         end
       end
diff --git a/spec/rspec/mocks/exclude_stubbed_classes_from_subclasses_spec.rb b/spec/rspec/mocks/exclude_stubbed_classes_from_subclasses_spec.rb
index 3b5ded94..16454922 100644
--- a/spec/rspec/mocks/exclude_stubbed_classes_from_subclasses_spec.rb
+++ b/spec/rspec/mocks/exclude_stubbed_classes_from_subclasses_spec.rb
@@ -63,6 +63,10 @@ if RUBY_VERSION >= '3.1'
         end
 
         describe '.excluded_subclasses' do
+          before do
+            described_class.enable!
+          end
+
           it 'returns excluded subclasses' do
             subclass = Class.new
             described_class.exclude_subclass(subclass)
@@ -75,14 +79,12 @@ if RUBY_VERSION >= '3.1'
             described_class.exclude_subclass(subclass)
 
             subclass = nil
-
             GC.start
 
             expect(described_class.excluded_subclasses).to eq([])
           end
 
           it 'does not return excluded subclasses that raises a ::WeakRef::RefError' do
-            require 'weakref'
             subclass = double(:weakref_alive? => true)
             described_class.instance_variable_set(:@excluded_subclasses, [subclass])
 


@excluded_subclasses ||= []
@excluded_subclasses.select(&:weakref_alive?).map do |ref|
begin # rubocop:disable Style/RedundantBegin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It slipped my mind, isn’t it possible to squash that so that rescue is on the map itself?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, nevermind, Ruby 1.8 probably doesn’t support that. Do you mind making a note to indicate that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the comment because the CI was failing because of that (I don't have the same behavior locally with the same version of Ruby/Rubocop).

ref.__getobj__
rescue ::WeakRef::RefError
nil
end
end.compact
end

def exclude_subclass(constant)
require 'weakref' unless defined?(::WeakRef)

@excluded_subclasses ||= []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in a few places. Might be it belongs to enable!, too?

@excluded_subclasses << ::WeakRef.new(constant)
end
end
end
end
end
2 changes: 2 additions & 0 deletions lib/rspec/mocks/mutate_const.rb
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ def to_constant
end

def reset
RSpec::Mocks::ExcludeStubbedClassesFromSubclasses.exclude_subclass(@mutated_value) if RSpec::Mocks.configuration.exclude_stubbed_classes_from_subclasses?
@constants_to_transfer.each do |const|
@mutated_value.__send__(:remove_const, const)
end
Expand Down Expand Up @@ -297,6 +298,7 @@ def to_constant
end

def reset
RSpec::Mocks::ExcludeStubbedClassesFromSubclasses.exclude_subclass(@mutated_value) if RSpec::Mocks.configuration.exclude_stubbed_classes_from_subclasses?
@parent.__send__(:remove_const, @const_name)
end

Expand Down
97 changes: 97 additions & 0 deletions spec/rspec/mocks/exclude_stubbed_classes_from_subclasses_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
if RUBY_VERSION >= '3.1'
class TestClass
end

module RSpec
module Mocks
RSpec.describe ExcludeStubbedClassesFromSubclasses do
after do
described_class.disable!
end

describe '.enable!' do
it 'extends Class with methods' do
expect {
described_class.enable!
}.to change { Class.respond_to?(:subclasses_with_rspec_mocks) }.from(false).to(true)
end

it 'does not extends Class when it has been enabled' do
allow(Class).to receive(:class_eval).and_call_original

described_class.enable!
described_class.enable!

expect(Class).to have_received(:class_eval).once
end

it 'excludes stubbed classes from subclasses' do
described_class.enable!

orignal_subclasses = TestClass.subclasses

subclass = Class.new(TestClass)
described_class.exclude_subclass(subclass)

expect(TestClass.subclasses).to an_array_matching(orignal_subclasses)
end
end

describe '.disable!' do
it 'does nothing when it has not been enabled' do
expect { described_class.disable! }.not_to raise_error
end

it 'removes methods from class when it has been enabled' do
described_class.enable!
expect {
described_class.disable!
}.to change { Class.respond_to?(:subclasses_with_rspec_mocks) }.from(true).to(false)
end

it 'does not exclude stubbed classes from subclasses' do
described_class.enable!
described_class.disable!

orignal_subclasses = TestClass.subclasses

subclass = Class.new(TestClass)
described_class.exclude_subclass(subclass)

expect(TestClass.subclasses).to an_array_matching(orignal_subclasses + [subclass])
end
end

describe '.excluded_subclasses' do
it 'returns excluded subclasses' do
subclass = Class.new
described_class.exclude_subclass(subclass)

expect(described_class.excluded_subclasses).to an_array_matching([subclass])
end

it 'does not return excluded subclasses that have been garbage collected' do
subclass = Class.new
described_class.exclude_subclass(subclass)

subclass = nil

GC.start

expect(described_class.excluded_subclasses).to eq([])
end

it 'does not return excluded subclasses that raises a ::WeakRef::RefError' do
require 'weakref'
subclass = double(:weakref_alive? => true)
described_class.instance_variable_set(:@excluded_subclasses, [subclass])

allow(subclass).to receive(:__getobj__).and_raise(::WeakRef::RefError)

expect(described_class.excluded_subclasses).to eq([])
end
end
end
end
end
end
28 changes: 28 additions & 0 deletions spec/rspec/mocks/mutate_const_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,20 @@ def change_const_value_to(value)
it 'returns nil' do
expect(hide_const(const_name)).to be_nil
end

if RUBY_VERSION >= '3.1'
describe 'with global exclude_stubbed_classes_from_subclasses option set' do
include_context "with isolated configuration"
include_context "with stubbed classes excluded from subclasses"

it 'gives the same subclasses after rspec clears its mocks' do
original_subclasses = TestClass.subclasses
stub_const(const_name, Class.new(TestClass))
reset_rspec_mocks
expect(TestClass.subclasses).to an_array_matching(original_subclasses)
end
end
end
end

describe "#hide_const" do
Expand Down Expand Up @@ -351,6 +365,20 @@ def change_const_value_to(value)
end
end
end

if RUBY_VERSION >= '3.1'
describe 'with global exclude_stubbed_classes_from_subclasses option set' do
include_context "with isolated configuration"
include_context "with stubbed classes excluded from subclasses"

it 'gives the same subclasses after rspec clears its mocks' do
original_subclasses = TestClass::Nested.subclasses
stub_const('TestClass', Class.new(TestClass::Nested))
reset_rspec_mocks
expect(TestClass::Nested.subclasses).to an_array_matching(original_subclasses)
end
end
end
end

context 'for a loaded nested constant' do
Expand Down
10 changes: 10 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,13 @@ def self.fake_matcher_description
RSpec::Mocks.configuration.syntax = orig_syntax
end
end

RSpec.shared_context "with stubbed classes excluded from subclasses" do
before do
RSpec::Mocks.configuration.exclude_stubbed_classes_from_subclasses = true
end

after do
RSpec::Mocks.configuration.exclude_stubbed_classes_from_subclasses = false
end
end
Loading