Skip to content

Regression: `:tag => nil` works in 2.6, but not 2.9 #611

Closed
fj opened this Issue May 3, 2012 · 3 comments

3 participants

@fj
fj commented May 3, 2012

Summary: Previously, having something like:

config.before :each, :tag => nil ...

would cause examples that were not tagged with :tag (or which were explicitly tagged with :tag => nil) to get the behavior in question. In 2.9 this is no longer the case.


It is often helpful to trigger RSpec behavior like before blocks off of a nil tag, so that examples that are not tagged get the behavior. This is nice because you don't have to repeat yourself across every spec file and you can just tag the exceptions to the rule.

Unfortunately, this behavior broke when upgrading to 2.9. Here's a simple test case:

# spec/spec_helper.rb
require 'rspec'

RSpec.configure do |c|
  c.before :each, :set_var => nil do
    @one_two_three = 123
  end
end

# spec/sample_spec.rb
require 'spec_helper'

describe "examples without the :set_var tag" do
  it "should set the field" do
    @one_two_three.should == 123
  end
end

describe "examples with the set_var tag set", :set_var => nil do
  it "should set the field" do
    @one_two_three.should == 123
  end
end

Here's the test run. Notice that the first spec passes in 2.6 but not in 2.9. The second spec, where the tag is set explicitly, passes in both cases.

# using RSpec 2.6
[~/projects/rspec-test] Ψ bundle exec rspec spec
..

Finished in 0.00098 seconds
2 examples, 0 failures

# using RSpec 2.9

[~/projects/rspec-test] Ψ bundle exec rspec spec
F.

Failures:

  1) examples without the :set_var tag should set the field
     Failure/Error: @one_two_three.should == 123
       expected: 123
            got: nil (using ==)
     # ./spec/sample_spec.rb:5:in `block (2 levels) in <top (required)>'

Finished in 0.0009 seconds
2 examples, 1 failure

Failed examples:

rspec ./spec/sample_spec.rb:4 # examples without the :set_var tag should set the field
@myronmarston
RSpec member

For the background on the changes that caused this, see #556.

I believe you can achieve the desired behavior like so:

RSpec.configure do |c|
  c.before :each do
    @one_two_three = 123 unless example.metadata[:set_var]
  end
end

I think this code is simpler and clearer, too.

As for the regression...:tag => nil is a bit of an edge case and I don't think rspec ever documented how it should work. It happened to work with the old implementation but I'm not sure if that was intended or not.

@dchelimsky -- can you speak to whether or not the original behavior was intended and you'd consider this a regression?

@dchelimsky
RSpec member

@myronmarston thanks for doing the research on this.

@fj I don't consider this a regression (afaik it was only working by accident), and I also think that using the assignment of nil to a key to mean "don't do something" is a bit confusing. I appreciate the desired outcome, but I'd recommend using a key that tells the story when it's value is true e.g.

RSpec.configure do |c|
  c.treat_symbols_as_metadata_keys_with_true_values = true
  c.before :each do
    @one_two_three = 123 unless example.metadata[:no_setup]
  end
end

describe "examples without :no_setup" do
  it "should set the field" do
    defined?(@one_two_three).should be_true
  end
end

describe "examples with", :no_setup do
  it "should set the field" do
    defined?(@one_two_three).should be_false
  end
end

I also opened #612 to suggest something more explicit to config.before. Feel free to comment there.

@dchelimsky dchelimsky closed this May 3, 2012
@fj
fj commented May 3, 2012

Gotcha. The main disadvantage of the suggested approach is that you have to enter the block every time, but I'm assuming that additional overhead is minimal. And in my case the suggestion will work. Thanks for the advice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.