Skip to content

Instantly share code, notes, and snippets.

@myronmarston
Created March 17, 2012 21:30
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save myronmarston/2065445 to your computer and use it in GitHub Desktop.
Save myronmarston/2065445 to your computer and use it in GitHub Desktop.
Proof of concept for yield_value rspec matcher
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
@zdennis
Copy link

zdennis commented Mar 18, 2012

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:

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

The API needs to support saying that :

  • when #no_args is called it yields 3
  • when #one_arg is called with 2 that is yields 3
  • when #many_args is called with 3 and 4 that it yields 4, and 5.

I'll follow up with code tomorrow if you or someone hasn't beaten me to it.

@myronmarston
Copy link
Author

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)?

@dchelimsky
Copy link

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).

@myronmarston
Copy link
Author

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?

@dchelimsky
Copy link

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.

@myronmarston
Copy link
Author

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.

@dchelimsky
Copy link

Absolutely agree a family of related matchers is a better call here.

@zdennis
Copy link

zdennis commented Mar 18, 2012

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?

@dchelimsky
Copy link

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.

@zdennis
Copy link

zdennis commented Mar 18, 2012

@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.

@mattwynne
Copy link

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)

@dchelimsky
Copy link

@mattwynne nice solution! One less matcher. We should do the same in rspec-mocks:

object.stub(:method).and_return(x).then(y).then(z)

@myronmarston
Copy link
Author

@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.

@myronmarston
Copy link
Author

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".

@myronmarston
Copy link
Author

I'll start taking a stab at this, and open a pull request with what I come up with.

@myronmarston
Copy link
Author

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.

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