test: Add test for piping large input from stdin - #5949
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Just out of curiosity, is there any significance to this number? Will this reproduce the issue on all operating systems?
Sorry, something went wrong.
|
LGTM with a question. |
Sorry, something went wrong.
|
@cjihrig Answering your questions outside of the diff since more people might ask themselves that: I think any input size above 65536 bytes works, but when adding an additional listener via .on('data', …) to process.stdin, the test became a little flaky (sometimes up to 5 blocks of 65536 bytes each would “make it through” the child process). I can only verify that this test works on Linux. But as far as I can tell, the underlying issue depended solely on the relative timing of pause() and resume() calls (and the emission of the corresponding events), so I think there’s a good chance this does also catch the bug on other OSes, too. |
Sorry, something went wrong.
Check that piping a large chunk of data from `process.stdin` into `process.stdout` does not lose any data by verifying that the output has the same size as the input. This is a regression test for nodejs#5927 and fails for the commits in the range [ace1009..89abe86).
|
Updated with your suggestion. |
Sorry, something went wrong.
Sorry, something went wrong.
|
LGTM |
Sorry, something went wrong.
Check that piping a large chunk of data from `process.stdin` into `process.stdout` does not lose any data by verifying that the output has the same size as the input. This is a regression test for #5927 and fails for the commits in the range [ace1009..89abe86). PR-URL: #5949 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
@thealphanerd @nodejs/lts ... while this test may not be entirely applicable to v4 but it may still be worth having. |
Sorry, something went wrong.
|
Should probably go onto lts so we can catch anything similar there |
Sorry, something went wrong.
Check that piping a large chunk of data from `process.stdin` into `process.stdout` does not lose any data by verifying that the output has the same size as the input. This is a regression test for #5927 and fails for the commits in the range [ace1009..89abe86). PR-URL: #5949 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Check that piping a large chunk of data from `process.stdin` into `process.stdout` does not lose any data by verifying that the output has the same size as the input. This is a regression test for #5927 and fails for the commits in the range [ace1009..89abe86). PR-URL: #5949 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Check that piping a large chunk of data from `process.stdin` into `process.stdout` does not lose any data by verifying that the output has the same size as the input. This is a regression test for #5927 and fails for the commits in the range [ace1009..89abe86). PR-URL: #5949 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
Landed in v4.x-staging as 1994ac0 with a slight modification as buffer.alloc does not exist in that release stream |
Sorry, something went wrong.
Pull Request check-list
this change (including linting)?
test (or a benchmark) included? (This is a regression test)
Affected core subsystem(s)
test
Description of change
Check that piping a large chunk of data from process.stdin into process.stdout does not lose any data by verifying that the output has the same size as the input.
This is a regression test for #5927 and fails for the commits in the range [ace1009..89abe86).