Skip to content

test: accept ENOTEMPTY from libc++ in test-fs-rm - #66065

Open
codebytere wants to merge 1 commit into
nodejs:mainfrom
codebytere:test-fs-rm-libcxx-enotempty
Open

codebytere wants to merge 1 commit into
nodejs:mainfrom
codebytere:test-fs-rm-libcxx-enotempty

Conversation

@codebytere

Copy link
Copy Markdown
Member

Refs: llvm/llvm-project#197104
Refs: #57103

fs.rmSync() is implemented with std::filesystem::remove_all(). When a read-only directory has a child, libc++ before LLVM 23 drops the child's EACCES and reports the ENOTEMPTY it then gets for the parent instead (llvm/llvm-project#197104). #57103 pinned that to macOS in test-fs-rm, but it depends on the C++ standard library rather than on the OS: a Linux build against libc++ fails the read-only-subdirectory case when the suite runs as non-root, and macOS will start reporting EACCES once it ships the fixed libc++.

This accepts either code for that case on all POSIX platforms. Checked with a standalone remove_all() program over the same directory layout - libstdc++ 13 reports EACCES, libc++ 18 reports ENOTEMPTY - and by running the test as non-root against a regular (libstdc++) Linux build before and after.


Disclosure: the code and this description were written by Claude Code, directed and reviewed by @codebytere.

fs.rmSync() is implemented with std::filesystem::remove_all(). When a
read-only directory has a child, libc++ before LLVM 23 drops the
child's EACCES and reports the ENOTEMPTY it then gets for the parent
instead. The test pinned that behavior to macOS, but it depends on the
C++ standard library rather than on the OS: a Linux build against
libc++ fails the test when run as non-root, and macOS will start
reporting EACCES once it ships the fixed libc++. Accept either code
for that case on all POSIX platforms.

Refs: llvm/llvm-project#197104
Refs: nodejs#57103
Signed-off-by: Shelley Vohr <[email protected]>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure. labels Sep 16, 2026
@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.25%. Comparing base (67e66b8) to head (8d19a6c).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66065      +/-   ##
==========================================
- Coverage   90.27%   90.25%   -0.03%     
==========================================
  Files         789      789              
  Lines      271473   271473              
  Branches    51808    51808              
==========================================
- Hits       245066   245006      -60     
- Misses      16880    16939      +59     
- Partials     9527     9528       +1     

see 30 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.

@codebytere codebytere added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 17, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 17, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants