test: retry on known SmartOS bug #5454
Conversation
Trott
added
http
|
Stress test for this version of the test, expected to show zero failures: https://ci.nodejs.org/job/node-stress-single-test/545/nodes=smartos14-32/console Stress test for current version on master, showing failures: https://ci.nodejs.org/job/node-stress-single-test/528/nodes=smartos14-32/console |
How is it done? |
|
@santigimeno wrote:
|
|
/cc @nodejs/testing |
|
Bump. @nodejs/collaborators |
|
Still fishing for an Maybe @mscdex who reported the problem? Or @jasnell who Or @indutny who reviewed a PR for a more generalized fix that helps other tests but not necessarily ones with lots of connections like this one? |
|
Why doesn't more generalized fix do not help here? |
| socket.on('error', (e) => { | ||
| // If SmartOS and ECONNREFUSED, then retry. See | ||
| // https://github.com/nodejs/node/issues/2663. | ||
| if (common.isSunOS && (e.code === 'ECONNREFUSED')) { |
There was a problem hiding this comment.
Please skip additional parens here.
There was a problem hiding this comment.
Please skip additional parens here.
Additional parens removed, branch rebased, force pushed. Will run CI one more time.
There's a build-in lint rule to catch extra parentheses, Alas, enabling it flags 315 instances in the code base. Probably not worth the churn. I might look more closely at it, though...
|
Anyway, if it helps - LGTM |
|
@indutny asked:
The more generalized fix retries the test one time if it's SmartOS and failed due to However, for tests that open dozens or hundreds of connections, one retry may not be enough. So, for this and for one other test, we ignore |
There is a known issue with SmartOS that is generally worked around in `tools/test.py`. However, a more robust workaround is required for some tests that open many network connections. `test-http-regr-nodejsgh-2928` is one such test. Fixes: nodejs#5445 Refs: nodejs#3941 PR-URL: nodejs#5454
|
Hopefully gratuitous CI: https://ci.nodejs.org/job/node-test-pull-request/1796/ |
There is a known issue with SmartOS that is generally worked around in `tools/test.py`. However, a more robust workaround is required for some tests that open many network connections. `test-http-regr-nodejsgh-2928` is one such test. Fixes: nodejs#5445 Refs: nodejs#3941 PR-URL: nodejs#5454 Reviewed-By: Fedor Indutny <fedor@indutny.com>
|
Landed in 8592697 |
There is a known issue with SmartOS that is generally worked around in `tools/test.py`. However, a more robust workaround is required for some tests that open many network connections. `test-http-regr-gh-2928` is one such test. Fixes: #5445 Refs: #3941 PR-URL: #5454 Reviewed-By: Fedor Indutny <fedor@indutny.com>
There is a known issue with SmartOS that is generally worked around in `tools/test.py`. However, a more robust workaround is required for some tests that open many network connections. `test-http-regr-gh-2928` is one such test. Fixes: #5445 Refs: #3941 PR-URL: #5454 Reviewed-By: Fedor Indutny <fedor@indutny.com>
There is a known issue with SmartOS that is generally worked around in `tools/test.py`. However, a more robust workaround is required for some tests that open many network connections. `test-http-regr-gh-2928` is one such test. Fixes: #5445 Refs: #3941 PR-URL: #5454 Reviewed-By: Fedor Indutny <fedor@indutny.com>
The flakiness issue for test-http-regr-nodejsgh-2928 on SmartOS was resolved in late February in nodejs#5454. This change removes its flaky designation in sequential.status.
The flakiness issue for test-http-regr-nodejsgh-2928 on SmartOS was resolved in late February in nodejs#5454. This change removes its flaky designation in sequential.status. PR-URL: nodejs#6540 Reviewed-By: James M Snell <jasnell@gmail.com>
There is a known issue with SmartOS that is generally worked around
in
tools/test.py. However, a more robust workaround is required forsome tests that open many network connections.
test-http-regr-gh-2928is one such test.Fixes: #5445
Refs: #3941