rspec / rspec-mocks

RSpec's 'test double' framework, with support for stubbing and mocking
https://rspec.info
MIT License
1.16k stars 357 forks source link

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

Open expeehaa opened 3 years ago

expeehaa commented 3 years ago

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

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 commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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