Feature for specific argument constraints (ex/ double.stub(:foo).with_fourth_argument(...))

[mikeabiezzi]

I wanted to propose a new feature for rspec that allows an argument constraint style for a specific argument.

So, instead of:

it "verifies one part" do
  ...
  x.should_receive(:foo).with(value, anything, anything, anything, anything)
  ...
end

it "verifies the other part" do
  ...
  x.should_receive(:foo).with(anything, value, anything, anything, anything)
  ...
end

it "verifies yet some other part" do
  ...
  x.should_receive(:foo).with(anything, anything, value, anything, anything)
  ...
end

I'd like it to be:

it "verifies one part" do
  ...
  x.should_receive(:foo).with_first_argument(value)
  ...
end

it "verifies the other part" do
  ...
  x.should_receive(:foo).with_second_argument(value)
  ...
end

it "verifies yet some other part" do
  ...
  x.should_receive(:foo).with_third_argument(value)
  ...
end

Some of the benefits

• It's easier to read
• Tests won't need to be updated if you add additional parameters to a method. With the current style of using anything, if I were to add a parameter to a method, I would then have to update all my with(...) constraints to include another anything argument. This feature eliminates the need to rework the existing tests.

This pull request includes a working implementation that's tested, but I don't think it's all the way there. Before putting more time into the implementation, I first wanted to get some feedback from the rspec team to see if you would accept a feature like this. If the team does like it, I would love to get it to the point that meets the rspec team's standards, and also would love to hear suggestions regarding any implementation ideas.

Thanks!
Mike

[mikeabiezzi]

If the original method is a stub, then the only way to figure out the number of arguments is to look at all of the stubs and take the max argument count. It doesn't feel like the cleanest solution, but there's really no other way to determine the number of arguments.

[myronmarston]

Thanks for the PR. This can also be solved by passing an implementation block:

expect(x).to receive(:bar) do |*args|
  expect(args[4]).to eq(value)
end

Any reason that doesn't meet your needs?

[mikeabiezzi]

I didn't think about that syntax. Thanks for pointing that out! And thanks for the immediate reply!

It solves the problem of having to update tests, but it's a bit more verbose compared to x.should_receive(:bar).with_fifth_argument(value). I guess I can go either way.

I'd like to hear your thoughts on the negatives of having the with_x_argument code/syntax? I realize I may be bias towards it since I've been thinking about it for a while :)

[myronmarston]

I'd like to hear your thoughts on the negatives of having the with_x_argument code/syntax? I realize I may be bias towards it since I've been thinking about it for a while :)

I'm on the fence about it. Things that concern me are:

• It's a bit arbitrary that we have with_ninth_argument but not with_tenth_argument. Really, it's arbitrary at whatever cutoff we provide. Users may complain and I really don't want to go there.
• It increases the fluent interface by 9 new methods, which feels like bloat to me.
• I'm of the opinion that if your method takes more than a few arguments (say 3), you're better off passing a single args hash (or object), since connascence of name is better than connascence of position. If you're finding it painful to update the method signature, then I think your tests are providing useful design feedback that a feature like with_nth_argument mutes. A primary purpose of mocking (in my mind) is to surface design feedback. It seems counterproductive to that purpose to provide a feature that lessens the pain from what I see to be a design issue.

[mikeabiezzi]

All very good arguments that are easy to agree with. Thanks for taking the time to share your thoughts!

[xaviershay]

FWIW I agree with @myronmarston on this one. Thanks for the suggestion though!

[JonRowe]

I'd have been open to a with_arg(3, value) type extension to the fluent interface, but I'd also argue that you should be caring about all your arguments, your comment about adding an extra arg breaking your tests, well that's correct, you've changed the method signature in a breaking fashion.

[mikeabiezzi]

@JonRowe, the with_arg syntax would be a better way to do it for sure. It eliminates the arbitrary limit of how far you can snipe an argument.

I do care about all of the parameters. It's just that there are some scenarios in which I prefer to address each parameter in a separate test.

As for the tests breaking after adding a parameter, @myronmarston's argument was very convincing. If the code in your tests are hard to change, then the implementation in your actual code will also be hard to change, and the tests are telling you that about your design. Also, I've always limited myself to 5 arguments in a method, and I probably should limit myself to less than that. Limiting arguments is good practice, and in doing so, it further makes the proposed feature unnecessary. The last thing that I would want is a feature that promotes poor design, and I think this feature would so just that.

This conversation reinforced a lot of good design practices with me. Thanks everyone for the feedback!

[JonRowe]

I do care about all of the parameters. It's just that there are some scenarios in which I prefer to address each parameter in a separate test.

That's generally an indication of a code smell; Test pain is a feedback mechanism to push your design :)

[mikeabiezzi]

That's generally an indication of a code smell; Test pain is a feedback mechanism to push your design :)

Sometimes the method that you're calling and passing arguments to is a third party's API, and you don't have control of the design.