Do not modify `subject` when defining an `its` example #771

Merged
merged 1 commit into from Jan 10, 2013

Conversation

Projects
None yet
2 participants
Contributor

exviva commented Jan 9, 2013

See the discussion at #768.

@myronmarston I've added some documentation and renamed the internal subject to __its_subject to avoid a potential (however unlikely) name conflict.

I'm also overriding the methods in-line, to keep the usages and definitions of __its_subject as close to each other as possible.

Let me know what you think.

@myronmarston myronmarston and 1 other commented on an outdated diff Jan 9, 2013

spec/rspec/core/memoized_helpers_spec.rb
@@ -322,6 +322,18 @@ def false_if_first_time
end
its(:false_if_first_time) { should be(false) }
end
+
+ describe 'accessing `subject` in `before` and `let`' do
+ subject { 'my subject' }
+ before { @subject_in_before = subject }
+ let(:subject_in_let) { subject }
+
+ # These examples read weirdly, because we're actually
+ # specifying the behaviour of `its` itself
+ its(:length) { expect(@subject_in_before.length).to eq(10) }
+ its(:length) { expect(subject_in_let.length).to eq(10) }
@myronmarston

myronmarston Jan 9, 2013

Owner

I think these two blocks would more clearly demonstrate the behavior we're after with expect(<subject_reference>).to eq("my subject") -- that clearly shows that the subject in before and let is the original, outer subject. The length is an indirect way to test that. Thoughts?

@exviva

exviva Jan 9, 2013

Contributor

doh, makes sense...done.

Owner

myronmarston commented Jan 9, 2013

Looks great!

@myronmarston myronmarston added a commit that referenced this pull request Jan 10, 2013

@myronmarston myronmarston Merge pull request #771 from exviva/its_subject
Do not modify `subject` when defining an `its` example
978f429

@myronmarston myronmarston merged commit 978f429 into rspec:master Jan 10, 2013

1 check failed

default The Travis build failed
Details
Owner

myronmarston commented Jan 10, 2013

Thanks, @exviva -- you've done a fantastic job ferreting out (and solving) some weird edge cases with its and subject!

Contributor

exviva commented Jan 11, 2013

Glad it's merged, thanks for the guidance. I'll give it a spin and hope it gets released soon.

exviva deleted the exviva:its_subject branch Jan 11, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment