'not defined constant' errors when using instance_double

[floydj]

I have the following:

let(:project) { instance_double 'Project' }

But when I run specs that access this double, I get Project is not a defined constant. Perhaps you misspelt it?

I have the following in my spec_helper.rb file:

config.mock_with :rspec do |mocks|
  mocks.verify_doubled_constant_names = true
end

I have a Project class with:

class Project < ActiveRecord::Base

What am I missing here?

[myronmarston]

At the time your spec runs, Project is not a defined constant. Are you loading it anywhere (e.g. with require 'project' or similar)?

If you are relying upon rails autoloading, you can change instance_double 'Project' to instance_double Project (it works fine to pass the class itself rather than the string) -- referencing the constant will cause rails to load the model.

In general, if you always want your verifying doubles to verify and are willing to pay the cost of loading the classes, passing the class as a constant reference works simpler/better than using the string form and verify_doubled_constant_names = true. OTOH, using the string form provides a way to write an isolated spec that doesn't require the named class to be loaded but will verify when it is loaded. When doing that, I tend to set verify_doubled_constant_names = true in a config file that is only loaded when all specs are run (and all implementation files are loaded). For individual spec files, using verify_doubled_constant_names = true kinda destroys the whole point of passing the class name as a string, as it prevents you from being able to run the spec w/o the class loaded, which is the entire point of supporting the string form in the first place.

Closing, but let me know if that doesn't make sense and we can reopen.

[floydj]

@myronmarston - Thanks for the explanation.

But when I use this:

let(:project) { instance_double(Project, metric?: false) }

I now get Project does not implement: metric?, even though Project does have metric as a boolean field.

[floydj]

@myronmarston - please disregard. I figured it out. Thanks again.

[mockdeep]

@myronmarston would it make sense for RSpec to anticipate autoloading and try to resolve it? It seems like it could be something like:

begin
  const_get(class_name)
rescue NameError
  # show error message
end

[JonRowe]

@mockdeep not really because that behaviour isn't prevalent outside of Rails, it's much more common to have to require the things you expect to use. If you're using rspec in rails environment using the constant directly is a much better way to force the desired behaviour.

[mockdeep]

@JonRowe I guess that makes sense, though it seems like it would make the implementation a little more flexible for these kinds of use cases (as well as less confusing). I'll use the constant for now.

[myronmarston]

@mockdeep - if you are wanting/expecting the class to be loaded, why use the string form at all? The entire reason the string form exists is for situations where the user doesn't want the class loaded, so adding logic to it to load the const goes against its entire purpose.

[mockdeep]

