Rspec-core: RSpec recovers from autoloading failures leading to partially loaded classes

Created on 2 Feb 2017  路  11Comments  路  Source: rspec/rspec-core

Consider the following example, in a Rails project (where lib has been added to the autoload path):

# lib/alice.rb
class Alice
  def self.alice
    Bob.bob
  end
end

# lib/alice/bob.rb
class Alice
  class Bob
    # try loading a constant that's not defined anywhere:
    Carol
    def self.bob
      puts "hello from bob!"
    end
  end
end

# spec/alice_spec.rb
RSpec.describe Alice do
  it "does something" do
    Alice.alice
  end

  it "does something else" do
    Alice.alice
  end
end
# bundle exec rspec spec/alice_spec.rb
Randomized with seed 27098
FF

Failures:

  1) Alice does something else
     Failure/Error: Carol

     NameError:
       uninitialized constant Alice::Bob::Carol
     # ./lib/alice/bob.rb:3:in `<class:Bob>'
     # ./lib/alice/bob.rb:2:in `<class:Alice>'
     # ./lib/alice/bob.rb:1:in `<top (required)>'
     # ./lib/alice.rb:3:in `alice'
     # ./spec/alice_spec.rb:7:in `block (2 levels) in <top (required)>'

  2) Alice does something
     Failure/Error: Bob.bob

     NoMethodError:
       undefined method `bob' for Alice::Bob:Class
     # ./lib/alice.rb:3:in `alice'
     # ./spec/alice_spec.rb:3:in `block (2 levels) in <top (required)>'

Finished in 0.63286 seconds (files took 4.93 seconds to load)
2 examples, 2 failures

Failed examples:

rspec ./spec/alice_spec.rb:6 # Alice does something else
rspec ./spec/alice_spec.rb:2 # Alice does something

What's interesting to me is that this fails in two different ways, even though both tests are doing the same thing. I'm guessing that the first test that gets run starts to load Bob, blows up because it fails to find Carol. I think RSpec is rescuing the exception, leaving a partially-loaded Bob class around in memory, hence the next test fails with a NoMethodError. I'm not sure if this is a problem that can be solved, or should be solved here, but sharing nonetheless!

Most helpful comment

This isn't an RSpec issue, the second failure in your contrived example is caused by your first failure, in a "pure ruby" situation it would exit after the raise, but we carry on because errors are treated as failures, thus class is defined but it never reaches def self.foo.

"just really odd behaviour that sort of undermines how I think Ruby works."

Is a great way of describing Rails autoloading ;) :joy:

All 11 comments

RSpec is completely agnostic about how your code should be loaded. It does not do autoloading. You either need to manually require files (what I tend to do), use ruby autoloading (e.g. via autoload :ClassName, "path/to/class_name") or use Rails autoloading.

If you are using ruby autoloading or rails autoloading, and either one isn't working how you expect, you need to take up your issue with the maintainers of ruby and/or rails.

@myronmarston Rails' autoloading is working how I expect - I can't reproduce this issue outside of RSpec. If something gets loaded when an example is running, it looks like RSpec is interfering with how things get cleaned up when autoloading fails.

@myronmarston Rails' autoloading is working how I expect - I can't reproduce this issue outside of RSpec. If something gets loaded when an example is running, it looks like RSpec is interfering with how things get cleaned up when autoloading fails.

RSpec does not have any code designed to interact with Rails autoloading in any way. My guess is that it's the simple fact that the autoloading is happening while an RSpec example is running, and RSpec rescues almost all exceptions that happen in an example. Our rescue of the error might cause this, but I'm not sure what we should do different.

Can you put together a repo that reproduces the issue?

@myronmarston reproducer here: https://github.com/imtayadeway/rspec_autoloading_issue. I'm also happy to dig more into this myself, just haven't yet in case you definitely didn't want RSpec to deal with this, just let me know!

just haven't yet in case you definitely didn't want RSpec to deal with this, just let me know!

