Implemented 'to have a call satisfying <object|function>'.

[papandreou]

Spinoff from #29.

// cc @sunesimonsen @gertsonderby @alexjeffburke @bruderstein

[papandreou]

Some of the error messages can be improved, so consider this a WIP for now. I'm pretty sure the semantics are ok.

[papandreou]

The exhaustively mode depends on this: unexpectedjs/unexpected#258

[papandreou]

Show of hands: Should

expect(spy, 'to have a call satisfying', [ 123 ]);

be shorthand for

expect(spy, 'to have a call satisfying', { args: [ 123 ] });

?

[bruderstein]

👍 for expect(spy, 'to have a call satisfying', [ 123 ]); as a shortcut.

Ideally this same shortcut should work for 'to have calls satisfying' too.

expect(spy, 'to have calls satisfying', [
   [ 123 ],
   { args: {1: 'foo' } },
   { args: [ 234, 'bar' ] }
 ]);

[sunesimonsen]

I like shortcuts as long as they are not ambiguous. This one seems fair.

[alexjeffburke]

I haven't read the rest of the API to know how well it composed. Agree with @sunesimonsen if it's not ambiguous. I'd be less of a fan if the asserting multiple calls is an array of arrays.. I write something similar on the other recent issue, not sure where that landed though :)

[alexjeffburke]

@bruderstein - just read your comment. I'm advocating keeping the multiple and single call assertions distinct enough they can't be confused, so I'd rather the args: syntax for one or the other case. (side note thats why in thought a varargs style might be useful to disambiguate).

[papandreou]

Implemented shorthands:

• to have a call satisfying [...] => to have a call satisfying { args: [...] }
• to hvae a call satisfying {0: ..., 1: ...} => to have a call satisfying { args: {0: ..., 1: ...} }

The second shorthand kicks in whenever the object has only numerical properties.

[papandreou]

@bruderstein @sunesimonsen @alexjeffburke I implemented the shorthands for both to have a call satisfying and to have calls satisfying. I think they're a nice addition.

[papandreou]

@alexjeffburke

I'd be less of a fan if the asserting multiple calls is an array of arrays

Are you ok with others having that possibility? :)

[alexjeffburke]

Haha I'm going to ask you a likely annoying Q but exactly which possibility do you mean? Want to make sure wires aren't crossed :)

What I was getting at, or trying to, is I quite like the multiple calls case using
[
{args: }, { args: }
]
which I think contrasts well against the short-hands you just implemented.

Wish I could follow these discussions more but I will do what I can to adopt conventions in the few places I 'own'.

[papandreou]

@alexjeffburke Hehe, sorry for the confusing comment from me, I think I misread your previous comment :)

I understand that you prefer the explicit version, which is perfectly fine. But it sounds like you think the availability of the shorthand introduces an ambiguity or risk?

[alexjeffburke]

Oh no probs! (:

Let me tell you where it came up for context. In -events, I used an array syntax for event matches [bunch, of, things].

Then I implemented multiple events, arrays of arrays and I found myself getting just a little more confused - see the last example in the README. The code seemed to suffer too - check supplied array contains an array. Do different lengths of the arrays inside the array matter? etc etc

But, you know: a) I might be a little too close to the impl side as a basis for my concern b) maybe reading the assertion is enough if you are then very strict about what should go on the RHS.

Lastly, side thought, this interfaces stuff is really tricky!

[papandreou]

@alexjeffburke Thanks for elaborating!

see the last example in the README

Right, I see what you mean. Aren't you missing a return in the it btw.?

Maybe I'm in a little too deep at this point -- API design is really hard. In this case I think that as long as you have a habit of formatting the expect with one expected spy call per line, all of these combinations are perfectly readable:

expect(mySpy, 'to have calls satisfying', [
    [ 1, 2, 3 ],
    [ 4, 5, 6 ]
]);

expect(mySpy, 'to have calls satisfying', [
    { 0: 1, 1: 2 },
    { 1: 5, 2: 6 }
]);

expect(mySpy, 'to have calls satisfying', [
    { args: [ 1, 2, 3 ] },
    { args: [ 4, 5, 6 ] }
]);

expect(mySpy, 'to have calls satisfying', [
    { args: { 0: 1, 1: 2 } },
    { args: { 1: 5, 2: 6 } }
]);

expect(mySpy, 'to have calls satisfying', {
    1: [ 1, 2, 3 ],
    3: [ 4, 5, 6 ]
});

expect(mySpy, 'to have calls satisfying', {
    1: { 0: 1, 1: 2 },
    3: { 1: 5, 2: 6 }
});

expect(mySpy, 'to have calls satisfying', {
    1: { args: [ 1, 2, 3 ] },
    3: { args: [ 4, 5, 6 ] }
});

expect(mySpy, 'to have calls satisfying', {
    1: { args: { 0: 1, 1: 2 } },
    3: { args: { 1: 5, 2: 6 } }
});

expect(mySpy, 'to have calls satisfying', function () {
    mySpy(1, 2, 3);
    mySpy(4, 5, 6);
});

As for whether they're writable in the sense that you don't find yourself referring to the docs all the time, that's harder to tell. For users that know the to satisfy semantics well, I have a feeling they'll try one of the above first. They might not realize that you can also assert the returnValue, thisValue, stack, threw etc. which makes it likely that they'll try the array-of-arrays shorthand first -- which would make this change even better.

[papandreou]

For completeness, here are the supported variants of to have a call satisfying as of the current state of this PR :)

expect(mySpy, 'to have a call satisfying', [ 1, 2, 3 ]);

expect(mySpy, 'to have a call satisfying', { 0: 1, 1: 2 });

expect(mySpy, 'to have a call satisfying', { args: [ 1, 2, 3 ] });

expect(mySpy, 'to have a call satisfying', { args: { 1: 2, 2: 3 } });

expect(mySpy, 'to have a call satisfying', function () {
    mySpy(1, 2, 3);
});

[joelmukuthu]

👍 :)

[sunesimonsen]

I really like how this turned out. Maybe we should just not touch was called with at all and write a deprecation notice in the documentation.

👍