Rspec-core: let! is called after before hook inside RSpec.configure

[daniel-g]

$steps_order = []

RSpec.configure do |config|
  config.before(:each, :test => true) do
    $steps_order << :configure_before_hook
  end
end
describe 'steps order', :test do
  let!(:object) do
    $steps_order << :let!
  end

  before do
    $steps_order << :before_hook
  end

  it 'should execute let! before any before hook' do
    $steps_order << :step
    $steps_order.should == [:let!, :configure_before_hook, :before_hook, :step]
  end
end

And I got:

expected: [:let!, :configure_before_hook, :before_hook, :step]
     got: [:configure_before_hook, :let!, :before_hook, :step] (using ==)

[dchelimsky]

As expected because let! declares a before hook just like any other. What problem is this causing for you?

[dchelimsky]

FYI: http://rubydoc.info/gems/rspec-core/RSpec/Core/Let/ExampleGroupMethods#let%21-instance_method

[daniel-g]

I am trying to reindex all objects in solr which steps have a :search tag. So when I run reindex_all! any object has been created yet.

[myronmarston]

In my opinion, it's a bad idea to have an ordering dependency between before hooks declared in different places. Relying on RSpec's ordering of these hooks creates a very implicit (and brittle) dependency. It's much better to make the order dependency explicit. Two simple ways to do this:

• Declare all before hooks in one context; RSpec wil run these in the declared order, and the order is clear since they are all listed one after the other.
• Move the logic that your before hook depends on into a helper method (e.g. ensure_objects_created or whatever) that no-ops if run again; then call it explicitly from your before hook.

[daniel-g]

Yea, I had the second option in my code... I just wanted to refactor all those helper method calls. I thought before filters were separated from let! methods. Maybe run all let! then before blocks... I will leave it as it is but I think let!s should run before before hooks in case the configuration need those objects like my case is.

[dchelimsky]

@daniel-g the additional complexity to support that is a deal breaker for me. I'd sooner remove let! from rspec (there are other gems that do similar things, like rspec-set) than add a run-order guarantee across let!s and befores across scopes.

[dchelimsky]

I should add that it's not simply a matter of "it's hard". I don't see how it would be impossible to guarantee the load order in all cases because it would depend on things outside rspec's control such that you'd be likely to run into problems where the inclusion of a gem that exploited the system would break your expected behavior.

[diophore]

This caught me by surprise. Intuitively, I expected a nested let! to be executed before a before(:each) block in an outer scope. Something like this arbitrary example:

describe Job do
  let!(:task) { FactoryGirl.create(:task, job: subject) }

  context 'when previously executed' do
    before(:each) do
      subject.run
    end

    context 'with two tasks' do
      let!(:task_two) { FactoryGirl.create(:task, job: subject) }

      it 'should not re-run the second task' do
        expect(task_two).to_not receive(:run)
        subject.run
      end
    end
  end
end

Of course, the before hook in the outer scope was executed before task_two was created. I know there are other ways to organize this, but this seemed the cleanest, most intuitive way to me. Given the options available, I've chosen to move let!(:task_two) to the outer scope, even though only the tests in the inner scope require it.

[myronmarston]

let! simply defines a let and defines a before hook to call it. So the ordering is exactly the same as any if you had put a before hook there.

For your example, I don't see why you need before/let!/etc at all..why not flatten it?

describe Job, "when previously executed, with two tasks" do
  it "should not re-run the second task" do
    task = FactoryGirl.create(:task, job: subject)
    subject.run

    task_two = FactoryGirl.create(:task, job: subject)
    expect(task_two).to_not receive(:run)
    subject.run
  end
end

It's less code, easier to understand, and the ordering of events is absolutely explicit.

[diophore]

The reason I wouldn't flatten it is that would introduce a lot of code duplication that I have removed through the use of context and let/let!. That example is only loosely modeled after my actual tests, which have a couple more layers of nested contexts and a whole lot more tests at every level. If it were really just one test, then sure, but there are a lot of tests and of course I want to reduce the noise and make the intent of each test more clear.

I thought it might be useful to know that another person out there had the mental model that all let! blocks would be executed before all before hooks. I also thought it might help to see another example.

[JonRowe]

@diophore as @myronmarston mentions this is an unavoidable consequence of how let! is implemented, we have no interest in changing this behaviour, I hope you better understand how it works now so you can avoid this issue in future.

[vergenzt]

I thought it might be useful to know that another person out there had the mental model that all let! blocks would be executed before all before hooks. I also thought it might help to see another example.

Just adding another voice to the mix; it took me a little while to figure out why my spec setup was failing. I had an identical structure to @diophore's example earlier where (1) the method under test runs in a before block, and (2) I was trying to let! an extra associated object in a narrower context and specify behavior with that extra association. (However I'm solving it by removing the before block and just running the method from each test.)

IMHO @diophore's example seems to be the most intuitive way to structure such an expectation. It surprised me that let! calls _didn't_ execute as a group before the other before blocks.

Having said that, now that I understand that let! really just adds a before block behind the scenes, the complicatedness of implementing something like that is much clearer. Maybe the answer is just a documentation note somewhere?

[myronmarston]

Maybe the answer is just a documentation note somewhere?

We'd welcome a PR improving our docs!

[vergenzt]

FWIW @diophore another pretty clear workaround I've found is to essentially "declare" the let! blocks you need and override them later. The invocation order is determined by the _first_ place the let! block is declared, not the last.

describe Job do
  let!(:task) { FactoryGirl.create(:task, job: subject) }
  let!(:task_two) { nil }

  context 'when previously executed' do
    before(:each) do
      subject.run
    end

    context 'with two tasks' do
      let(:task_two) { FactoryGirl.create(:task, job: subject) }

      it 'should not re-run the second task' do
        expect(task_two).to_not receive(:run)
        subject.run
      end
    end
  end
end

[jrpolidario]

If you are willing to not use before(:each) but would like to retain its DRYness ability for each example, I resolved to using something like the following (if it will help anyone):

describe Job do
  let!(:task) { FactoryGirl.create(:task, job: subject) }

  context 'when previously executed' do
    # rename this to whatever you want that describes this "supposedly" before(:each)
    let(:perform_subject_run) { -> {
      subject.run
    }}

    context 'with two tasks' do
      let!(:task_two) { FactoryGirl.create(:task, job: subject) }

      it 'should not re-run the second task' do
        perform_subject_run.call
        expect(task_two).to_not receive(:run)
        subject.run
      end
    end
  end
end

I intentionally declared perform_subject_run as a lambda block because rspec seems to cache the return value of the let(:block) once it is already been called, which will not work when we need to call perform_subject_run at least twice, if ever a need may arise.

• I would say that this even makes the specific examples very readable as there are no hidden before(:each), and that you won't have to scroll above each time, just to take note of any before(:each) block that has been declared, in order to understand the sequence of the spec run. This saves time for me at least for a big spec file, when I'm reviewing and understanding an old spec code that I have written as I can immediately see what's happening.
• This also enables you to reorder the perform_subject_run inside the it example block. As there are certain cases that you might want to "do" something first before running perform_subject_run.
• The compromise however though is that you'll need to write this perform_subject_run for each example, which in a way kind of defeats the purpose of before(:each) itself.