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: move test-cli-syntax to sequential #24907

Closed
wants to merge 1 commit into from
Closed

test: move test-cli-syntax to sequential #24907

wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Dec 8, 2018

It is unreliable under load and the CI failures are getting a bit out of
hand. Let's move it to sequential.

Refs: #24403

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

@Trott
test: move test-cli-syntax to sequential
385cffd 
It is unreliable under load and the CI failures are getting a bit out of
hand. Let's move it to sequential.

Refs: nodejs#24403
@nodejs-github-bot
Copy link
Collaborator

@Trott build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1872/pipeline

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Dec 8, 2018
@Trott
Copy link
Member Author

Trott commented Dec 8, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request/19327/

@Trott
Copy link
Member Author

Trott commented Dec 8, 2018

Assuming green CI, I'd like to fast-track this. Collaborators, please 👍 here for fast-tracking.

@Trott Trott added the fast-track PRs that do not need to wait for 48 hours to land. label Dec 8, 2018
@Trott
Copy link
Member Author

Trott commented Dec 8, 2018

/ping @joyeecheung @targos on the fast-track request above

@addaleax
Copy link
Member

addaleax commented Dec 8, 2018

Fwiw, I would prefer to mark this as a flaky test…? Moving to sequential is not a solution, and making the failures appear less often is also not a good thing imo…

@Trott
Copy link
Member Author

Trott commented Dec 8, 2018

Fwiw, I would prefer to mark this as a flaky test…? Moving to sequential is not a solution, and making the failures appear less often is also not a good thing imo…

I see what you're saying for sure. IMO it fails so frequently that marking it flaky will likely turn our CI more-or-less permanently yellow, which will make us start ignoring flaky tests completely. I'd rather have it less frequently show up as red so at least we take note when it fails. Regardless, the issue for this should definitely remain open until this is sorted out. (I included Refs: and not Fixes:, which you probably noticed.)

@Trott
Copy link
Member Author

Trott commented Dec 8, 2018

Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/19330/

The AIX failure was still this test. 😱

@Trott
Copy link
Member Author

Trott commented Dec 8, 2018

AIX again. I'm alarmed that moving to sequential doesn't help AIX much it seems.

Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/19331/

@joyeecheung
Copy link
Member

Maybe moving it to sequential and mark it as flaky? If it still shows up in AIX that much..

@Trott
Copy link
Member Author

Trott commented Dec 8, 2018

It's green this time. If it continues to fail more than it succeeds on AIX, we can mark it flaky.

@Trott
Copy link
Member Author

Trott commented Dec 8, 2018

Landed in 5011dfe

@Trott Trott closed this Dec 8, 2018
Trott added a commit to Trott/io.js that referenced this pull request Dec 8, 2018
@Trott
test: move test-cli-syntax to sequential
5011dfe 
It is unreliable under load and the CI failures are getting a bit out of
hand. Let's move it to sequential.

Refs: nodejs#24403

PR-URL: nodejs#24907
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@gireeshpunathil gireeshpunathil mentioned this pull request Dec 9, 2018
Merged
4 tasks
@vsemozhetbyt vsemozhetbyt mentioned this pull request Dec 9, 2018
Closed
2 tasks
BethGriggs pushed a commit that referenced this pull request Dec 17, 2018
@Trott @BethGriggs
test: move test-cli-syntax to sequential
f41443c 
It is unreliable under load and the CI failures are getting a bit out of
hand. Let's move it to sequential.

Refs: #24403

PR-URL: #24907
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Dec 18, 2018
Merged
@beevelop beevelop mentioned this pull request Dec 18, 2018
Closed
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@Trott @refack
test: move test-cli-syntax to sequential
6d77552 
It is unreliable under load and the CI failures are getting a bit out of
hand. Let's move it to sequential.

Refs: nodejs#24403

PR-URL: nodejs#24907
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 12, 2019
@Trott @BethGriggs
test: move test-cli-syntax to sequential
4e51e3d 
It is unreliable under load and the CI failures are getting a bit out of
hand. Let's move it to sequential.

Refs: #24403

PR-URL: #24907
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Feb 12, 2019
Merged
BethGriggs pushed a commit that referenced this pull request Feb 20, 2019
@Trott @BethGriggs
test: move test-cli-syntax to sequential
09e092e 
It is unreliable under load and the CI failures are getting a bit out of
hand. Let's move it to sequential.

Refs: #24403

PR-URL: #24907
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
@Trott @rvagg
test: move test-cli-syntax to sequential
fedb5d8 
It is unreliable under load and the CI failures are getting a bit out of
hand. Let's move it to sequential.

Refs: #24403

PR-URL: #24907
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott Trott deleted the mv-flaky branch January 13, 2022 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joyeecheung joyeecheung joyeecheung approved these changes

@cjihrig cjihrig cjihrig approved these changes

@targos targos targos approved these changes

Assignees
No one assigned
Labels
fast-track PRs that do not need to wait for 48 hours to land. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Trott @nodejs-github-bot @addaleax @joyeecheung @targos @cjihrig @BethGriggs