-
-
Save myronmarston/2065445 to your computer and use it in GitHub Desktop.
RSpec::Matchers.define :yield_value do |expected_yielded_value| | |
match do |block| | |
yielded_value = nil | |
block.call(lambda { |arg| yielded_value = arg }) | |
yielded_value == expected_yielded_value | |
end | |
end | |
describe "yield_value matcher" do | |
it 'passes a valid positive expectation' do | |
expect { |b| 3.tap(&b) }.to yield_value(3) | |
end | |
it 'fails an invalid positive expectation' do | |
expect { | |
expect { |b| 3.tap(&b) }.to yield_value(2) | |
}.to raise_error(RSpec::Expectations::ExpectationNotMetError) | |
end | |
end |
With a slight tweak to allow a variable number of args it works just fine:
RSpec::Matchers.define :yield_value do |*expected_yielded_values|
match do |block|
yielded_values = nil
block.call(lambda { |*args| yielded_values = args })
yielded_values == expected_yielded_values
end
end
class Foo
def no_args ; yield 3 ; end
def one_arg(a); yield a+1 ; end
def many_args(a,b) ; yield a+1, b+1 ; end
end
describe "yield_value matcher" do
alias yield_values yield_value
def fail
raise_error(RSpec::Expectations::ExpectationNotMetError)
end
it 'works with no args' do
expect { |b| Foo.new.no_args(&b) }.to yield_value(3)
expect {
expect { |b| Foo.new.no_args(&b) }.to yield_value(2)
}.to fail
end
it 'works with one args' do
expect { |b| Foo.new.one_arg(2, &b) }.to yield_value(3)
expect {
expect { |b| Foo.new.one_arg(2, &b) }.to yield_value(4)
}.to fail
end
it 'works with many args' do
expect { |b| Foo.new.many_args(3, 4, &b) }.to yield_values(4, 5)
expect {
expect { |b| Foo.new.many_args(3, 4, &b) }.to yield_values(5, 6)
}.to fail
end
end
The bigger question I have is how do we let people specify that a method yields multiple arguments vs. a method yielding multiple times (such as for an iterator)?
For multiple calls, we can use multiple expectations:
class Foo
def bar
@bar ||= 0
yield @bar += 1
end
end
foo = Foo.new
expect { |b| foo.bar(&b) }.to yield_value(1)
expect { |b| foo.bar(&b) }.to yield_value(2)
I think this is simpler to understand than trying to come up with an ordering scheme like stubs have (which is confusing both internally and externally).
For multiple calls, we can use multiple expectations.
Agreed...I have a hard time imagining it being any other way.
What I've sketched above works pretty well for methods that yield once, and yield one or more values as part of that. There are a few more cases this doesn't work so well for.
Methods that yield multiple times
Think about #each
. This won't work:
expect { |b| [1, 2, 3].each(&b) }.to yield_values(1, 2, 3)
...be cause to yield_values(1, 2, 3)
means "yield once, with the arguments 1, 2 and 3". I can't think of a good way to make this yield_values
matcher work for cases like these. One option is a sister matcher, yield_in_sequence
:
expect { |b| [1, 2, 3].each(&b) }.to yield_in_sequence(1, 2, 3)
For the case where each yield yields more than one argument, this would work OK:
hash = { a: 5, b: 10 }
expect { |b| hash.each(&b) }.to yield_in_sequence([:a, 5], [:b, 10])
It's a little weird for using on hashes on 1.8.7 (since there's no guaranteed ordering). I guess you could do this, though:
hash = { a: 5, b: 10 }
expect { |b| hash.each(&b) }.to yield_in_sequence { |sequence| sequence.should =~ [[:a, 5], [:b, 10]] }
That's not too bad.
Methods that yields control but no arguments
Again, maybe a sister matcher would be the answer here?
Class Foo
def bar
yield
end
end
expect { |b| Foo.new.bar(&b) }.to yield_control
That's the best I can come up with for these cases. Are there any other weird cases we haven't listed yet?
I like yield_in_sequence
or yield_consecutive_values
or some such. Not sure about yield_control
- is that something you've ever wanted? Or are you just thinking in terms of being thorough.
I like yield_in_sequence or yield_consecutive_values or some such.
Yeah, yield_consecutive_values
is nice, too. yield_in_sequence
is a bit shorter and equally descriptive, I think, so unless anyone has a preference the other way I'd probably prefer it.
Not sure about yield_control - is that something you've ever wanted? Or are you just thinking in terms of being thorough.
Mostly being thorough. I've written plenty of methods that yield but don't yield any arguments. Even if yield_control
isn't the right name for the matcher, it'd be good to have some way to specify that a method yields but doesn't yield arguments (if nothing else, it would be odd to have matchers fo the other cases but not this one). Neither yield_value
nor yield_in_sequence
will work well for this case.
I was hoping to come up with one matcher that works for all of these cases, but I think multiple related matchers is much better than a single matcher with a confusing API--and I can't figure out how to keep a single matcher for all these cases w/o a confusing API.
Absolutely agree a family of related matchers is a better call here.
I went out for a run, came back, started to type up a yield_successive_values
and then saw the conversation had continued. :)
I like the idea of a family of related matchers since it avoids the situation where using one API would introduce an ambiguity factor based on the three different situations:
- yield without value
- yield with value(s)
- yielding successively
I'm not a huge fan of yield_control
. The term control is not consistent with the language already being shared throughout the other matchers. While it isn't shorter, I find yield_without_value
(or a variant) to be a better fit even though its a little longer. If I am using yield_value
and come across a situation where I expect no values, moving to yield_control
is a bit of a mental leap (and vise versa), and not one that is natural. It's seems like a dichotomy you have to memorize and I think we can avoid it.
Here's a suggestion for the matcher family if we keep value:
yield_without_value or yield_with_no_value
yield_value
yield_values
yield_successive_values or yield_values_sequence
Now, I want to try to make an argument against the term value. Here's my attempt... I never think of block arguments as values; rather, I think of them as block arguments. While yield_value
sounds okay I don't think it's consistent with the how we think of blocks and their arguments. It may be more intuitive if we replace value with arg.
Here's an update suggestion for the matcher family:
yield_without_args or yield_with_no_args
yield_arg
yield_args
yield_successive_args or yield_args_in_sequence
The above arg could be replaced with argument but I feel like the term arg is a part of our every day vernacular and offering it in place of argument is shorter and does not lose meaning.
Thoughts?
I like arg
v value
. Not sure we need both yield_arg
and yield_args
. I think yield_args(with_one_arg)
is perfectly readable.
re: yield_without_args
v yield_control
, these mean two different things: "yield without args" v "yield control, but I don't care if there are args or not". Thinking this through, I think we should have both, since the latter supports a looser coupling that might be appropriate in some cases.
@dchelimsky: I see your point about yield_arg/yield_args and agree. Same goes for yield_without_args and yield_control. I hadn't thought of it that way.
I like where this is heading. For multiple yields (e.g. Array#each), what about being able to chain calls to construct more matchers, like this:
expect { |b| [1, 2, 3].each(&b) }.to yield_args(1).then(2).then(3)
@mattwynne nice solution! One less matcher. We should do the same in rspec-mocks:
object.stub(:method).and_return(x).then(y).then(z)
@mattwynne: I like that as a solution for short arrays (of one or two items), but imagine doing that on an array of 100 items. It doesn't scale to larger arrays. Or consider trying to write a helper method that accepts an array as an argument, and after doing a bunch of stuff, needs to make an assertion that all the items in the array were yielded. With that approach, the helper method would have to use a messy #inject:
first_arg = array.shift
matcher = array.inject(yield_args(first_arg)) do |matcher, arg|
matcher.then(arg)
end
expect { |b| my_object_that_wraps_the_array.each(&b) }.to matcher
This would be much, much cleaner with a dedicated matcher for the multi-yield case.
BTW, on the topic of the naming of the matchers, I think I like yield_with_args
better than yield_args
. It has a nice symmetry to yield_without_args
, and it's more closely aligned with how I talk about this stuff: "3.tap yields with 3 as the argument".
I'll start taking a stab at this, and open a pull request with what I come up with.
FWIW, I've pushed what I've got so far into a branch:
https://github.com/rspec/rspec-expectations/tree/yield_matchers
There's more I plan to do before submitting a pull request, but I figured I'd push something to back it up and so people who care to can take a look.
I don't think this will work because we're only ensuring that value is yielded. It needs to ensure that a particular method call yields the proper values (with or without arguments). Here's a quick sample set:
The API needs to support saying that :
I'll follow up with code tomorrow if you or someone hasn't beaten me to it.