test: exercise EE function type checking paths #8168
Conversation
cjihrig
added
the
events
| @@ -42,7 +42,7 @@ e.emit('hello', 'a', 'b'); | |||
|
|
|||
|
|
|||
| // just make sure that this doesn't throw: | |||
| var f = new events.EventEmitter(); | |||
| var f = new EventEmitter(); | |||
There was a problem hiding this comment.
prefer wrapping in a block rather than having a new variable name?
or is that too large of a change?
There was a problem hiding this comment.
Switched to const and did some refactoring of this file for scoping, dropping the on exit handler, preferring strictEqual(), etc.
|
LGTM if CI is green |
| const ee = new EventEmitter(); | ||
|
|
||
| ee.on('foo', null); | ||
| }, /^TypeError: "listener" argument must be a function$/); |
There was a problem hiding this comment.
Adding a final message param may be helpful (I forget but I don't think throws logs the regex?)
There was a problem hiding this comment.
What would you like the message to be? If an incorrect error is thrown, the message isn't used at all. If an error isn't thrown, it says AssertionError: Missing expected exception., which is what I would have used as the message.
There was a problem hiding this comment.
Ping @Fishrock123 on ideas here.
|
lgtm with ci & nits |
| // Verify that the listener must be a function | ||
| assert.throws(() => { | ||
| const ee = new EventEmitter(); | ||
|
|
There was a problem hiding this comment.
extremely minor nit: unnecessary blank line?
There was a problem hiding this comment.
I dunno. A blank line after a variable declaration is pretty common throughout the codebase.
|
LGTM with the nits addressed. |
cjihrig
force-pushed
the
ee-test
branch
2 times, most recently
from
31ba26c to
e80e56f
Compare
This commit adds tests for on(), once(), removeListener(), and prependOnceListener(), which all throw a TypeError if the listener argument is not a function. PR-URL: nodejs#8168 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: James M Snell <[email protected]>
This commit adds tests for on(), once(), removeListener(), and prependOnceListener(), which all throw a TypeError if the listener argument is not a function. PR-URL: #8168 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: James M Snell <[email protected]>
|
This does not land cleanly in LTS. Added dont-land label. Please feel free to manually backport |
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
test
Description of change
This commit adds tests for
on(),once(),removeListener(), andprependOnceListener(), which all throw aTypeErrorif the listener argument is not a function.