@myronmarston So I can think of two arguments in favor of a more flexible implementation. The first is surprise. Various examples on the internet (including Avdi's video) use the string syntax, so when someone tries to emulate that and it doesn't work for them, they're going to have to google around to figure out what happened, or they'll open an issue on RSpec, or maybe they'll just give up. There's this subtle caveat that if you're in Rails or another auto-loaded environment, you have to use this other syntax in order to get it to work.

The other point is that one might want to be able to toggle the verification on and off. I could see a use case that while you're driving something out you aren't necessarily going to be worried that the interfaces are spot on, so you would be inclined to use the string syntax, but once the individual components are in place you could toggle on the verification. Right now it sort of pushes a "depth first" approach to testing in a Rails environment, as you have to define each of the classes you're mocking out down the tree. I personally don't mind, but I could see it being a little frustrating at times.

[JonRowe]

You forget that Rails is causing the surprising case here, and even then only without eager_loading turned on, in normal Ruby the string usage behaves as expected, you'd have to require the class before use.

[mockdeep]

That's true, I'm not blaming RSpec for the confusion necessarily, but it's there. @myronmarston explained it in the comments for the video, but eager_loading doesn't really make much sense either, since you might only want to run an individual spec, which shouldn't load all of the classes in the app.

[JonRowe]

Rails autoloading has more confusing behaviour than just this. If you expect something to be present you should load the entire of app or specify the bits you do want.

[mockdeep]

How so? I haven't run into problems with it in my tests, aside from making sure all of my folders are included in the autoload paths. I've never had to manually require individual files for it.

[JonRowe]

It has some quirks regarding name spaced constants and the like

[myronmarston]

The first is surprise. Various examples on the internet (including Avdi's video) use the string syntax, so when someone tries to emulate that and it doesn't work for them, they're going to have to google around to figure out what happened, or they'll open an issue on RSpec, or maybe they'll just give up.

I'm thinking we should update our docs to show the non-string form and explain when you'd want to use one vs the other.

There's this subtle caveat that if you're in Rails or another auto-loaded environment, you have to use this other syntax in order to get it to work.

What? I admittedly haven't done a real rails project since the rails 2 days, but I find this surprising. I believe @xaviershay (the designer of verifying doubles) tends to use the string form in rails projects without any issues. What makes you say you have to use the non-string syntax to get it to work? Is it just that RSpec doesn't load the constant for you? (if so, that's it working as designed).

The other point is that one might want to be able to toggle the verification on and off. I could see a use case that while you're driving something out you aren't necessarily going to be worried that the interfaces are spot on, so you would be inclined to use the string syntax, but once the individual components are in place you could toggle on the verification.

This is what the string form is meant to provide -- you can use the double before the interface is defined, and later, once it is defined, when your test that uses the verified double runs with the class definition loaded, the verification will be enabled.

[mockdeep]

What? I admittedly haven't done a real rails project since the rails 2 days, but I find this surprising. I believe @xaviershay (the designer of verifying doubles) tends to use the string form in rails projects without any issues. What makes you say you have to use the non-string syntax to get it to work? Is it just that RSpec doesn't load the constant for you? (if so, that's it working as designed).

So I think we might be talking about different scenarios here. I'm not sure how @xaviershay handles it, of course, but there is the situation that is automatically provided, and then the more strict configuration mocks.verify_doubled_constant_names = true. The implicit version only takes effect when the class has been loaded, which doesn't happen in an auto-loaded environment until it has been referenced, so the protection might not ever be triggered unless you use the constant syntax, which defies the point of being able to use the double before the interface or class has been defined.

Setting the stricter verify_doubled_constant_names doesn't even work in an auto-loaded environment if you use the string option, since it fails with "Blah" is not a defined constant, even if the class exists, because, again, the class hasn't been referenced yet to trigger the auto-load.

In general, I think it makes sense to implement Rails specific adjustments in rspec-rails, but here it seems a little more fuzzy. A small adjustment in the implementation here would make both strategies work transparently for autoloaded environments and otherwise. It seems to me like at least in the second case it should try triggering an autoload for a class before giving the error, though a reasonable approach to me seems to be something like:

begin
  klass = const_get(class_name)
rescue NameError
  # if `mocks.verify_doubled_constant_names = true`
    # make noise
  # else
    # don't worry about it
end

[myronmarston]

So I think we might be talking about different scenarios here. I'm not sure how @xaviershay handles it, of course, but there is the situation that is automatically provided, and then the more strict configuration mocks.verify_doubled_constant_names = true.

Yep, I think we are talking about different situations here. I didn't realize you were talking about mocks.verify_doubled_constant_names = true...

The implicit version only takes effect when the class has been loaded, which doesn't happen in an auto-loaded environment until it has been referenced, so the protection might not ever be triggered unless you use the constant syntax, which defies the point of being able to use the double before the interface or class has been defined.

Something else in your environment should be loading or referencing the class constant though, right? If you're depending upon RSpec to load the constant than how would it ever get loaded in dev and prod?

(As an aside, this is one reason I dislike rails autoloading: there's a lot of uncertainty about whether or not a specific thing is loaded or not).

Regardless, now that I know you're specifically talking about when verify_doubled_constant_names = true is set, I think it might make sense to try to force the constant to load if that config option is set.

[mockdeep]

Something else in your environment should be loading or referencing the class constant though, right? If you're depending upon RSpec to load the constant than how would it ever get loaded in dev and prod?

In development and test the classes are loaded on demand when they are referenced. In production Rails goes through the auto-load paths and requires everything at boot time. In the case of tests where you're mocking out the class under question, it wouldn't necessarily be loaded, so it makes sense to me to have RSpec try to reference it and respond appropriately if it doesn't pop up. It's a semantic difference, but I wouldn't see it so much as getting "RSpec to load the constant" so much as having RSpec look for it in a way that would trigger const_missing, which is what Rails uses for auto-loading.

(As an aside, this is one reason I dislike rails autoloading: there's a lot of uncertainty about whether or not a specific thing is loaded or not).

It can be confusing at times, though I honestly haven't run up against it too much. It certainly seems better than the alternative for large projects. In smaller projects/gems it clearly makes sense to just use manual requires, but when you've got a large app with tons of classes it can become a real pain to have to manually require each thing you need (though that might create some interesting design pressure...). Not sure where it plays in, but not caching the classes also makes it possible to change your classes without having to reboot your server when in development.

[xaviershay]

So turns out I don't actually use verify_doubled_constant_names option. That's not deliberate, I probably should. (Just grepped a few projects to discover this.) @myronmarston pretty sure that was a feature you added/suggested ;)

My second reaction was that the test environment should eager load everything, though this doesn't appear to be the rails default.

Let me try setting it on a few projects and then I'll have some opinions.

though that might create some interesting design pressure...

this times one thousand, but it's an academic point ;)

[mockdeep]

@xaviershay Yeah, not too sure what the implications of eager loading in test environment are. I don't see any performance differences, but I seem to remember some odd behavior around it with gems like simplecov, spork, and teaspoon. Maybe those have been resolved, though.

[myronmarston]

So turns out I don't actually use verify_doubled_constant_names option. That's not deliberate, I probably should. (Just grepped a few projects to discover this.) @myronmarston pretty sure that was a feature you added/suggested ;)

Yep, I was indeed the source of the feature :). Unfortunately, using that feature well is a bit tricky. You don't want to always turn it on, as doing so totally defeats the purpose of supporting the string form of instance doubles. It's a good option to have on on CI and when running your entire spec suite, though. Here's my usual habit for it:

• I set it in a file like spec/support/verify_doubled_constant_names.rb (or similar).
• I require this support file from spec/acceptance_spec_helper.rb, which is in turn required by my end-to-end acceptance specs. Since those specs are designed to run in a fully loaded environment, it makes sense to turn it on. I don't actually tend to use any sort of test doubles in my acceptance specs, but the fact that it gets turned on when they are loaded means it'll be enabled when I run the entire spec suite (since doing so will load the acceptance specs).
• Having the config in a stand-alone file means I can also do something like rspec spec/unit spec/integration -rsupport/verify_doubled_constant_names to toggle it on via a command line flag when I'm skipping acceptance specs but still want the option turned on.

[xaviershay]

Turns out I do use eager loading, just missed it the first time I looked. I have a file spec/load_rails.rb that looks like:

ENV["RAILS_ENV"] = 'test'

require 'coverage_helper'
require 'support/verify_doubled_constant_names'

require_relative '../config/environment'

MyApp::Application.eager_load!

ActiveRecord::Migration.maintain_test_schema!

I then can use it like rspec -r./spec/load_rails spec/unit whenever I want a full run. I have bin/test-unit script that does this for me.

I don't use spork or the like, but this works fine with simplecov.

[myronmarston]

That would certainly do the trick. Still, I think it's suboptimal for users to have to set that up. If they've configured verify_doubled_constant_names then they almost certainly don't want to get X is not a defined constant. Perhaps you misspelt it? for a constant that was spelled properly but hasn't been loaded by rails. I think it's reasonable for us to try to load it in that case. The main question is how and where we do it.

@JonRowe, could the verifying doubles callback you're working with be used in rspec-rails to try to force the constant to load if verify_doubled_constant_names is set?

[tomstuart]

Got bitten by this today. Is it likely to be fixed automatically in upcoming rspec-rails versions, or is there already an official way of working around the interaction between Rails autoloading and verify_doubled_constant_names?

[myronmarston]

@tomstuart: it depends on how you want to use verifying doubles. I'm not sure that rspec-rails can fix this in a way that pleases everyone.

If you want the verifying double to always get verified (which requires the constant to be loaded, but has the downside of adding the load time of the desired class and any of its dependencies), my suggestion is to not use the string form of instance_double and friends. For example, use instance_double(Project) instead of instance_double("Project"). Referencing the Project constant will cause rails to auto-load it, ensuring its loaded. As a bonus, you won't really need the verify_doubled_constant_names option since you'll get an undefined constant error from ruby if you mis-spell it.

OTOH, if you want to be able to use the verifying double in both modes (e.g. w/o the constant loaded and w/o verification vs w/ the constant loaded and w/ verification), then you'll have to use the string form (instance_double('Project')), at least for the specs for which you want to be runnable w/o the constant loaded. In that case, it's up to you to decide how you want to do things. You can forego the verify_doubled_constant_names config option entirely but that runs the risk of verifying doubles with mis-spelled constant names never getting verified, and it also means that if you remove the Project class your specs that double it won't notice. If you want to use the verify_doubled_constant_names option, you'll need to decide when it should be turned on, and ensure that all constants (or at least all doubled constants) are loaded when it is set. One approach would be to define a file like spec/support/ensure_all_verifying_doubles_verified.rb:

RSpec.configure do |c|
  c.mock_with :rspec do |mocks|
    mocks.verify_doubled_constant_names = true
  end
end

# load all doubled constants here, either by putting explicit requires
# or by looping over the appropriate directories and loading all ruby
# files in them.
require 'project'

Then you could configure things so this file is only loaded in particular situations (e.g. as part of your CI builds).

Would either of those options work for you? If not, we'll need more information about how you want to use verifying doubles to advise.

[JonRowe]

I think we should try to load the constant (when verify_doubled_constant_names is set) before raising that error message, this is on my list to tackle I just haven't gotten to it yet.

[tomstuart]

In an imaginary ideal world, what I want is for these features and their benefits to just all work as advertised in a Rails app. And a pony.

That would mean:

• I can use verifying doubles with strings, not constants, so I get the fabled "best of both worlds" behaviour: when the class hasn't been loaded I get a vanilla double, and when it has been loaded I get verification. This means I can run individual unit tests (spec_helper.rb) quickly in isolation without having to load the whole app, but when I run all the integration/acceptance tests as well (rails_helper.rb) I get failures if I've accidentally stubbed methods that don't exist. There is a smooth continuum here: if I run a subset of unit tests such that some tests happen to load classes that other tests reference with verifying doubles, I get a bit of checking.
• I can use verify_doubled_constant_names. This means I see failures if I accidentally make a typo in the strings I use to create verifying doubles, whenever that is possible to detect.

I understand the tension here, since it's hard to tell the difference between "this class doesn't exist" and "this class hasn't been autoloaded yet" in a Rails app. But at a minimum it would be helpful to have some documentation that says: we recommend you do such-and-such if you're using Rails. (There are a few candidates in this thread already, but those are just GitHub comments, not official advice.) If that recommendation is to have a special separate Rake task for preloading everything and running with verify_doubled_constant_names because autoloading and name verification don't mix, or even to not use name verification at all with Rails, that's absolutely fine. Just say so.

A more ideal solution in principle (although perhaps impossible in practice) would be for name verification to decide whether a particular constant name looks plausible for the current Rails app, i.e. does there exist a file whose name and path would cause Rails' autoloader to try to define this constant if we referenced it? That would preserve the benefits of "we don't have to load everything" but without the downside of "if we make a typo in a name string, we may literally never be told about it".

In any event, the eventual solution must account for the realities of Rails. Rails projects are not like plain Ruby projects. Rails applications cannot have all of their dependencies loaded with require 'project'. Most Rails devs don't even know how autoloading works, so they need specific documentation (if not automatic implementation) of "just require all your files"; Rails does not do this for you in the test environment.

[tomstuart]

To further clarify: the dream is to be told "you made a typo" without having to load the class, since in many cases the class can't be successfully loaded without doing a bunch of other setup (i.e. bootstrapping the Rails app). I accept that this may well not be possible.

[JonRowe]

Given how Rails conventions work, I'd like it to be possible to ask Rails "what is the file name convention for this constant" and then say "does that file exist in the current autoload paths" and treat that as a best guess for "does this exist"