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: make fs watch test more stable #41715

Conversation

benjamingr
Copy link
Member

@benjamingr benjamingr commented Jan 27, 2022

The test currently uses a timeout to deal with a watch event for the directory being created.

This is flaky e.g. https://ci.nodejs.org/job/node-test-binary-windows-js-suites/RUN_SUBSET=2,nodes=win10-COMPILED_BY-vs2019/lastCompletedBuild/testReport/(root)/test/pummel_test_fs_watch_non_recursive_/

Instead this changes the common.mustNotCall to allow calls with testdirname so we do not depend on timing as much.

@benjamingr benjamingr force-pushed the test-fs-watch-non-recursive-stabilize branch from e6f5abb to ec2ac66 Compare January 27, 2022 10:41
@benjamingr benjamingr added flaky-test Issues and PRs related to the tests with unstable failures on the CI. test Issues and PRs related to the tests. labels Jan 27, 2022
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jan 27, 2022
@benjamingr benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 27, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 27, 2022
@nodejs-github-bot

This comment has been minimized.

@Trott
Copy link
Member

Trott commented Jan 27, 2022

@nodejs/testing

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 if CI is green.

@benjamingr benjamingr force-pushed the test-fs-watch-non-recursive-stabilize branch from ec2ac66 to 92af8b3 Compare January 27, 2022 11:09
@benjamingr benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 27, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 27, 2022
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 29, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 29, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/41715
✔  Done loading data for nodejs/node/pull/41715
----------------------------------- PR info ------------------------------------
Title      test: make fs watch test more stable (#41715)
Author     Benjamin Gruenbaum  (@benjamingr)
Branch     benjamingr:test-fs-watch-non-recursive-stabilize -> nodejs:master
Labels     test, flaky-test, needs-ci
Commits    1
 - test: make fs watch test more stable
Committers 1
 - Benjamin Gruenbaum 
PR-URL: https://github.com/nodejs/node/pull/41715
Reviewed-By: Rich Trott 
Reviewed-By: Colin Ihrig 
Reviewed-By: Richard Lau 
Reviewed-By: Luigi Pinca 
Reviewed-By: James M Snell 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/41715
Reviewed-By: Rich Trott 
Reviewed-By: Colin Ihrig 
Reviewed-By: Richard Lau 
Reviewed-By: Luigi Pinca 
Reviewed-By: James M Snell 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 27 Jan 2022 10:41:26 GMT
   ✔  Approvals: 5
   ✔  - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/41715#pullrequestreview-864717732
   ✔  - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/41715#pullrequestreview-865074990
   ✔  - Richard Lau (@richardlau) (TSC): https://github.com/nodejs/node/pull/41715#pullrequestreview-865081510
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/41715#pullrequestreview-865332525
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/41715#pullrequestreview-866774036
   ✖  This PR needs to wait 9 more hours to land
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-01-27T15:58:30Z: https://ci.nodejs.org/job/node-test-pull-request/42195/
- Querying data for job/node-test-pull-request/42195/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/1764418351

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jan 29, 2022
@richardlau richardlau removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jan 29, 2022
@Trott Trott added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 29, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 29, 2022
@nodejs-github-bot nodejs-github-bot merged commit 05e9cb6 into nodejs:master Jan 29, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 05e9cb6

Linkgoron pushed a commit to Linkgoron/node that referenced this pull request Jan 31, 2022
PR-URL: nodejs#41715
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
ruyadorno pushed a commit that referenced this pull request Feb 8, 2022
PR-URL: #41715
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this pull request Mar 2, 2022
PR-URL: #41715
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this pull request Mar 3, 2022
PR-URL: #41715
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this pull request Mar 14, 2022
PR-URL: #41715
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. 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.

7 participants