-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
Investigate flaky test-stream-finished #43623
Comments
it seems the flakiness was introduced in this PR Line 481 in 4267b92
I have added a delay in the response, converting node/test/parallel/test-stream-finished.js Line 654 in 4267b92
into fs.createReadStream(__filename).map(async x => {
await setTimeout(1);
return x;
}).pipe(res); adding such a delay made this reproduce 100% of the time on my mac, and commenting out this line fixed that Line 481 in 4267b92
I am not very familiar with this code but it like a real bug since the test already recieved a response so why is the socket still open? |
adding |
In #43522, four out of five CI runs failed due to |
I will send a PR within couple of hours that will fix that test as suggested above. |
Thanks for help. |
I agree with @tniessen. Revert and re-land once we're confident no more flakiness was introduced. |
#43641 seems to be have fixed the flakyness on those tests now. I think we are good to go there. |
Test
test-stream-finished
Platform
not specific on a single platform
Console output
Build links
https://ci.nodejs.org/job/node-test-commit-osx/45891/nodes=osx11-x64/testReport/junit/(root)/test/parallel_test_stream_finished/
https://ci.nodejs.org/job/node-test-commit-linux/46348/nodes=rhel8-x64/testReport/(root)/test/parallel_test_stream_finished/
https://ci.nodejs.org/job/node-test-commit-osx-arm/6307/nodes=osx11/testReport/junit/(root)/test/parallel_test_stream_finished/
Additional information
I can reproduce it on my macOS 12.2.1:
Related test:
node/test/parallel/test-stream-finished.js
Lines 652 to 667 in 350a6a8
It seems like caused by a regression, this sub-test is added at #40941 by @ronag 8 months ago.
The text was updated successfully, but these errors were encountered: