Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.42%. Comparing base (b02cd41) to head (9613c19).
Report is 8 commits behind head on main.

@@            Coverage Diff             @@
##             main   #55553      +/-   ##
==========================================
- Coverage   88.42%   88.42%   -0.01%     
==========================================
  Files         654      654              
  Lines      187852   187852              
  Branches    36134    36135       +1     
==========================================
- Hits       166102   166101       -1     
- Misses      14989    14991       +2     
+ Partials     6761     6760       -1     

... and 35 files with indirect coverage changes

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

Let's wait for #55548 to land first: since there's no other test failure, it seems plausible that this change does not have any impact on the public API, it might also be due to a lack of coverage, so better safe than sorry.

This is not directly connected to the deprecated .path property. This PR fixes .parentPath, while .path just happens to return same value.
AFAICT this issue is facing public API, but it's hard to reproduce: it must be withFileTypes: true, it must be async, and most importantly, libuv must report UV_DIRENT_UNKNOWN. I have no idea how to robustly make it return UV_DIRENT_UNKNOWN type (assigning invalid type using debugfs didn't work for me). It also explains why this bug was never caught: if someone encountered corrupted files, they probably had more important and urgent problems to investigate than wrong .parentPath value.

If anyone knows how to test this on CI without exposing internals, I'd be happy to increase coverage here.

Landing this after #55548 (which is semver-major and will change tests for .path property) is likely to make backporting this PR to previous release lines necessary. But given that the bug is hard to encounter, and that test in #55547 is rewritten using sync call and can be landed without this, I'm fine with it.

#55548 […] will change tests for .path property

It should remove them yes

Landing this after #55548 […] is likely to make backporting this PR to previous release lines necessary

Hum no, I would say we should not backport it, or at least not unless we find a way to not interfere with the .path property

#55548 […] will change tests for .path property

It should remove them yes

Landing this after #55548 […] is likely to make backporting this PR to previous release lines necessary

Hum no, I would say we should not backport it, or at least not unless we find a way to not interfere with the .path property

Rebased on top of #55548 ( semver-major PRs that contain breaking changes and should be released in the next major version. ), hence the dont-land labels.

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

Landed in c91ce21