Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.3k
test: revert fail coverage target if tests fail"#25543
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 Jan 16, 2019 • 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.
This reverts commit f216d5b. Seems like it breaks the nightly job so reverting until we figure that out.
nodejs-github-bot commented Jan 16, 2019
mhdawson commented Jan 16, 2019
This needs to be reverted as we do have failing tests when run with coverage enabled (and have forever I think) so the original change prevents the gathering and printing of coverage information. |
coverage target if tests fail"coverage target if tests fail"mhdawson commented Jan 16, 2019
There is something up with the main coverage job as well, but testing locally I confirmed we need the revert |
mhdawson commented Jan 17, 2019
Would also like to fast track this. |
mhdawson commented Jan 17, 2019
mhdawson commented Jan 17, 2019
Coverage job CI run, shows that this resolves the issue: https://ci.nodejs.org/job/node-test-commit-linux-coverage-daily/2/ |
mhdawson commented Jan 17, 2019
Renamed jobs as something was broken with the CI job for coverage even though coverage build locally. New CI which references the updated job: https://ci.nodejs.org/view/Node.js%20Daily/job/node-test-commit-linux-coverage/3/ |
mhdawson commented Jan 17, 2019 • 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.
Windows Node.js CI failure was known issue:#25512 Will resume build. |
mhdawson commented Jan 17, 2019
Build resumed: https://ci.nodejs.org/job/node-test-pull-request/20186/ |
BridgeAR commented Jan 17, 2019
I marked this as do not land as the original commit is marked as such. |
mhdawson commented Jan 17, 2019
@BridgeAR thanks. |
mhdawson commented Jan 17, 2019
Strangely renaming a working job, with no other changes caused the job to fail so there is some state somewhere. I've cleared out the workspace, deleted the files on disk but that still did not help. I'm just going to leave it as the new name with "-daily" in the job name. |
mhdawson commented Jan 17, 2019 • 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.
This link is now valid again https://ci.nodejs.org/job/node-test-commit-linux-coverage-daily/2/ |
mhdawson commented Jan 17, 2019
Ok resumed CI job is green :). |
mhdawson commented Jan 17, 2019
Think I've got the ok's for fast-track, landing now. |
mhdawson commented Jan 17, 2019
Landed in 1375af2 |
This reverts commit f216d5b. Seems like it breaks the nightly job so reverting until we figure that out. PR-URL: #25543 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
refack commented Jan 17, 2019 • 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.
That's sounds like a sub-optimal situation. I'll try to look into that. Cross-ref: #25432 |
refack commented Jan 17, 2019
P.S. renaming the job the same name as the broken one caused Jenkins to reuse the broken workspace directory. It might store a broken compilation artifact. |
This reverts commit f216d5b.
Seems like it breaks the nightly job so reverting until
we figure that out.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes