Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assert.throws can succeed if actual value is not a function and no matchers are used #1637

Closed
smcclure15 opened this issue Jul 15, 2021 · 1 comment
Assignees

Comments

@smcclure15
Copy link
Member

I noticed that passing something like "1" into the throws assertion actually passes:

assert.throws(1);
// or
assert.throws(1, 'succeeds');

What's actually going on is shown if you use a matcher:

assert.throws(1, /somematch/);
  message: fails
  severity: failed
  actual  : TypeError: block.call is not a function
  expected: "/somematch/"

Which is what is used to invoke the actual function inside a try/catch block. So this is true for really anything that isn't a function:

assert.throws(undefined, 'succeeds');
assert.throws(undefined, /somematch/, 'fails');

assert.throws('notafunction', 'succeeds');
assert.throws('notafunction', /somematch/, 'fails');

assert.throws([], 'succeeds');
assert.throws([], /somematch/, 'fails');

assert.throws({}, 'succeeds');
assert.throws({}, /somematch/, 'fails1');

On one hand, sure, when the actual value is invoked, an exception is thrown. But I think this has the potential of false-positives; passing tests but it's really hiding something there. Even something like the following now has me second-guessing and desiring more validation (maybe assertingtypeof ahead of the throws check?):

let myfcn = utils.myFunctionGenerator();
assert.throws(myfcn)

let myfcn2 = utils.myFunctionGenerator(config);
assert.throws(myfcn2)

I'm proposing something more explicit for this case...

As a summary spec, throws shall:

  • pass if the actual value is a function, which throws, and the expected value is undefined
  • pass if the actual value is a function, which throws, and the expected value "matches"
  • fail if the actual value is a function and does not throw
  • (NEW) fail if the actual value is not a function
  • error if the expected value is invalid (Assert: Improve rejects/throws validation handling #1635)

The new item would more align with the rejects validation that the actual value must be a promise (or thenable).

@Krinkle Krinkle added this to the 3.0 release milestone Jul 15, 2021
@Krinkle
Copy link
Member

Krinkle commented Jul 15, 2021

Agreed fully. I propose we deprecate this along with the invalid expected values, possibly in the same patch if you like.

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

No branches or pull requests

2 participants