Skip to content

test: close dirs in fs-opendir test #58855

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

Merged

Conversation

LiviaMedeiros
Copy link
Member

Two subtests in test-fs-opendir.js are not closing dir. Let's fix it.

@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 Jun 26, 2025
Copy link

codecov bot commented Jun 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.09%. Comparing base (b4c5fb4) to head (49591bc).
Report is 15 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #58855      +/-   ##
==========================================
+ Coverage   90.08%   90.09%   +0.01%     
==========================================
  Files         640      640              
  Lines      188446   188446              
  Branches    36960    36963       +3     
==========================================
+ Hits       169757   169777      +20     
+ Misses      11412    11399      -13     
+ Partials     7277     7270       -7     

see 26 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@LiviaMedeiros LiviaMedeiros added 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 Jun 28, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 28, 2025
@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.

Shouldn't we close it in a finally – or even better, use using?

@LiviaMedeiros
Copy link
Member Author

I'd just keep it aligned with the rest of the file for now, and do a larger-scale dont-land-on- refactor across test files replacing finally and success-only cleanups with using later.

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 28, 2025
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 28, 2025
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/58855
✔  Done loading data for nodejs/node/pull/58855
----------------------------------- PR info ------------------------------------
Title      test: close dirs in `fs-opendir` test (#58855)
Author     Livia Medeiros <[email protected]> (@LiviaMedeiros)
Branch     LiviaMedeiros:test-fs-opendir-close-handles -> nodejs:main
Labels     test, author ready, needs-ci
Commits    2
 - test: close dirs in `fs-opendir` test
 - squash: lint
Committers 2
 - LiviaMedeiros <[email protected]>
 - GitHub <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/58855
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/58855
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 26 Jun 2025 21:11:38 GMT
   ✔  Approvals: 2
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/58855#pullrequestreview-2967628139
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/58855#pullrequestreview-2968449945
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2025-06-28T06:36:30Z: https://ci.nodejs.org/job/node-test-pull-request/67695/
- Querying data for job/node-test-pull-request/67695/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 58855
From https://github.com/nodejs/node
 * branch                  refs/pull/58855/merge -> FETCH_HEAD
✔  Fetched commits as 1c2fa7ee7090..49591bc17abb
--------------------------------------------------------------------------------
Auto-merging test/parallel/test-fs-opendir.js
[main bec7fb0a62] test: close dirs in `fs-opendir` test
 Author: LiviaMedeiros <[email protected]>
 Date: Fri Jun 27 05:07:18 2025 +0800
 1 file changed, 2 insertions(+)
Auto-merging test/parallel/test-fs-opendir.js
[main c78fbf9abf] squash: lint
 Author: Livia Medeiros <[email protected]>
 Date: Fri Jun 27 05:16:15 2025 +0800
 1 file changed, 1 insertion(+), 1 deletion(-)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test: close dirs in `fs-opendir` test

PR-URL: #58855
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>

[detached HEAD 2e4e6ca735] test: close dirs in fs-opendir test
Author: LiviaMedeiros <[email protected]>
Date: Fri Jun 27 05:07:18 2025 +0800
1 file changed, 2 insertions(+)
Rebasing (3/4)
Rebasing (4/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
squash: lint

PR-URL: #58855
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>

[detached HEAD f21ea6ecfa] squash: lint
Author: Livia Medeiros <[email protected]>
Date: Fri Jun 27 05:16:15 2025 +0800
1 file changed, 1 insertion(+), 1 deletion(-)
Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/15948276759

@LiviaMedeiros LiviaMedeiros 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. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jun 28, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 28, 2025
@nodejs-github-bot nodejs-github-bot merged commit f0a9478 into nodejs:main Jun 28, 2025
76 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in f0a9478

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. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. 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.

4 participants