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

Prevent mocks leaking out of their scope when mocked more than once in different nested scopes. #1424

Open
expeehaa opened this issue Jun 3, 2021 · 3 comments

Comments

@expeehaa
Copy link
Contributor

expeehaa commented Jun 3, 2021

Subject of the issue

When mocking a method in an outer and inner scope using combinations of expect and allow, the mocks leak out of the inner scope. I suppose that this is a bug because it only happens when the method was first mocked in the outer scope.
If it is a feature it is a pretty unintuitive one.

Your environment

  • Ruby version: 2.6.6, 3.0.1
  • rspec-mocks version: 3.10.2

Steps to reproduce

#!/usr/bin/env ruby
# 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.10.0'
  gem 'rspec-mocks', '3.10.2'
end

puts "Ruby version is: #{RUBY_VERSION}"
require 'rspec/autorun'

RSpec.describe 'nested space' do
  # Succeeds.
  it 'does not leak without previous mocks' do
    String.new

    RSpec::Mocks.with_temporary_scope do
      allow(String).to receive(:new).once do
        allow(String).to receive(:new).and_raise 'fail'
      end

      String.new
    end

    expect{String.new}.not_to raise_error
  end

  context 'when mocking with allow in the outer scope' do
    context 'when mocking with allow in the inner scope' do
      # Fails.
      it 'does not leak return values' do
        allow(String).to receive(:new).once

        String.new

        RSpec::Mocks.with_temporary_scope do
          allow(String).to receive(:new).once do
            allow(String).to receive(:new).and_raise 'fail'
          end

          String.new
        end

        expect{String.new}.not_to raise_error
      end
    end

    context 'when mocking with expect in the inner scope' do
      # Fails.
      it 'does not leak counting' do
        allow(String).to receive(:new).once

        String.new

        RSpec::Mocks.with_temporary_scope do
          expect(String).to receive(:new).once.and_return('test')

          String.new
        end

        expect(String.new).not_to eq 'test'
      end
    end
  end

  context 'when mocking with expect in an outer scope' do
    context 'when mocking with allow in the inner scope' do
      # Fails.
      it 'does not leak return values' do
        expect(String).to receive(:new).once

        String.new

        RSpec::Mocks.with_temporary_scope do
          allow(String).to receive(:new).once do
            allow(String).to receive(:new).and_raise 'fail'
          end

          String.new
        end

        expect{String.new}.not_to raise_error
        # Even if the line above did not fail, the test would fail because String.new was called 3 times instead of just once.
        pending
      end

      # Fails.
      it 'does not leak counting' do
        expect(String).to receive(:new).twice

        String.new

        RSpec::Mocks.with_temporary_scope do
          allow(String).to receive(:new).twice

          String.new
          String.new
        end

        String.new
      end
    end
  end
end

Expected behavior

All tests should pass.

Actual behavior

Failures:

  1) nested space when mocking with allow in the outer scope when mocking with allow in the inner scope does not leak return values
     Failure/Error: expect{String.new}.not_to raise_error
     
       expected no Exception, got #<RuntimeError: fail> with backtrace:
         # test_spec.rb:53:in `block (5 levels) in <main>'
         # test_spec.rb:53:in `block (4 levels) in <main>'
     # test_spec.rb:53:in `block (4 levels) in <main>'

  2) nested space when mocking with allow in the outer scope when mocking with expect in the inner scope does not leak counting
     Failure/Error: expect(String.new).not_to eq 'test'
     
       (String (class)).new(no args)
           expected: 1 time with any arguments
           received: 2 times
     # test_spec.rb:70:in `block (4 levels) in <main>'

  3) nested space when mocking with expect in an outer scope when mocking with allow in the inner scope does not leak return values
     Failure/Error: expect{String.new}.not_to raise_error
     
       expected no Exception, got #<RuntimeError: fail> with backtrace:
         # test_spec.rb:91:in `block (5 levels) in <main>'
         # test_spec.rb:91:in `block (4 levels) in <main>'
     # test_spec.rb:91:in `block (4 levels) in <main>'

  4) nested space when mocking with expect in an outer scope when mocking with allow in the inner scope does not leak counting
     Failure/Error: String.new
     
       (String (class)).new(no args)
           expected: 2 times with any arguments
           received: 3 times
     # test_spec.rb:109:in `block (4 levels) in <main>'

Finished in 0.01682 seconds (files took 0.05268 seconds to load)
5 examples, 4 failures

