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: mark as flaky #16694

Merged

Conversation

refack
Copy link
Contributor

@refack refack commented Nov 2, 2017

Refs: #16210

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Nov 2, 2017
@refack
Copy link
Contributor Author

refack commented Nov 2, 2017

/CC @nodejs/testing @nodejs/async_hooks

@refack refack requested a review from Trott November 2, 2017 17:32
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mscdex
Copy link
Contributor

mscdex commented Nov 2, 2017

The commit message could be a little more descriptive...

@gibfahn
Copy link
Member

gibfahn commented Nov 2, 2017

@refack could you add a link to the parallel.status open issue tracking this flaky test? Ideally any flaky test would have an issue link IMO.

Copy link
Contributor

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with the nits mentioned above being taken care of

  • more descriptive commit message
  • link included to issue

I'd like to see this land asap

@MylesBorins MylesBorins mentioned this pull request Nov 3, 2017
4 tasks
`parallel/test-async-wrap-uncaughtexception.js` has become flaky.
At this time investigating the cause is still on going, but this issue
become has prevalent. In order to restore CI status to be relevant,
this marks the test as explicitly FLAKY.

PR-URL: nodejs#16694
Refs: nodejs#16210
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@refack refack force-pushed the test-async-wrap-uncaughtexception-flaky branch from 3a75e89 to 75095d7 Compare November 3, 2017 02:50
@refack refack merged commit 75095d7 into nodejs:master Nov 3, 2017
@refack
Copy link
Contributor Author

refack commented Nov 3, 2017

Landed with nits addressed

@refack refack deleted the test-async-wrap-uncaughtexception-flaky branch November 3, 2017 02:56
cjihrig pushed a commit to cjihrig/node that referenced this pull request Nov 6, 2017
`parallel/test-async-wrap-uncaughtexception.js` has become flaky.
At this time investigating the cause is still on going, but this issue
become has prevalent. In order to restore CI status to be relevant,
this marks the test as explicitly FLAKY.

PR-URL: nodejs#16694
Refs: nodejs#16210
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@cjihrig cjihrig mentioned this pull request Nov 6, 2017
@gibfahn
Copy link
Member

gibfahn commented Nov 14, 2017

Marking as don't land, because I basically just landed both in #16702

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants