Skip to content

test_runner: handle file rename and deletion under watch mode - #53114

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
jakecastelli:fix-test-runner-rename-delete-error
Jun 2, 2024
Merged

test_runner: handle file rename and deletion under watch mode#53114
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
jakecastelli:fix-test-runner-rename-delete-error

Conversation

@jakecastelli

@jakecastellijakecastelli commented May 23, 2024•
edited
Loading

Copy link
Copy Markdown
Member

Fixes: #53113

@nodejs-github-bot

nodejs-github-bot commented May 23, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels May 23, 2024
@jakecastelli
jakecastelliforce-pushed the fix-test-runner-rename-delete-error branch from 174efbb to 019625eCompareMay 23, 2024 07:05
atlowChemi
atlowChemi reviewed May 23, 2024
View reviewed changes
Comment threadlib/internal/test_runner/runner.js Outdated
Comment threadlib/internal/test_runner/runner.js Outdated
@jakecastelli
jakecastelliforce-pushed the fix-test-runner-rename-delete-error branch from 019625e to d23cbc4CompareMay 24, 2024 00:06
MoLow
MoLow reviewed May 24, 2024
View reviewed changes

@MoLowMoLow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please add a test

@jakecastelli
jakecastelliforce-pushed the fix-test-runner-rename-delete-error branch 2 times, most recently from 1f0ae63 to 0d22545CompareMay 25, 2024 15:55
@jakecastelli

jakecastelli commented May 25, 2024

Copy link
Copy Markdown
MemberAuthor

@MoLow I have added tests to cover both rename and delete a watched test file 👍 could you please take a look when you have time

@jakecastelli
jakecastelliforce-pushed the fix-test-runner-rename-delete-error branch from 0d22545 to b1816bbCompareMay 25, 2024 16:04
MoLow
MoLow reviewed May 25, 2024
View reviewed changes
Comment threadtest/parallel/test-runner-watch-mode.mjs Outdated
@jakecastelli

jakecastelli commented May 31, 2024•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Could someone help me to kick start the CI? 👀

Thanks!

@nodejs-github-bot

nodejs-github-bot commented May 31, 2024

Copy link
Copy Markdown
Collaborator

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

@jakecastelli

jakecastelli commented Jun 1, 2024

Copy link
Copy Markdown
MemberAuthor

Looks like there are a few failing jobs (most likely flaky tests 👀) I guess they need a rerun?

@nodejs-github-bot

nodejs-github-bot commented Jun 1, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Jun 2, 2024

Copy link
Copy Markdown
Collaborator

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

@MoLowMoLow added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jun 2, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 2, 2024
@nodejs-github-bot
nodejs-github-bot merged commit 7125931 into nodejs:mainJun 2, 2024
@nodejs-github-bot

nodejs-github-bot commented Jun 2, 2024

Copy link
Copy Markdown
Collaborator

Landed in 7125931

targos pushed a commit that referenced this pull request Jun 3, 2024
@jakecastelli@targos
test_runner: handle file rename and deletion under watch mode
d4f5f80
Fixes: #53113
PR-URL: #53114
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
@RafaelGSSRafaelGSS mentioned this pull request Jun 7, 2024
Merged
eliphazbouye pushed a commit to eliphazbouye/node that referenced this pull request Jun 20, 2024
@jakecastelli@eliphazbouye
test_runner: handle file rename and deletion under watch mode
ec89e0c
Fixes: nodejs#53113
PR-URL: nodejs#53114
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
@jakecastellijakecastelli mentioned this pull request Jun 21, 2024
Merged
@mcollinamcollina mentioned this pull request Jul 17, 2024
Merged
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
@jakecastelli@marco-ippolito
test_runner: handle file rename and deletion under watch mode
9d3699b
Fixes: #53113
PR-URL: #53114
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
@marco-ippolitomarco-ippolito mentioned this pull request Jul 19, 2024
Merged
nodejs-github-bot pushed a commit that referenced this pull request Aug 14, 2024
@jakecastelli
test_runner: fix delete test file cause dependency file not watched
1212eca
When a watched test file is being deleted then the referenced dependency
file(s) will be updated incorrect when `unfilterFilesOwnedBy` method is
called, which will cause tests not being rerun when its referenced
dependency changed. To prevent this case, we can simply `return` when we
detect a watched test file being deleted.
PR-URL: #53533
Refs: #53114
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 19, 2024
@jakecastelli@RafaelGSS
test_runner: fix delete test file cause dependency file not watched
676bbd5
When a watched test file is being deleted then the referenced dependency
file(s) will be updated incorrect when `unfilterFilesOwnedBy` method is
called, which will cause tests not being rerun when its referenced
dependency changed. To prevent this case, we can simply `return` when we
detect a watched test file being deleted.
PR-URL: #53533
Refs: #53114
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduh95aduh95aduh95 left review comments
@MoLowMoLowMoLow approved these changes
@rluvatonrluvatonrluvaton approved these changes
@atlowChemiatlowChemiatlowChemi approved these changes

Assignees

No one assigned

Labels

commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.needs-ciPRs that need a full CI run.test_runnerIssues and PRs related to the test runner subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Rename or delete a watched test file will cause MODULE_NOT_FOUND error

6 participants

@jakecastelli@nodejs-github-bot@MoLow@aduh95@rluvaton@atlowChemi