Implicit subject renaming (subject without block) #767

Closed
avit opened this Issue Jan 4, 2013 · 6 comments

Comments

Projects
None yet
3 participants
Contributor

avit commented Jan 4, 2013

I think this is a minor regression since it was previously supported for renaming the implicit subject:

describe Filter do
  subject :filter

  it "is an instance of Filter" do
    filter.should be_a Filter
  end
end

As of 2.12.2 it breaks with "no block supplied". Should this continue to be supported?

Owner

myronmarston commented Jan 4, 2013

This is the first time I've ever seen anyone try that! @dchelimsky may know more, but I believe that that was never intended to work (and in fact, I'm surprised that it ever did)--if anything, it was an accident of the implementation, and not intended behavior. I wouldn't consider it a regression since it was never documented that you could call subject on your example group with a name but not a block.

I'd also say that subject was designed for one simple purpose: to enable one-liners like:

describe User do
  it { validates_presence_of :email }
end

In your example, I don't think subject :filter is any more clear than let(:filter) { Filter.new }.

Owner

dchelimsky commented Jan 4, 2013

I don't remember this being supported, but the git log can be the ultimate arbiter: if there is a spec for it somewhere, then it was supported and should continue to be - otherwise it just worked by accident.

@avit you could also use the recently added named subject if you want subject and filter to return the same object:

describe Filter do
  subject(:filter) { Filter.new }
  # ...
end
Contributor

avit commented Jan 4, 2013

Yes, named subject is the behaviour I originally had, like subject(:filter) { described_class.new }

The only point was for renaming to use a descriptive name instead of "subject". The block shouldn't be necessary: I would expect it to do what subject already does implicitly when it hasn't been explicitly defined.

Owner

dchelimsky commented Jan 4, 2013

I would expect it to do what subject already does implicitly when it hasn't been explicitly defined.

That behavior, implicitly using an instance of the described class, is at the instance level (i.e. when you reference subject in an example). At the class level (when you declare a subject), I don't think what you're describing was never intended to work.

Owner

myronmarston commented Jan 4, 2013

I don't remember this being supported, but the git log can be the ultimate arbiter: if there is a spec for it somewhere, then it was supported and should continue to be - otherwise it just worked by accident.

Looking through the logs, here's what I found:

  • In 2e20683#L1R183, @dchelimsky added support for named subjects. At this point, @avit's subject :filter thing worked, but I don't believe it was intentional. It only worked because the existing subject logic had a guard for the case that no block was passed, although, for the life of me, I can't figure out why you'd want want this...it allowed you to use subject (with no args or block) in an example group and have it return the proc that would be run to get the subject when subject is later used in an example.
  • In c83eaf9, a bug was fixed with named subjects, and it unintentionally broke what subject :filter because it delegates to let(name, &block) and let does not work without a block.

There's no spec for this behavior anywhere in the history, and I'm not inclined to add it--we already have many ways to do what @avit is trying to do:

describe Filter do
  alias filter subject
  alias_method :filter, :subject
  let(:filter) { subject }
  let(:filter) { described_class.new }
  let(:filter) { Filter.new }
end

subject :filter seems like a confusing way to alias the implicit subject to a new name, particularly given the fact that there are already so may ways to achieve this (including plane-old-ruby constructs like alias).

I'm going to close this, but feel free to continue the conversation :).

Contributor

avit commented Jan 4, 2013

Thanks! I was mostly wondering if this was intended or not, and I agree that alias (or similar) is much clearer anyway.

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