stub_const fails to set name of stubbed Class if example is not in context

[istolpov]

Hello! I found strange issue with stub_const helper:

Given lib/conveyor.rb

module Conveyor
  class NotFound < ArgumentError; end

  def self.find_module(key)
    raise NotFound.new(key)
  end

  def self.registered_modules
    []
  end

  def self.register(klass)
    return if registered_modules.include? klass
    mod = Module.new do
      define_method :find_module do |key|
        (key.to_s == klass.name.underscore.match(/\/(\w*)$/)[1]) ? klass : super(key)
      end

      define_method :registered_modules do
        super() + [klass]
      end
    end
    singleton_class.prepend mod
  end
end

lib/conveyor_modules.rb

module ConveyorModules
  class Base
  end
end

spec/test_spec.rb

require 'spec_helper'
require 'active_support/all'
require_relative '../lib/conveyor'
require_relative '../lib/conveyor_modules/base'


RSpec.describe Conveyor do
  describe '.register' do
    let!(:module_not_found) { stub_const 'Conveyor::NotFound', Class.new(ArgumentError) }

    let!(:s_module) { stub_const 'ConveyorModules::Foos', Class.new(ConveyorModules::Base) }

    before do
      subject.register s_module
    end

    context 'when key does not match' do
      it 'passes the .find_module message down the chain' do
        expect{subject.find_module 'not_foos'}.to raise_exception(module_not_found)
      end
    end

    it 'returns all registered modules' do
      expect(subject.registered_modules).to eq([s_module])
    end

    # context 'foo' do
    #   it 'returns all registered modules' do
    #     expect(subject.registered_modules.map(&:name)).to eq([s_module, s_module].map(&:name))
    #   end
    # end
  end
end

rspec -fd returns that:

Conveyor
  .register
    returns all registered modules
    when key does not match
      passes the .find_module message down the chain (FAILED - 1)

Failures:

  1) Conveyor.register when key does not match passes the .find_module message down the chain
     Failure/Error: expect{subject.find_module 'not_foos'}.to raise_exception(module_not_found)
     
       expected Conveyor::NotFound, got #<NoMethodError: undefined method `underscore' for nil:NilClass> with backtrace:
         # ./lib/conveyor.rb:16:in `block (2 levels) in register'
         # ./lib/conveyor.rb:16:in `block (2 levels) in register'
         # ./spec/test_spec.rb:19:in `block (5 levels) in <top (required)>'
         # ./spec/test_spec.rb:19:in `block (4 levels) in <top (required)>'
     # ./spec/test_spec.rb:19:in `block (4 levels) in <top (required)>'

Finished in 0.01177 seconds (files took 0.15501 seconds to load)
2 examples, 1 failure

Failed examples:

rspec ./spec/test_spec.rb:18 # Conveyor.register when key does not match passes the .find_module message down the chain

if commented example (with context) is uncommented, and same without context is commented, that suite passes

# it 'returns all registered modules' do
#   expect(subject.registered_modules).to eq([s_module])
# end

context 'foo' do
  it 'returns all registered modules' do
    expect(subject.registered_modules.map(&:name)).to eq([s_module, s_module].map(&:name))
  end
end

if we decide to use one without context, but explicitly call s_module.name in before clause it works just fine too

before do
  s_module.name
  subject.register s_module
end

it 'returns all registered modules' do
  expect(subject.registered_modules).to eq([s_module])
end

# context 'foo' do
#   it 'returns all registered modules' do
#     expect(subject.registered_modules.map(&:name)).to eq([s_module, s_module].map(&:name))
#   end
# end

So, it seems to me when it's out of context it executes first and somehow messes with the RSpec, and when we call name explicitly, that fixes the bug. Ruby sets name to the module when it is assigned to constant, so RSpec somewhere assigns it(or mimics that behavior), and when without context and name is not explicitly called that assign is not triggered anywhere.

I figured that when run all 3 examples without mapping :name to last one: spec/test_spec.rb

it 'returns all registered modules' do
  expect(subject.registered_modules).to eq([s_module])
end

context 'foo' do
  it 'returns all registered modules' do
    expect(subject.registered_modules).to eq([s_module, s_module])
  end
end

produces output

Conveyor
  .register
    returns all registered modules
    when key does not match
      passes the .find_module message down the chain (FAILED - 1)
    foo
      returns all registered modules (FAILED - 2)

