Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arguments passed to its #306

Closed
wants to merge 1 commit into from
Closed

Arguments passed to its #306

wants to merge 1 commit into from

Conversation

knzai
Copy link

@knzai knzai commented Feb 9, 2011

Allow passing of arguments into 'its'
its(:at, 1) { should == 'b' }

cukes and specs included

@dchelimsky
Copy link
Contributor

I think it's time to extract its out to its own library.

I can see why this is a logical extension of its related methods, but I don't find it readable or intuitive, or core to what rspec is about. I understand that a lot of people use explicit subjects and one liners using it { should ... } and its(:attr), but I find that they don't provide much value and, because they are so simple to write, they actually exacerbate the problem that RSpec set out to help solve: TDD is about specifying behavior, not about structure or implementation.

I'm going to leave this open as I think it does make sense in the context of the other its methods, but I'm not going to merge it just yet. I'll write a note to the rspec-users list and see if we can get some other thoughts and opinions on this issue.

@rubyredrick
Copy link

I agree with David.

Personally I rarely use one liners like that, and I didn't even know about its until now.

its(:length) {should == 3}

seems somewhat readable and marginally useful.

but
its(:at, 1) {should == 'b'}

just doesn't seem to be either to me.

But that's just one man's opinion.

@knzai
Copy link
Author

knzai commented Feb 10, 2011

I actually agree, even though I submitted the patch. I didn't even expect that much response, and I'm not sure it's super readable. It just was the natural extension of the usage that grows out of subject and its.

It was also driven by trying to come up with some more functional option for testing methods than the ivar/global shuffle that comes out of before, let, and subject.

@knzai
Copy link
Author

knzai commented Feb 10, 2011

And it didn't seem much worse than its([:key]) or its('foo.bar') ;).

@dchelimsky
Copy link
Contributor

@timocratic - no, it didn't seem much worse than either of those, but both of those made my stomach a little ill. I accepted them because they seemed to align w/ the intent of its, but this request just compounded on top of that sense that this all leads down a rabbit hole, and it's the wrong rabbit.

@knzai
Copy link
Author

knzai commented Feb 10, 2011

I agree. Maybe it was time for the straw that broke the camels back. Maybe it is time for an abstracted out its/calling functionally lib. Sort of shoulda or the subject.

@leehambley
Copy link

I have no idea what
its(:at, 1) {should == 'b'}
is supposed to do… based on those grounds alone (does anyone know, without thinking really hard?) it shouldn't be allowed. The on-liners from shoulda/etc are lovely, but they are a little close to implementation, not specification. This is obfuscated beyond belief.

@knzai
Copy link
Author

knzai commented Feb 24, 2011

I agree it's not a great syntax, it is more exploratory of where the logical limit of its([:key]) is as useful. And if you have a 50 tests that fit in one page full of it{}, its(){} you suddenly want to handle the method call case better.

Maybe some of the magic its could be abstracted into something else with even more magic, that would allow things like it.method_name(5) { should}. or some other method for fp style testing - I've experimented with setting subject to a lambda so I could subject.call(), and that wasn't any better, but solved a similar use case.

@dchelimsky
Copy link
Contributor

I'm closing this as its definitely something I don't want in rspec-core, but feel free to keep the conversation going about extracting its to a separate gem with additional one-line support in it.

@dchelimsky dchelimsky closed this Apr 17, 2011
@zubin
Copy link
Contributor

zubin commented Jun 27, 2012

I found this thread when looking for this functionality. I tend to agree: in many cases it won't make a lot of sense, however my use case reads quite well:

describe '#price_in(unit)' do
  context "no price" do
    its(:price_in, :grams) { should be_nil }
  end
end

@andrewhavens
Copy link

I also found this thread while searching for this functionality. My current implementation is getting a bit wordy. In my case it reads better to be able to pass an argument:

describe '#conditions_apply_to?' do
  let(:product){ Fabricate :product }
  context 'when combining with other conditions' do
    before { subject.conditions_combination_operator = 'and' }
    it 'returns false' do
      subject.conditions_apply_to?(product).should be_false
    end
  end
end

versus...

describe '#conditions_apply_to?' do
  let(:product){ Fabricate :product }
  context 'when combining with other conditions' do
    before { subject.conditions_combination_operator = 'and' }
    its(:conditions_apply_to?, product){ should be_false }
  end
end

Has this feature ever been implemented? Or have you thought of a better practice that you would recommend as an alternative to people searching for this feature?

@myronmarston
Copy link
Member

Has this feature ever been implemented?

No. its is being moved into an external gem for RSpec 3:

http://myronmars.to/n/dev-blog/2013/07/the-plan-for-rspec-3#core__will_be_moved_into_an_external_gem

Whoever maintains that may choose to implement something like this.

Or have you thought of a better practice that you would recommend as an alternative to people searching for this feature?

I recommend calling the method with arguments:

it '<describe your behavior>' do
  subject.conditions_combination_operator = 'and'
  expect(subject.conditions_apply_to?(product)).to be_false
end

@alindeman
Copy link
Contributor

I agree. Already its takes code away from how it is normally used, and this takes it even a step farther.

As much as is possible, invoking your code in a test should reflect how it is used in the "real world"

@zubin
Copy link
Contributor

zubin commented Jul 23, 2013

Fair enough. Implementing this could get ridiculous if, for example, the method took a block argument.

its(:purpose, { do_something_funky }) { should be_clear } # FAIL

@andrewhavens
Copy link

Thanks @myronmarston You helped me realize that my desire for an its that supported arguments was a symptom of my inability to come up with a good description. I think I was getting too granular with contexts. I like this better:

describe '#conditions_apply_to?' do
  let(:product){ Fabricate :product }
  it 'returns false when combining with other conditions' do
    subject.conditions_combination_operator = 'and'
    expect(subject.conditions_apply_to?(product)).to be_false
  end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants