Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
test: mark test failing on AIX as flaky#8065
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
mhdawson commented Aug 11, 2016 • edited by Trott
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by Trott
Uh oh!
There was an error while loading. Please reload this page.
mhdawson commented Aug 11, 2016
I'm heading out on vacation so do you think once I get your ok I can land or do we need to wait the normal 48 hours. |
addaleax commented Aug 11, 2016
Could you use a |
We now have adequate AIX hardware to add AIX to the regular regression runs. However, there are a couple of failing tests even though AIX was green at one point. This PR marks those tests as flaky so that we can add AIX so that we can spot any new regressions without making the builds RED The tests are being worked under the following PRs - being worked under nodejs#7564 test-async-wrap-post-did-throw test-async-wrap-throw-from-callback test-crypto-random - being worked under nodejs#7973 test-stdio-closed - covered by nodejs#3796 test-debug-signal-cluster
mhdawson commented Aug 11, 2016
don't know why I was thinking "doc:" instead of "test:" fixed |
Trott commented Aug 11, 2016
LGTM. I think changes to .status files should be exempt from the 48-hour rule, but that's just my opinion. So I certainly won't complain if you land it sooner. Should definitely get a CI run first, though, as I'm not sure what types of mishaps can happen if there is a typo in the .status file. So...uh...CI: https://ci.nodejs.org/job/node-test-pull-request/3624/ |
mhdawson commented Aug 11, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Thanks, agree on the CI run was going to do that, thanks for launching |
joaocgreis commented Aug 11, 2016
LGTM I agree with @Trott , marking tests as flaky should be able to land immediately. I started an AIX job with the exact same parameters that would have been used if the job had been launched from node-test-commit: https://ci.nodejs.org/view/All/job/node-test-commit-aix/331/ |
mhdawson commented Aug 11, 2016
@joaocgreis thanks, I had run my report on onnode-test-commit-aix but good to see it as will will be landed. |
We now have adequate AIX hardware to add AIX to the regular regression runs. However, there are a couple of failing tests even though AIX was green at one point. This PR marks those tests as flaky so that we can add AIX so that we can spot any new regressions without making the builds RED The tests are being worked under the following PRs - being worked under #7564 test-async-wrap-post-did-throw test-async-wrap-throw-from-callback test-crypto-random - being worked under #7973 test-stdio-closed - covered by #3796 test-debug-signal-cluster PR-URL: #8065 Reviewed-By: joaocgreis - João Reis <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
mhdawson commented Aug 11, 2016
Landed as a0b3bc5 |
mhdawson commented Aug 11, 2016 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Added AIX back to node-test-commit Build on master to validate all is still green:https://ci.nodejs.org/job/node-test-commit/4519/ (had wrong link) |
Trott commented Aug 11, 2016
This is strange. I thought test-aix was supposed to be yellow, not green. (Wrong setup in the status file maybe?) And even though it's green, it's showing up as red in the status posted to this PR. Huh? |
mhdawson commented Aug 11, 2016
@joaocgreis I'm pretty sure I added the entries as suggested to make it yellow but do see its green instead. |
joaocgreis commented Aug 11, 2016
The status file seems good. I haven't seen anything yellow in a while in CI, I'll investigate this. The bits to show the status in PR's are quite new, and according to @jbergstroem still not final. I see a difference between what is in test-aix and test-linux, one runs if it succeeds and the other if it fails. But I'm no expert in this. |
We now have adequate AIX hardware to add AIX to the regular regression runs. However, there are a couple of failing tests even though AIX was green at one point. This PR marks those tests as flaky so that we can add AIX so that we can spot any new regressions without making the builds RED The tests are being worked under the following PRs - being worked under #7564 test-async-wrap-post-did-throw test-async-wrap-throw-from-callback test-crypto-random - being worked under #7973 test-stdio-closed - covered by #3796 test-debug-signal-cluster PR-URL: #8065 Reviewed-By: joaocgreis - João Reis <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Trott commented Aug 11, 2016
So strange that the post-status-to-PR widget is showing test-aix failing even though it succeeds.... |
Fishrock123 commented Aug 11, 2016
cc @jbergstroem ^ any ideas? |
mhdawson commented Aug 12, 2016
Going to close this since PR landed. Open this issue to track nodejs/build#467 issue with how flaky test failures are reported. |
jbergstroem commented Aug 14, 2016
@Trott, @Fishrock123: yeah, the PR bot/jenkins stuff is not quite done hence my suggestion of delaying making it default. Fixed a few cases post this issue but not quite there yet! |
MylesBorins commented Sep 30, 2016
adding to lts watch but it is not landing cleanly. I'm going to wait until we run ci on staging to see if we have AIX failures and revisit |
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
test
Description of change
We now have adequate AIX hardware to add AIX to
the regular regression runs.
However, there are a couple of failing tests even
though AIX was green at one point. This PR
marks those tests as flaky so that we can add AIX
so that we can spot any new regressions without making
the builds RED
The tests are being worked under the following PRs
test-async-wrap-post-did-throw
test-async-wrap-throw-from-callback
test-crypto-random
test-stdio-closed
test-debug-signal-cluster