Skip to content

t.throws passes with false, undefined, and null #1676

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

Closed
awmottaz opened this issue Jan 31, 2018 · 2 comments
Closed

t.throws passes with false, undefined, and null #1676

awmottaz opened this issue Jan 31, 2018 · 2 comments
Labels
bug current functionality does not work as desired scope:assertions

Comments

@awmottaz
Copy link

Description

t.throws() will pass if provided the arguments false, undefined and null. I would expect these tests to fail since, according to the docs:

error can be an error constructor, error message, regex matched against the error message, or validation function.

For my particular use case, I wanted to test that the error thrown was an instance of a custom error. However, the custom error was not implemented and so undefined was passed into t.throws(). The test still succeeded, which is not the desired behavior.

Test Source

The following tests will incorrectly pass

test('false', t => {
  t.throws(() => { throw new Error('foo') }, false)
})

test('undefined', t => {
  t.throws(() => { throw new Error('foo') }, undefined)
})

test('null', t => {
  t.throws(() => { throw new Error('foo') }, null)
})

Error Message & Stack Trace

▶ node_modules/.bin/ava -v

  ✔ false
  ✔ undefined
  ✔ null

  3 tests passed

Config

Using vanilla ava—no config

Command-Line Arguments

ava -v

Environment

▶ node -e "var os=require('os');console.log('Node.js ' + process.version + '\n' + os.platform() + ' ' + os.release())"
Node.js v8.9.4
linux 4.13.0-32-generic

▶ node_modules/.bin/ava --version
0.25.0

▶ npm --version
5.6.0
@novemberborn novemberborn added bug current functionality does not work as desired scope:assertions labels Jan 31, 2018
@novemberborn
Copy link
Member

Yea it should have complained about those arguments. Yet another reason we need to refactor it: #1047.

@novemberborn
Copy link
Member

I think we'll have to leave null as an allowable value. It's impossible to specify an assertion message otherwise:

t.throws(fn, null, 'shown when assertion fails')

But as part of #1047 I'll be introducing a more explicit way of defining constructor expectations.

novemberborn added a commit that referenced this issue Feb 12, 2018
Remove `core-assert` dependency. When passed a function as the second
argument, `t.throws()` now assumes its a constructor. This removes
support for a validation function, which may or may not have worked.
Refs #1047.

`t.throws()` now fails if the exception is not an error. Fixes #1440.

Regular expressions are now matched against the error message, not the
result of casting the error to a string. Fixes #1445.

Validate second argument to `t.throws()`. Refs #1676.
novemberborn added a commit that referenced this issue Feb 12, 2018
Remove `core-assert` dependency. When passed a function as the second
argument, `t.throws()` now assumes its a constructor. This removes
support for a validation function, which may or may not have worked.
Refs #1047.

`t.throws()` now fails if the exception is not an error. Fixes #1440.

Regular expressions are now matched against the error message, not the
result of casting the error to a string. Fixes #1445.

Validate second argument to `t.throws()`. Refs #1676.

Assertion failures now display how AVA arrived at the exception.
Constructors are printed when the error is not a correct instance.
Fixes #1471.
novemberborn added a commit that referenced this issue Feb 13, 2018
Remove `core-assert` dependency. When passed a function as the second
argument, `t.throws()` now assumes its a constructor. This removes
support for a validation function, which may or may not have worked.
Refs #1047.

`t.throws()` now fails if the exception is not an error. Fixes #1440.

Regular expressions are now matched against the error message, not the
result of casting the error to a string. Fixes #1445.

Validate second argument to `t.throws()`. Refs #1676.

Assertion failures now display how AVA arrived at the exception.
Constructors are printed when the error is not a correct instance.
Fixes #1471.
novemberborn added a commit that referenced this issue Feb 13, 2018
Fixes #1047. Fixes #1676.

A combination of the following expectations is supported:

```js
t.throws(fn, {of: SyntaxError}) // err instanceof SyntaxError
t.throws(fn, {name: 'SyntaxError'}) // err.name === 'SyntaxError'
t.throws(fn, {is: expectedErrorInstance}) // err === expectedErrorInstance
t.throws(fn, {message: 'expected error message'}) // err.message === 'expected error message'
t.throws(fn, {message: /expected error message/}) // /expected error message/.test(err.message)
```
novemberborn added a commit that referenced this issue Feb 13, 2018
Remove `core-assert` dependency. When passed a function as the second
argument, `t.throws()` now assumes its a constructor. This removes
support for a validation function, which may or may not have worked.
Refs #1047.

`t.throws()` now fails if the exception is not an error. Fixes #1440.

Regular expressions are now matched against the error message, not the
result of casting the error to a string. Fixes #1445.

Validate second argument to `t.throws()`. Refs #1676.

Assertion failures now display how AVA arrived at the exception.
Constructors are printed when the error is not a correct instance.
Fixes #1471.
novemberborn added a commit that referenced this issue Feb 13, 2018
Fixes #1047. Fixes #1676.

A combination of the following expectations is supported:

```js
t.throws(fn, {of: SyntaxError}) // err instanceof SyntaxError
t.throws(fn, {name: 'SyntaxError'}) // err.name === 'SyntaxError'
t.throws(fn, {is: expectedErrorInstance}) // err === expectedErrorInstance
t.throws(fn, {message: 'expected error message'}) // err.message === 'expected error message'
t.throws(fn, {message: /expected error message/}) // /expected error message/.test(err.message)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug current functionality does not work as desired scope:assertions
Projects
None yet
Development

No branches or pull requests

2 participants