Skip to content
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

test: remove postmortem test from flaky list #21092

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Jun 2, 2018

test-postmortem-metadata does not ever seem to fail on master. According
to Michaël Zasso, when it fails locally for him during a V8 upgrade, he
pings Colin Ihrig who fixes it almost immediately.

There was some discussion about marking it flaky when it first landed
but it's not clear to me that it is necessary. (We can always mark it
flaky again if it turns out that it is necessary.)

The reason I want this test removed from the status file for flaky tests
is that I believe that file can and should be used as a list of tests to
fix. But this test doesn't need any fixing.

Refs: #17685

@bnoordhuis @cjihrig @targos @nodejs/v8-update

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@Trott Trott added the post-mortem Issues and PRs related to the post-mortem diagnostics of Node.js. label Jun 2, 2018
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jun 2, 2018
test-postmortem-metadata does not ever seem to fail on master. According
to Michaël Zasso, when it fails locally for him during a V8 upgrade, he
pings Colin Ihrig who fixes it almost immediately.

There was some discussion about marking it flaky when it first landed
but it's not clear to me that it is necessary. (We can always mark it
flaky again if it turns out that it is necessary.)

The reason I want this test removed from the status file for flaky tests
is that I believe that file can and should be used as a list of tests to
fix. But this test doesn't need any fixing.

Refs: nodejs#17685
@Trott
Copy link
Member Author

Trott commented Jun 2, 2018

@joyeecheung

@Trott
Copy link
Member Author

Trott commented Jun 2, 2018

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 2, 2018
@Trott
Copy link
Member Author

Trott commented Jun 2, 2018

Linux CI stalled on one host. Re-running: https://ci.nodejs.org/job/node-test-commit-linux/19269/

@mmarchini
Copy link
Contributor

@Trott FYI the flaky status on test-postmortem-metadata will be removed once #20783 lands.

@addaleax
Copy link
Member

addaleax commented Jun 7, 2018

As per @mmarchini’s comment, I think we don’t need this PR anymore, the flaky marker is gone either way. :) I’m closing but feel free to re-open if I’m missing something.

@addaleax addaleax closed this Jun 7, 2018
@Trott Trott deleted the postmortem branch January 13, 2022 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. post-mortem Issues and PRs related to the post-mortem diagnostics of Node.js. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants