Skip to content

test: use case-insensitive path checking on Windows in fs.cpSync tests #59475

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Aug 14, 2025

In certain machine configurations on Windows, fs.readlinkSync() may return a path with upper case drive letter while the other paths may be constructed from a base path with a lower case drive letter (e.g. from process.cwd()). Checking path mismatch in a case-sensitive manner can lead to failure in some tests, specifically with the Windows machine configurations in the Jenkins CI. Since paths are case-insensitive on Windows anyway, compare them in a case-insensitive manner in the tests.

This reduces the orange-ness of the Windows CI. I discovered it by logging the output in a test run

  process.env.NODE_TEST_DIR: undefined
  tmpdir.path: c:\workspace\node-test-binary-windows-js-suites\node\test\.tmp.337
  dest: c:\workspace\node-test-binary-windows-js-suites\node\test\.tmp.337\copy_%2
  src: c:\workspace\node-test-binary-windows-js-suites\node\test\.tmp.337\copy_%1
  join(dest, "a", "c") c:\workspace\node-test-binary-windows-js-suites\node\test\.tmp.337\copy_%2\a\c
  link: C:\workspace\node-test-binary-windows-js-suites\node\test\.tmp.337\copy_%1

In certain machine configurations on Windows, fs.readlinkSync() may
return a path with upper case drive letter while the other paths may be
constructed from a base path with a lower case drive letter (e.g.
from process.cwd()). Checking path mismatch in a case-sensitive
manner can lead to failure in some tests, specifically with the
Windows machine configurations in the Jenkins CI. Since paths
are case-insensitive on Windows anyway, compare them in a
case-insensitive manner in the tests.
@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 Aug 14, 2025
@nodejs-github-bot
Copy link
Collaborator

Copy link

codecov bot commented Aug 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.87%. Comparing base (af5d1c9) to head (0dace4f).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #59475      +/-   ##
==========================================
+ Coverage   89.35%   89.87%   +0.52%     
==========================================
  Files         656      656              
  Lines      192963   192963              
  Branches    37543    37852     +309     
==========================================
+ Hits       172417   173424    +1007     
+ Misses      13026    12093     -933     
+ Partials     7520     7446      -74     

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

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung joyeecheung added the review wanted PRs that need reviews. label Aug 15, 2025
@panva panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 15, 2025
@joyeecheung joyeecheung removed the review wanted PRs that need reviews. label Aug 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue Add this label to land a pull request using GitHub Actions. 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