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 <shelley.vohr@gmail.com>
@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).

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.

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.

2 participants