Well, I would definitely prefer that we don't have any code written to deal with Rails autoloading (and am hopeful that we won't have to) but there may be something we are doing that is poorly behaved that interacts with Rails somehow, and it's at least digging into to understand how we are affecting this (since I don't think we should be).

@rspec/rspec -- are any of you (particularly those of you who use rails) available to help @imtayadeway dig into this? I don't have any time to spend on this.

@imtayadeway I'd says Rails is wrong in this case, if Carol is not defined anywhere you should get an error, if it is defined somewhere then you should be able to load it in advance, are you using eager loading?

@JonRowe I'm not using eager loading in the test environment. I do get an error on the first failure to load, but it leaves behind an empty shell of a class which fails in the second example on a NoMethodError.

@myronmarston I am totally happy to - will let you know if I find anything on RSpec's side. Thanks! :heart:

I do get an error on the first failure to load, but it leaves behind an empty shell of a class which fails in the second example on a NoMethodError.

That sounds like a rails autoloading issue, I suggest treating test like production and turning on eager loading.

@JonRowe yeah, that should prevent that from happening, but I anticipate the consensus on my team being against doing that.

If it helps, I've managed to contrive a much smaller reproduction that removes Rails from the picture. I believe though, that it is fundamentally what Rails is doing when it autoloads:

RSpec.describe "eval" do
  it "does something" do
    load_foo unless Module.const_defined?(:Foo)
    Foo.foo
  end

  it "does something else" do
    load_foo unless Module.const_defined?(:Foo)
    Foo.foo
  end

  def load_foo
    eval(
      <<-EOF
class Foo
  raise

  def self.foo
    puts "hi from foo!"
  end
end
EOF
    )
  end
end

When run, this gives me:

Failures:

  1) eval does something
     Failure/Error:
           eval(
             <<-EOF
       class Foo
         raise

         def self.foo
           puts "hi from foo!"
         end
       end
       EOF

     RuntimeError:
       (eval):2:in `<class:Foo>': 
     # ./spec/models/alice_spec.rb:13:in `eval'
     # (eval):1:in `load_foo'
     # ./spec/models/alice_spec.rb:13:in `eval'
     # ./spec/models/alice_spec.rb:13:in `load_foo'
     # ./spec/models/alice_spec.rb:3:in `block (2 levels) in <top (required)>'

  2) eval does something else
     Failure/Error: Foo.foo

     NoMethodError:
       undefined method `foo' for Foo:Class
     # ./spec/models/alice_spec.rb:9:in `block (2 levels) in <top (required)>'

Finished in 0.00065 seconds (files took 0.0608 seconds to load)
2 examples, 2 failures

Failed examples:

rspec ./spec/models/alice_spec.rb:2 # eval does something
rspec ./spec/models/alice_spec.rb:7 # eval does something else

This is an obviously contrived example, the solution to which I would ordinarily identify as "don't do that". But for illustration purposes, it's essentiailly the same issue as when Rails fails to autoload within an example. I'm not going to insist this is an RSpec issue, just really odd behavior that sort of undermines how I think Ruby works.

This isn't an RSpec issue, the second failure in your contrived example is caused by your first failure, in a "pure ruby" situation it would exit after the raise, but we carry on because errors are treated as failures, thus class is defined but it never reaches def self.foo.

"just really odd behaviour that sort of undermines how I think Ruby works."

Is a great way of describing Rails autoloading ;) :joy:

I've closed this because I believe RSpec's behaviour is the correct behaviour here. Carol doesn't exist and prevents the definition of the file's methods, so the errors are to be expected. I'm not sure why it works in Rails, it must be defining the constant so the file can be evaluated, but this is not something RSpec should do.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

fimmtiu picture fimmtiu  路  3Comments

phuongnd08 picture phuongnd08  路  5Comments

take picture take  路  5Comments

Drenmi picture Drenmi  路  4Comments

ankit8898 picture ankit8898  路  3Comments