test: pummel test fixes - #4998
Conversation
Sorry, something went wrong.
This fixes some variable redeclarations in pummel tests and, in the process, gets `test-stream-pipe-multi.js` to run again. It had been failing to run.
There was a problem hiding this comment.
Are you sure this is safe to remove? Obviously the chunkSize[i] part is wrong, but perhaps this should be kept and just replace chunkSize[i] with data[i] assuming that was the original intention?
Sorry, something went wrong.
There was a problem hiding this comment.
I think it's safe to remove because I don't think the contents of the buffer affects anything in the test. But regardless, I can see how failing to initialize the buffer contents doesn't pass the smell test even if those contents are unused. So I'll put some initialization back.
It's odd because the modulus operator is superfluous because the buffer has a length of 250 so % 256 doesn't do anything.
And all this stuff was in the very first commit for the test. So it seems likely that this test has never run successfully.
Sorry, something went wrong.
There was a problem hiding this comment.
OK, initialization restored.
Sorry, something went wrong.
|
LGTM if CI is happy: https://ci.nodejs.org/job/node-test-pull-request/1457/ |
Sorry, something went wrong.
|
LGTM |
Sorry, something went wrong.
This fixes some variable redeclarations in pummel tests and, in the process, gets `test-stream-pipe-multi.js` to run again. It had been failing to run. PR-URL: #4998 Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: James M Snell <jasnell@gmail.com>
This fixes some variable redeclarations in pummel tests and, in the process, gets `test-stream-pipe-multi.js` to run again. It had been failing to run. PR-URL: #4998 Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: James M Snell <jasnell@gmail.com>
This fixes some variable redeclarations in pummel tests and, in the process, gets `test-stream-pipe-multi.js` to run again. It had been failing to run. PR-URL: #4998 Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: James M Snell <jasnell@gmail.com>
This fixes some variable redeclarations in pummel tests and, in the process, gets `test-stream-pipe-multi.js` to run again. It had been failing to run. PR-URL: #4998 Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: James M Snell <jasnell@gmail.com>
This fixes some variable redeclarations in pummel tests and, in the process, gets `test-stream-pipe-multi.js` to run again. It had been failing to run. PR-URL: #4998 Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: James M Snell <jasnell@gmail.com>
This fixes some variable redeclarations in pummel tests and, in the process, gets `test-stream-pipe-multi.js` to run again. It had been failing to run. PR-URL: nodejs#4998 Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: James M Snell <jasnell@gmail.com>
This fixes some variable redeclarations in pummel tests and, in the
process, gets test-stream-pipe-multi.js to run again. It had been
failing to run.