-
Notifications
You must be signed in to change notification settings - Fork 30k
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
flaky test-net-throttle on daily master #33135
Comments
We were almost green last night... albeit by diligent skipping of tests. |
First failed daily run was for f8d5474 and last successful daily run was for 24a4e61. That's a span of only 6 commits. If this reproduces locally, a bisect should find the problematic commit pretty quickly. And if it doesn't reproduce locally, using CI to find the problematic commit should be feasible. |
Starting the bisect on CI since that will probably finish before the local compilation I'm doing for this will finish. https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/14177/ will be f8d5474 and should fail. https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/14178/ will be 24a4e61 and should pass. https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/14179/ will be 658cae0, which is between the two. |
The test passes locally for me, and all three CI jobs failed, so that suggests it may have been a change on the CI host and not a change in the code... |
It looks to me like the test makes assumptions about how much data it will take before the kernel buffer fills and data gets queued in memory, and that this assumption may no longer be true for the CI host. Is that your assessment too, @sam-github? Or should all systems queue data with this test? @nodejs/build Any chance anyone beefed up test-rackspace-ubuntu1604-x64-1 around two weeks ago? |
#33329 seems to fix it. |
Failed last three nightlies:
The text was updated successfully, but these errors were encountered: