Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

inconsistent behavior of subject when using its #819

Closed
soupmatt opened this Issue · 4 comments

3 participants

@soupmatt

When upgrading from rspec 2.12 to 2.13, I came across a change in the behavior of its blocks. It seems that the implied subject is different from accessing the subject by name. I see this was discussed and changed in #768 and #771, and that it is a deep and non-trivial topic, so I will leave judgement on what the correct behavior should be to those with a better understanding than mine.

The following specs pass under 2.12.2 and fail under 2.13.0.

Here's a simple example that demonstrates the behavior.

describe "weird its behavior" do
  subject { {:foo => :bar, :bar => :baz} }

  its([:foo]) { should eq(:bar) }
  its([:foo]) { subject.should eq(:bar) } #fails under 2.13.0
end

Here's a example that demonstrates the use case for the 2.12.2 behavior. This is taken from actual specs in a codebase I work on at my job.

require 'json'
require 'ostruct'

describe "a JSON string" do
  let(:expected_body) { {"a" => "b", "c" => [1,2,3]} }
  subject { OpenStruct.new(:headers => [:a, :b], :body => JSON.dump(expected_body)) }

  its(:headers) { should eq([:a, :b]) }
  its(:body) { should be_a String }
  its(:body) { JSON.parse(subject).should eq(expected_body) } #fails under 2.13.0
end
@cupakromer
Collaborator

Looks related to d177f54. Seems fixing how subject gets referenced by the outer let and before blocks changed being able to reference the explicit subject inside the example.

@myronmarston
Owner

Indeed, @cupakromer is right. @exviva reported some apparent bugs with cases like:

describe Person do
  before { subject.age = 19 }
  its(:can_vote?) { should be_true }
end

In RSpec 2.12, and all previous version, this failed, because in the only example subject was the return value of Person.new.can_vote?, so the before block was actually calling Person.new.can_vote?.age = 19.

However, this is very subtle, and the original intention of its was to allow you to use expressions like its(:attribute) { should match_some_matcher } -- in other words, changing the value of subject was an accident in its implementation and had nothing to do with how its was intended to be used.

So we in 2.13 we changed it (in d177f54, as @cupakromer pointed out), so that subject continues to refers to what subject already referred to, and its expressions continue to work properly.

IMO, its(:attribute) { should match_some_matcher } reads pretty nicely, which is why its was added to rspec in the first place, but once you start explicitly referencing subject in there, (as in your example: its(:body) { JSON.parse(subject).should eq(expected_body) }), it is completely unreadable.

I think your spec would be much more readable, simple, clear and direct doing something like:

describe "a JSON string" do
  let(:expected_body) { {"a" => "b", "c" => [1,2,3]} }
  subject { OpenStruct.new(:headers => [:a, :b], :body => JSON.dump(expected_body)) }

  it 'allows the body to be be parsed as json' do
    expect(JSON.parse(subject.body)).to eq(expected_body)
  end
end

(That said, I don't think I'd advocate the use of subject here at all--it's not an intention revealing name).

its will be moving into an external gem in RSpec 3.0 (explanation), so I'm not planning to change this behavior back.

@soupmatt

Thanks for the explanation, @myronmarston. I have learned something about writing better specs instead of just being lazy about writing specs.

@myronmarston
Owner

@soupmatt -- my general advice is to stick to the simplest constructs you can; most of my specs are just describe, it and some let declarations. I rarely use subject, almost never use its, and almost never use it one-liners. The more advanced constructs are fine when they are needed, but it's best not to reach for them as the main way you write specs.

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.