Failures:

  1) Conveyor.register when key does not match passes the .find_module message down the chain
     Failure/Error: expect{subject.find_module 'not_foos'}.to raise_exception(module_not_found)
     
       expected Conveyor::NotFound, got #<NoMethodError: undefined method `underscore' for nil:NilClass> with backtrace:
         # ./lib/conveyor.rb:16:in `block (2 levels) in register'
         # ./lib/conveyor.rb:16:in `block (2 levels) in register'
         # ./spec/test_spec.rb:19:in `block (5 levels) in <top (required)>'
         # ./spec/test_spec.rb:19:in `block (4 levels) in <top (required)>'
     # ./spec/test_spec.rb:19:in `block (4 levels) in <top (required)>'

  2) Conveyor.register foo returns all registered modules
     Failure/Error: expect(subject.registered_modules).to eq([s_module, s_module])
     
       expected: [ConveyorModules::Foos, ConveyorModules::Foos]
            got: [#<Class:0x00000002c493c8>, ConveyorModules::Foos, ConveyorModules::Foos]
     
       (compared using ==)
     
       Diff:
       @@ -1,2 +1,2 @@
       -[ConveyorModules::Foos, ConveyorModules::Foos]
       +[#<Class:0x00000002c493c8>, ConveyorModules::Foos, ConveyorModules::Foos]
       
     # ./spec/test_spec.rb:29:in `block (4 levels) in <top (required)>'

Finished in 0.01312 seconds (files took 0.1532 seconds to load)
3 examples, 2 failures

Failed examples:

rspec ./spec/test_spec.rb:18 # Conveyor.register when key does not match passes the .find_module message down the chain
rspec ./spec/test_spec.rb:28 # Conveyor.register foo returns all registered modules

Do not pay attention to difference like [s_module, s_module] != [s_module] and mapping :name, i know why those fail, that is not the point of report.

Test suite is available here: https://github.com/wyde19/rspec_fail

[xaviershay]

Thanks for the report! There's a lot going on here and I couldn't quite grok it all on a first pass. I'll try digging in more when I have some more time, but also if there is anything incidental in here you could remove that would be really helpful! For example:

• Is the third example required?
• The commented example and the uncommented one aren't the same. (So context might not be the defining characteristic?) Does this still repro if you make them the same?
• There's a lot going on in the .register method. How much for that is required for a repro?

[JonRowe]

I suspect this will be related to Rails autoloading, (brought in by require 'active_support/all' and how that interacts with the missing constant.

[istolpov]

@xaviershay I was trying to implement some extendable pipeline. Conveyor class is prepended by modules for each plug-in into pipeline, one finds respective to the key class in the pipeline and the other returns all pipeline modules.

Third example (if you are talking about it 'passes the .find_module message down the chain' do) is the one that fails. You see, when anonymous class is registered in pipeline, not matching anything key does not reach the root method (the Conveyor.find_module defined statically on Conveyor. Instead, it reaches one of the methods of prepended module, which have anonymous class enclosed in it, tries to call name.underscore on it and crashes into NoMethodError for nil:NilClass)

The commented example is the same from the issue's point of view. They differ in array size, in converting to string, but those are because when we register new module in before, we have unique s_module everytime (because of let) and so safeguard in methodregister does not work, it still registers the same (but not from the ruby point of view) class over and over. Hence comparison by class names, not factial objects, hence different size of array in different examples. Play with it yourself, you'll see what I mean.

That all originally was part of library in project at my work and I cut pretty much all I could to isolate the problem and still cause it to reproduce. All .register does, as I described above is overrides 2 methods on conveyor both of them are tested in examples. I tried to reproduce similar situation, but simply enclosed klass.name.nil? in method defined in module in .register and tested against that. Does not reproduce.

[istolpov]

@JonRowe probably not. You see, activesupport is there for inflections, namely klass.name.underscore which caused the error leading to discovery of the bug. There is no autoload expressions in the test app, moreover, no class extends or includes any activesupport module. Hence, only core extensions work. I could probably try to include core extensions only, but I didn't -- thought didn't cross my mind.

[JonRowe]

@wyde19 I'm not talking about the Ruby autoload declaration, but the fact active_support hijacks const_missing to load files, if you wish to use inflections I suggest you load only the inflections rather than require 'active_support/all'.

I suspect if you do a --backtrace you'll see active support lines in your tack trace.

[wtfiwtz]

To get around issues with autoloading I ended up doing this instead of using stub_const:

after(:each) do
  Base::Class.send(:remove_const, 'DISPATCH_STRATEGY')
  Base::Class.const_set('DISPATCH_STRATEGY', :original_value)
end

%i[a_value another_value original_value].each do 
  it "should work for all 3 constants" do
    Base::Class.send(:remove_const, 'DISPATCH_STRATEGY')
    Base::Class.const_set('DISPATCH_STRATEGY', kind)
  end
end

Not sure if there will be any issues down the track but it seems to work ok (... and no flakey tests yet!)

[JonRowe]

Closing due to inactivity, and a reasonable workaround? during the monorepo migration.