'when called with' + 'to throw a' not working correctly

[dasilvacontin]

Doesn't work:

    expect(parseActivity, 'when called with', ['M1'], 'to throw a', SyntaxError)

  1 failing

  1) parseActivity should throw on syntax err:

  SyntaxError
      at Function.parseActivity (src/lib/index.js:42:29)
      at Function.<anonymous> (node_modules/unexpected/lib/assertions.js:1436:37)
      at executeExpect (node_modules/unexpected/lib/Unexpected.js:1196:50)
      at Unexpected.expect [as _expect] (node_modules/unexpected/lib/Unexpected.js:1204:22)
      at expect (node_modules/unexpected/lib/Unexpected.js:845:35)
      at Context.<anonymous> (test/index.spec.js:22:5)

Works:

expect(function () {
      parseActivity('M1')
}, 'to throw a', SyntaxError)

[papandreou]

The when called with/when called assertion actually yields the return value of the function, which is then passed as the subject for the subsequent assertions. Your example will only work if parseActivity('M1') returns a function that then throws a SyntaxError when called without any arguments.

I agree that this appears unintuitive, primarily because when called [with] doesn't explicitly say that the subsequent assertions will receive the return value rather than some context about how the function call went, including whether it threw an exception. Also, due to the general direction of things in Unexpected plugin land, users get accustomed to assertions that form complete sentences, including filler words -- so I'm not really surprised that you gave when called with... to throw a a shot.

We could fix the main problem by renaming the assertion. If we pull in https://github.com/unexpectedjs/unexpected/pull/383 we could fix both problems along with https://github.com/unexpectedjs/unexpected/issues/394 by renaming/changing it to the exact thing that you expected to work:

expect(fn, 'when called to return value satisfying', 'yadda');
expect(fn, 'when called with parameters', [1, 2], 'to return value satisfying', 'yadda');
expect(fn, 'when called with parameter', 123, 'to return value satisfying', 'yadda');
expect(fn, 'when called with parameter', 123, 'to throw a', SyntaxError);

etc.

[sunesimonsen]

We can consider depreciating it, but I don't think doing a major version for this is a good idea. On Sat, 6 May 2017 at 11.02, Andreas Lind [email protected] wrote:

The when called with/when called assertion actually yields the return value of the function, which is then passed as the subject for the subsequent assertions. Your example will only work if parseActivity('M1') returns a function that then throws a SyntaxError when called without any arguments.

I agree that this appears unintuitive, primarily because when called [with] doesn't explicitly say that the subsequent assertions will receive the return value rather than some context about how the function call went, including whether it threw an exception. Also, due to the general direction of things in Unexpected plugin land, users get accustomed to assertions that form complete sentences, including filler words -- so I'm not really surprised that you gave when called with... to throw a a shot.

We could fix the main problem by renaming the assertion. If we pull in #383 https://github.com/unexpectedjs/unexpected/pull/383 we could fix both problems along with #394 https://github.com/unexpectedjs/unexpected/issues/394 by renaming/changing it to the exact thing that you expected to work:

expect(fn, 'when called to return value satisfying', 'yadda');expect(fn, 'when called with parameters', [1, 2], 'to return value satisfying', 'yadda');expect(fn, 'when called with parameter', 123, 'to return value satisfying', 'yadda');expect(fn, 'when called with parameter', 123, 'to throw a', SyntaxError);

etc.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/unexpectedjs/unexpected/issues/395#issuecomment-299626527, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFisnIuxNPW-nWWdfp_RMCY6XUpG_h9ks5r3DcOgaJpZM4NR1Yo .

[alexjeffburke]

I’ve made a start updating the docs for this.

Side thought, of we’re going to clarify this case of an assertion that executed a function perhaps we want to do others - but I imagine that’s separate vomits per doc so let’s see where we get.

[papandreou]

Vomits? :)

[alexjeffburke]

@papandreou sigh "commits" 🙃

[alexjeffburke]

@dasilvacontin would you have been more likely to avoid this trap if the "to throw" documentation were as in this PR: https://github.com/unexpectedjs/unexpected/pull/493

[dasilvacontin]

@alexjeffburke I don't think so. I would still pass the function directly and try the same thing.

[sunesimonsen]

expect(fn, 'when called to return value satisfying', 'yadda');
expect(fn, 'when called with parameters', [1, 2], 'to return value satisfying', 'yadda');
expect(fn, 'when called with parameter', 123, 'to return value satisfying', 'yadda');
expect(fn, 'when called with parameter', 123, 'to throw a', SyntaxError);

@papandreou I don't really like any of these variants as they tie the two asserts together. And I really never intended the assertions to be perfect english. But I do agree that some of these middle assertions can be confusing without reading the documentation. I'm actually a bit puzzled why you wont get a type error. to throw should only accept functions.

[sunesimonsen]

Mhh I get the following error, maybe something was fixed 🤔

[sunesimonsen]

We could consider 'when called' and 'when called with' to yield a function invocation. Then we could make versions of 'to throw' that executes the function invocation. That would allow us to have 'to return value satisfying' on a function invocation. It would be a bit more verbose but also more explicit.

[papandreou]

@sunesimonsen, or use https://github.com/unexpectedjs/unexpected/pull/383 to do it without the intermediate type :smirk_cat:

[alexjeffburke]

This assertion has come up in conversations a couple of times recently: I'm going to be bold and say I feel a consensus is growing that we might need to rethink it, and my personal feeling is Sune's mid-2017 comment (https://github.com/unexpectedjs/unexpected/issues/395#issuecomment-299627343) might be something we should consider.

Given the above, and that it doesn't seem likely we can easily address this behavioural wart, would folks be comfortable if I closed this in favour of a task about looking into withdrawing it?