Failed examples:

rspec test_spec.rb:40 # nested space when mocking with allow in the outer scope when mocking with allow in the inner scope does not leak return values
rspec test_spec.rb:59 # nested space when mocking with allow in the outer scope when mocking with expect in the inner scope does not leak counting
rspec test_spec.rb:78 # nested space when mocking with expect in an outer scope when mocking with allow in the inner scope does not leak return values
rspec test_spec.rb:97 # nested space when mocking with expect in an outer scope when mocking with allow in the inner scope does not leak counting
@pirj
Copy link
Member

pirj commented Jun 3, 2021

Looks reasonable. Would you like to hack on this?

Wondering how do you use nested spaces in your specs, can you give a hint?

@expeehaa
Copy link
Contributor Author

expeehaa commented Jun 3, 2021

I'll maybe try to write a fix, but it could take a while. If someone else wants to do it earlier I'ld be totally fine with it.

Regarding the usage: I have a class that is instantiated with some well-defined arguments and the initializer also conditionally calls a method (let's call it C.rnd_method) that returns a random value that I unfortunately cannot pass to the method. My spec tests a method that uses this random value and returns a string with it. I don't want to interpolate the expected string, so I created a mock for the class initializer that mocks the random-value-returning method to return a specific value and calls the original initializer method.
Since C.rnd_method is also used in other parts of the test code I wrapped the mock method code in RSpec::Mocks.with_temporary_scope.

Something like this (very much simplified):

class A
  def initialize(*args)
    if some_condition
      @test = C.rnd_method
    end
    
    ...
  end
end

RSpec.describe A do
  before(:each) do
    allow(A).to receive(:new).and_wrap_original do |method, *args|
      RSpec::Mocks.with_temporary_scope do
        allow(C).to receive(:rnd_method).once.and_return(5)
        
        method.call(*args)
      end
    end
  end
  
  it 'works' do
    expect(C).to receive(:rnd_method).twice.and_return(2)
    
    some_method_that_calls_A_new_and_C_rnd_method
  end
end

I suppose that, even though this is a simplified version, the real code could be optimized to not need the temporary scope.

@Inversion-des
Copy link

Inversion-des commented Nov 3, 2021

I have some different problem with nested scopes. Maybe it is related…

The next code produces unexpected results for nested scopes:

class Keys
  def Keys.private_key = '0'
end

RSpec.describe 'cases' do
  it 'not nested mock' do
    puts Keys.private_key
		
    RSpec::Mocks.with_temporary_scope do
      allow(Keys).to receive(:private_key).and_return('1')
      puts ' '+Keys.private_key
    end
    
    puts Keys.private_key
		
    RSpec::Mocks.with_temporary_scope do
      allow(Keys).to receive(:private_key).and_return('2')
      puts '' + Keys.private_key
    end
    
    puts Keys.private_key
		
    RSpec::Mocks.with_temporary_scope do
      allow(Keys).to receive(:private_key).and_return('3')
      puts ' '+Keys.private_key
    end

    puts Keys.private_key
		
    RSpec::Mocks.with_temporary_scope do
      allow(Keys).to receive(:private_key).and_return('4')
      puts ' '+Keys.private_key
    end

    puts Keys.private_key
  end

  it 'nested mock' do
    puts Keys.private_key

    RSpec::Mocks.with_temporary_scope do
      allow(Keys).to receive(:private_key).and_return('1')
      
      puts ' '+Keys.private_key
      
      RSpec::Mocks.with_temporary_scope do
        allow(Keys).to receive(:private_key).and_return('2')

        puts '  '+Keys.private_key

        RSpec::Mocks.with_temporary_scope do
          allow(Keys).to receive(:private_key).and_return('3')

          puts '   '+Keys.private_key

          RSpec::Mocks.with_temporary_scope do
            allow(Keys).to receive(:private_key).and_return('4')

            puts '    '+Keys.private_key
          end

          puts '   '+Keys.private_key
        end

        puts '  '+Keys.private_key
      end
			
      puts ' '+Keys.private_key
    end
    
    puts Keys.private_key
  end
end

Results:

not nested mocks:
0
 1
0
 2
0
 3
0
 4
0

nested mocks:
0
 1
  2
   3
    4
   4
  2
 2
0
  • Ruby v3.0.2.107 (2021-07-07) [x64-mingw32]
  • rspec-core: v3.10.1
  • rspec-mocks: v3.10.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants