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.

Sign up for GitHub

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

Jump to bottom

Unnecessary parameter in assert call in parallel/test-stream2-basic.js #21305

Closed
djmgit opened this issue Jun 13, 2018 · 1 comment
Closed

Unnecessary parameter in assert call in parallel/test-stream2-basic.js #21305

djmgit opened this issue Jun 13, 2018 · 1 comment

Comments

@djmgit
Copy link
Contributor

djmgit commented Jun 13, 2018 •
edited
Loading

  • Version: v11.0.0-pre
  • Platform: Ubuntu, x86_64 GNU/Linux
  • Subsystem: test

In file test/parallel/test-stream2-basic.js, in line 215 and 218, the third argument is a string literal ("first" on line 215 and "second" on line 218). Which means if there is an AssertionError, the cause error that is difference between expected and received will not be displayed, but only first or second will be printed on terminal which is quite odd.
Moreover, it is quite strange to specify "first" or "second" as assertion failure message as it will be clear from stack trace which assertion has failed.
So there is no need for the third argument in both of those two lines.
@djmgit
Copy link
Contributor Author

djmgit commented Jun 13, 2018

I am working on this.

djmgit added a commit to djmgit/node that referenced this issue Jun 13, 2018
@djmgit
test: removing unnecessary parameter from assert call
ed254a0 
Removed unnecessary parameter from assert call in
test/parallel/test-stream2-basic.js.

Fixes: nodejs#21305
@djmgit djmgit mentioned this issue Jun 13, 2018
Closed
4 tasks
targos pushed a commit that referenced this issue Jun 14, 2018
@djmgit @targos
test: removing unnecessary parameter from assert call
0b00172 
Removed unnecessary parameter from assert call in
test/parallel/test-stream2-basic.js.

Fixes: #21305

PR-URL: #21307
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove unnecessary parameter in assert call in test-stream2-basic.js djmgit/node
1 participant
@djmgit