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 test-gc-http-client-timeout as flaky on arm #43754

Closed

Conversation

legendecas
Copy link
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 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 legendecas added the fast-track PRs that do not need to wait for 48 hours to land. label Jul 10, 2022
@legendecas
Copy link
Member Author

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

@github-actions
Copy link
Contributor

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

@legendecas legendecas force-pushed the flaky-test-gc-http-client-timeout branch from 7e807c8 to 71ffde5 Compare July 10, 2022 14:23
@legendecas legendecas force-pushed the flaky-test-gc-http-client-timeout branch from 71ffde5 to aaa5461 Compare July 10, 2022 14:23
@lpinca
Copy link
Member

lpinca commented Jul 10, 2022

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

@legendecas
Copy link
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
Copy link
Member

lpinca commented Jul 11, 2022

@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
Copy link
Member Author

legendecas commented Jul 11, 2022

@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 legendecas removed the fast-track PRs that do not need to wait for 48 hours to land. label Jul 11, 2022
@lpinca
Copy link
Member

lpinca commented Jul 11, 2022

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 lpinca added the fast-track PRs that do not need to wait for 48 hours to land. label Jul 11, 2022
@github-actions
Copy link
Contributor

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

@tniessen
Copy link
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 tniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 11, 2022
@github-actions 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
Copy link
Collaborator

legendecas added a commit that referenced this pull request Jul 11, 2022
`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
Copy link
Member Author

Landed in c6ec630, thank you for reviewing!

@legendecas legendecas closed this Jul 11, 2022
@legendecas 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
`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 pushed a commit that referenced this pull request Jul 20, 2022
`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 pushed a commit that referenced this pull request Jul 31, 2022
`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>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
`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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

5 participants