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

question: best practice for handling unexpected err in tests #13115

Closed
refack opened this issue May 19, 2017 · 3 comments
Closed

question: best practice for handling unexpected err in tests #13115

refack opened this issue May 19, 2017 · 3 comments
Labels
help wanted Issues that need assistance from volunteers or PRs that need help to proceed. question Issues that look for answers. test Issues and PRs related to the tests.

Comments

@refack
Copy link
Contributor

refack commented May 19, 2017

  • Version: master
  • Platform: *
  • Subsystem: test

Ref: #13092 (comment)
Do we have a better construct for handling err in callbacks:
What I found that gives the most informative results (stack of exception and a frame for the test) are:

  • for regular callbacks common.mustCall((err) => {if (err) assert.fail(err)})
  • for .on('error') handlers (err) => assert.fail(err)

That is the only way to get a frame in the stack.

regular callback

common.mustCall((err) => {if (err) assert.fail(err)})

AssertionError [ERR_ASSERTION]: Error: ENOENT: no such file or directory, open 'd:\code\node-cur\test\tmp\watch\foo.txt?'
    at fs.writeFile.common.mustCall (d:\code\node-cur\test\parallel\test-fs-watchfile.js:79:86)
    at d:\code\node-cur\test\common\index.js:504:15
    at fs.js:1239:7
    at FSReqWrap.oncomplete (fs.js:112:15)

with common.mustCall((err) => {assert.ifError(err)})

or common.mustCall(assert.ifError)

assert.js:574
assert.ifError = function ifError(err) { if (err) throw err; };
                                                  ^

Error: ENOENT: no such file or directory, open 'd:\code\node-cur\test\tmp\watch\foo.txt?'

.on('error')

with (err) => assert.fail(err)

assert.js:92
  throw new AssertionError({
  ^

AssertionError [ERR_ASSERTION]: Error: write EPROTO 101057795:error:140770FC:SSL routines:SSL23_GET_SERVER_HELLO:unknown protocol:openssl\ssl\s23_clnt.c:794:

    at ClientRequest.req.on (D:\code\node-cur\test\parallel\test-async-wrap-GH13045.js:63:35)
    at emitOne (events.js:115:13)
    at ClientRequest.emit (events.js:210:7)
    at TLSSocket.socketErrorListener (_http_client.js:397:9)
    ...

with just assert.fail

assert.js:92
  throw new AssertionError({
  ^

AssertionError [ERR_ASSERTION]: Error: write EPROTO 101057795:error:140770FC:SSL routines:SSL23_GET_SERVER_HELLO:unknown protocol:openssl\ssl\s23_clnt.c:794:

    at emitOne (events.js:115:13)
    at ClientRequest.emit (events.js:210:7)
    at TLSSocket.socketErrorListener (_http_client.js:397:9)
    ...
@refack refack added help wanted Issues that need assistance from volunteers or PRs that need help to proceed. question Issues that look for answers. test Issues and PRs related to the tests. labels May 19, 2017
@refack
Copy link
Contributor Author

refack commented May 19, 2017

/cc @nodejs/testing

@Trott
Copy link
Member

Trott commented Aug 7, 2017

I think what you wrote above is a pretty good summary. Some stuff may have changed, I'm not sure, but you probably know because you're probably the person that would have changed it :-D

If you think it belongs in the test-writing guide, by all means, go for it. Lint rules might be feasible too, not sure to be honest. I'm going to close, but feel free to re-open if there's more to talk about.

@Trott Trott closed this as completed Aug 7, 2017
@refack
Copy link
Contributor Author

refack commented Aug 7, 2017

Yeah, time to turn this into action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues that need assistance from volunteers or PRs that need help to proceed. question Issues that look for answers. test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

2 participants