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: fix flaky test-fs-promises-file-handle-read #37371

Merged
merged 1 commit into from
Feb 17, 2021

Conversation

Trott
Copy link
Member

@Trott Trott commented Feb 14, 2021

tmpdir.refresh() cannot be called multiple times reliably on Raspberry
Pi in CI because NFS might optimistically report a path as
removed before it actually is. At least, that's what I think is going
on. Anyway, tmpdir.refresh() is generally designed to be called once,
so let's just call it once.

@Trott Trott added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Feb 14, 2021
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Feb 14, 2021
@nodejs-github-bot
Copy link
Collaborator

Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with or without my suggestion.

test/parallel/test-fs-promises-file-handle-read.js Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member Author

Trott commented Feb 15, 2021

Stress test on master on a Pi 3 shows 7 failures in 100 runs.
https://ci.nodejs.org/job/node-stress-single-test-pi1-binary/130/label=pi3-raspbian-stretch/console

Stress test on this PR on a Pi 3 shows 0 failures in 100 runs.
https://ci.nodejs.org/job/node-stress-single-test-pi1-binary/131/label=pi3-raspbian-stretch/console

@RaisinTen
Copy link
Contributor

We could add this in the commit message too:

Fixes: https://github.com/nodejs/node/issues/37266

tmpdir.refresh() cannot be called multiple times reliably on Raspberry
Pi in CI because NFS might optimistically report a path as
removed before it actually is. At least, that's what I think is going
on. Anyway, tmpdir.refresh() is generally designed to be called once,
so let's just call it once.

PR-URL: nodejs#37371
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott
Copy link
Member Author

Trott commented Feb 17, 2021

Landed in 41516b9

@Trott Trott merged commit 41516b9 into nodejs:master Feb 17, 2021
@Trott Trott deleted the fix-fs-test branch February 17, 2021 03:36
targos pushed a commit that referenced this pull request Feb 28, 2021
tmpdir.refresh() cannot be called multiple times reliably on Raspberry
Pi in CI because NFS might optimistically report a path as
removed before it actually is. At least, that's what I think is going
on. Anyway, tmpdir.refresh() is generally designed to be called once,
so let's just call it once.

PR-URL: #37371
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@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. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants