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: reduce flakiness of test-runner-output.mjs #52146

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Mar 18, 2024

This commit is similar to #51952. When the system is under load it is possible for these timeout tests to become flaky. We work around that by using a much longer setTimeout() in the test so that it is not racing against the test's timeout. But, we have to unref() such a large timeout. And, because test timeouts do not currently keep the event loop alive, we use a different setTimeout() for that purpose.

Fixes: #52139
Refs: #52140

This commit is similar to nodejs#51952. When the system is under load
it is possible for these timeout tests to become flaky. We
work around that by using a much longer setTimeout() in the test
so that it is not racing against the test's timeout. But, we have
to unref() such a large timeout. And, because test timeouts do
not currently keep the event loop alive, we use a different
setTimeout() for that purpose.

Fixes: nodejs#52139
Refs: nodejs#52140
@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 Mar 18, 2024
@aduh95 aduh95 added fast-track PRs that do not need to wait for 48 hours to land. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Mar 18, 2024
Copy link
Contributor

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

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 18, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@MoLow MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 19, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 19, 2024
@nodejs-github-bot nodejs-github-bot merged commit 978d5a2 into nodejs:main Mar 19, 2024
68 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 978d5a2

@cjihrig cjihrig deleted the flake branch March 19, 2024 13:19
rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Mar 26, 2024
This commit is similar to nodejs#51952. When the system is under load
it is possible for these timeout tests to become flaky. We
work around that by using a much longer setTimeout() in the test
so that it is not racing against the test's timeout. But, we have
to unref() such a large timeout. And, because test timeouts do
not currently keep the event loop alive, we use a different
setTimeout() for that purpose.

Fixes: nodejs#52139
Refs: nodejs#52140
PR-URL: nodejs#52146
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
This commit is similar to #51952. When the system is under load
it is possible for these timeout tests to become flaky. We
work around that by using a much longer setTimeout() in the test
so that it is not racing against the test's timeout. But, we have
to unref() such a large timeout. And, because test timeouts do
not currently keep the event loop alive, we use a different
setTimeout() for that purpose.

Fixes: #52139
Refs: #52140
PR-URL: #52146
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
This commit is similar to #51952. When the system is under load
it is possible for these timeout tests to become flaky. We
work around that by using a much longer setTimeout() in the test
so that it is not racing against the test's timeout. But, we have
to unref() such a large timeout. And, because test timeouts do
not currently keep the event loop alive, we use a different
setTimeout() for that purpose.

Fixes: #52139
Refs: #52140
PR-URL: #52146
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
lukins-cz pushed a commit to lukins-cz/OS-Aplet-node that referenced this pull request Jun 1, 2024
This commit is similar to nodejs#51952. When the system is under load
it is possible for these timeout tests to become flaky. We
work around that by using a much longer setTimeout() in the test
so that it is not racing against the test's timeout. But, we have
to unref() such a large timeout. And, because test timeouts do
not currently keep the event loop alive, we use a different
setTimeout() for that purpose.

Fixes: nodejs#52139
Refs: nodejs#52140
PR-URL: nodejs#52146
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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.

flaky: test-runner-output
4 participants