Skip to content

Conversation

@cruzz77
Copy link
Contributor

@cruzz77 cruzz77 commented Nov 23, 2025

Refs: #60822

Problem

The implementation of path.posix.normalize() explicitly handles the empty string case:

if (path.length === 0)
  return '.';

However, the test suite (test/parallel/test-path-normalize.js) did not include a test for this behavior, leaving this branch untested.

Fix

This PR adds the following assertion to ensure the empty path case is properly covered:

assert.strictEqual(path.posix.normalize(''), '.');
  • Improves test coverage for the path subsystem
  • Ensures edge-case behavior is tested and protected
  • Helps prevent regressions during future refactors

Tests

  • ✔ Added new test case
  • ✔ All existing tests pass

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/path

@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 Nov 23, 2025
@codecov
Copy link

codecov bot commented Nov 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.53%. Comparing base (1758b74) to head (77e97e2).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #60823      +/-   ##
==========================================
- Coverage   88.56%   88.53%   -0.03%     
==========================================
  Files         703      703              
  Lines      208268   208268              
  Branches    40175    40171       -4     
==========================================
- Hits       184444   184394      -50     
- Misses      15828    15874      +46     
- Partials     7996     8000       +4     

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

@aduh95
Copy link
Contributor

aduh95 commented Nov 23, 2025

That's already covered by the tests

// Normalize will return '.' if the input is a zero-length string
assert.strictEqual(path.posix.normalize(''), '.');

@aduh95 aduh95 closed this Nov 23, 2025
@cruzz77
Copy link
Contributor Author

cruzz77 commented Nov 23, 2025

Thank you for pointing that out and for taking the time to review my PR. I totally understand why the PR was closed since this case is already properly tested.
I'm still learning about the codebase and appreciate the guidance. Thank you for your support!

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 the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants