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.

Sign up for GitHub

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

Jump to bottom

test: test-inspector-port-zero-cluster flaky #13375

Merged
merged 0 commits into from
Jun 1, 2017
Merged

test: test-inspector-port-zero-cluster flaky #13375

merged 0 commits into from
Jun 1, 2017

Conversation

refack
Copy link
Contributor

@refack refack commented Jun 1, 2017

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 dont-land-on-v4.x inspector Issues and PRs related to the V8 inspector protocol test Issues and PRs related to the tests. labels Jun 1, 2017
@refack
Copy link
Contributor Author

refack commented Jun 1, 2017

Ref: #13343
@nodejs/testing can we fast-tack this?

@refack refack requested a review from Trott June 1, 2017 19:16
@addaleax
Copy link
Member

addaleax commented Jun 1, 2017

Could you edit the commit message to let people know that this is marking a test as flaky?

@refack
Copy link
Contributor Author

refack commented Jun 1, 2017

Could you edit the commit message to let people know that this is marking a test as flaky?

Ack.
I was short-tempered! Got over it.

@mscdex
Copy link
Contributor

mscdex commented Jun 1, 2017

Minor nit: I suggest using present-tense verbs in commit messages: 'mark' instead of 'marking'

lpinca
lpinca reviewed Jun 1, 2017
View reviewed changes
test/inspector/inspector.status Outdated
@@ -5,5 +5,6 @@ prefix inspector
# sample-test : PASS,FLAKY

[true] # This section applies to all platforms
test-inspector-port-zero-cluster.js : PASS,FLAKY
Copy link
Member

Choose a reason for hiding this comment

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

Nit: comment above says without ".js".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

@refack
Copy link
Contributor Author

refack commented Jun 1, 2017

Minor nit: I suggest using present-tense verbs in commit messages: 'mark' instead of 'marking'

ack

@refack
Copy link
Contributor Author

refack commented Jun 1, 2017

🌝 funny how many nit I can make in literally 2 lines.

@Trott
Copy link
Member

Trott commented Jun 1, 2017

literally 2 lines.

Nit: 1 line.

No, just kidding. LGTM

Trott
Trott approved these changes Jun 1, 2017
View reviewed changes
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

@Trott
Copy link
Member

Trott commented Jun 1, 2017

Should get a CI run, but I don't think it needs to wait before landing.

@refack
Copy link
Contributor Author

refack commented Jun 1, 2017

CI: https://ci.nodejs.org/job/node-test-commit/10291/
Will land if green

@refack refack closed this Jun 1, 2017
@refack refack merged commit 45d7839 into nodejs:master Jun 1, 2017
@refack
Copy link
Contributor Author

refack commented Jun 1, 2017

Landed in 45d7839
Let there be green 💚

@refack refack deleted the 13343-flaky branch June 1, 2017 22:26
@mcollina mcollina mentioned this pull request Jun 2, 2017
Closed
@refack refack restored the 13343-flaky branch June 4, 2017 03:42
@refack refack deleted the 13343-flaky branch June 10, 2017 21:16
gibfahn referenced this pull request Jul 18, 2017
@refack @jasnell
test: mark inspector-port-zero-cluster as flaky
2a29c07 
Ref: #13343
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpinca lpinca lpinca left review comments

@gibfahn gibfahn gibfahn approved these changes

@cjihrig cjihrig cjihrig approved these changes

@addaleax addaleax addaleax approved these changes

@Trott Trott Trott approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
inspector Issues and PRs related to the V8 inspector protocol test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@refack @addaleax @mscdex @Trott @jasnell @lpinca @cjihrig @gibfahn @MylesBorins @nodejs-github-bot