FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: mark test-gc-http-client-timeout as flaky on arm by legendecas · Pull Request #43754 · nodejs/node · GitHub

/ node Public

test: mark test-gc-http-client-timeout as flaky on arm - #43754

Closed
legendecas wants to merge 1 commit into
nodejs:mainfrom
legendecas:flaky-test-gc-http-client-timeout
ConversationCommits (1)ChecksFiles changed
Closed

test: mark test-gc-http-client-timeout as flaky on arm#43754
legendecas wants to merge 1 commit into
nodejs:mainfrom
legendecas:flaky-test-gc-http-client-timeout

Conversation

legendecas commented Jul 10, 2022

Copy link
Copy Markdown
Member

sequential/test-gc-http-client-timeout is failing on arm frequently. Mark it as flaky to unblock PRs to land.

Related: nodejs/reliability#320
Related: #43638

nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Jul 10, 2022
legendecas added the fast-track PRs that do not need to wait for 48 hours to land. label Jul 10, 2022

legendecas commented Jul 10, 2022

Copy link
Copy Markdown
Member Author

Requesting fast track since this is purely a status file change.

github-actions Bot commented Jul 10, 2022

Copy link
Copy Markdown
Contributor

Fast-track has been requested by @legendecas. Please 👍 to approve.

RaisinTen approved these changes Jul 10, 2022
View reviewed changes
Comment thread test/sequential/sequential.status Outdated
legendecas force-pushed the flaky-test-gc-http-client-timeout branch from 7e807c8 to 71ffde5 Compare July 10, 2022 14:23
legendecas force-pushed the flaky-test-gc-http-client-timeout branch from 71ffde5 to aaa5461 Compare July 10, 2022 14:23

lpinca commented Jul 10, 2022

Copy link
Copy Markdown
Member

Should we wait until @ShogunPanda's investigation is done? See #43641 (comment).

legendecas commented Jul 11, 2022

Copy link
Copy Markdown
Member Author

@lpinca the intention of this PR is to unblock other PR that have been retrying the CI again and again. I think #43641 is still able to run stress tests with https://ci.nodejs.org/job/node-stress-single-test/ to make sure the fix is effective.

lpinca commented Jul 11, 2022

Copy link
Copy Markdown
Member

@legendecas I understand but a lot of tests have been marked as flaky lately. Ideally we should fix them and remove the flaky designation.

legendecas commented Jul 11, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

@lpinca Sorry, but I may not get your point clearly. Do you suggest that we should not mark tests as flaky and wait until they are fixed?

Marking the flaky tests and keeping track of them asynchronously unblocks other contributors from landing a PR that is not related to the flaky tests. I believe it is vital for helping contributors not be overwhelmed by the constantly failing CI because of flaky tests.

As the change seems to be contentious, I'm removing the fast-track label.

legendecas removed the fast-track PRs that do not need to wait for 48 hours to land. label Jul 11, 2022

lpinca commented Jul 11, 2022

Copy link
Copy Markdown
Member

Do you suggest that we should not mark tests as flaky and wait until they are fixed?

No, I'm fine with this. I'm just concerned about the growing number of flaky tests.

lpinca added the fast-track PRs that do not need to wait for 48 hours to land. label Jul 11, 2022

github-actions Bot commented Jul 11, 2022

Copy link
Copy Markdown
Contributor

Fast-track has been requested by @lpinca. Please 👍 to approve.

tniessen commented Jul 11, 2022

Copy link
Copy Markdown
Member

CI flakiness is a major problem at this point. Most recently, #43522 landed despite relevant test failures, blocking other contributors' work, and there was pushback against reverting it. It took days to fix the tests (to some degree). I can only assume that this happened because CI was resumed multiple times without investigating the actual errors, which probably would not happen if CI did not constantly fail due to unrelated errors.

My take is: mark everything as flaky that is indeed flaky. Once that is done, every CI failure is worth investigating in detail, and we can treat the list of flaky tests as a to-do list.

tniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 11, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 11, 2022

nodejs-github-bot commented Jul 11, 2022

Copy link
Copy Markdown
Collaborator

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

legendecas added a commit that referenced this pull request Jul 11, 2022
test: mark test-gc-http-client-timeout as flaky on arm
c6ec630 
`sequential/test-gc-http-client-timeout` is failing on arm
frequently. Mark it as flaky to unblock PRs to land.

PR-URL: #43754
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

legendecas commented Jul 11, 2022

Copy link
Copy Markdown
Member Author

Landed in c6ec630, thank you for reviewing!

legendecas closed this Jul 11, 2022
legendecas deleted the flaky-test-gc-http-client-timeout branch July 11, 2022 13:42
targos pushed a commit that referenced this pull request Jul 12, 2022
test: mark test-gc-http-client-timeout as flaky on arm
38e92fd 
`sequential/test-gc-http-client-timeout` is failing on arm
frequently. Mark it as flaky to unblock PRs to land.

PR-URL: #43754
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
targos mentioned this pull request Jul 12, 2022
Merged
tniessen mentioned this pull request Jul 12, 2022
Merged
targos pushed a commit that referenced this pull request Jul 20, 2022
test: mark test-gc-http-client-timeout as flaky on arm
97c1096 
`sequential/test-gc-http-client-timeout` is failing on arm
frequently. Mark it as flaky to unblock PRs to land.

PR-URL: #43754
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
This was referenced Jul 23, 2022
Closed
Closed
F3n67u mentioned this pull request Jul 24, 2022
Merged
targos pushed a commit that referenced this pull request Jul 31, 2022
test: mark test-gc-http-client-timeout as flaky on arm
42610cc 
`sequential/test-gc-http-client-timeout` is failing on arm
frequently. Mark it as flaky to unblock PRs to land.

PR-URL: #43754
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
targos mentioned this pull request Aug 3, 2022
Merged
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
test: mark test-gc-http-client-timeout as flaky on arm
b3972a3 
`sequential/test-gc-http-client-timeout` is failing on arm
frequently. Mark it as flaky to unblock PRs to land.

PR-URL: nodejs/node#43754
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lpinca lpinca approved these changes
tniessen tniessen approved these changes
RaisinTen RaisinTen approved these changes

Assignees

No one assigned

Labels

fast-track PRs that do not need to wait for 48 hours to land. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL