test: add coverage for toRelativePath and getDirectoryForPath#95
Conversation
These two exported functions in src/utils/path.ts lacked unit tests. toRelativePath wraps Node's path.relative() and keeps the absolute path when the result would start with '..' (i.e. the target is outside the CWD). Four tests cover: child-of-cwd, outside-cwd, cwd-itself, and the type-safety invariant. getDirectoryForPath uses statSync to distinguish directories from files, falling back to dirname() for non-existent paths and bypassing the filesystem for UNC paths (NTLM credential-leak prevention). Three tests cover: existing directory, known file, and non-existent path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe pull request extends the test suite for path utilities by adding comprehensive test coverage for two newly exported functions: Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/utils/__tests__/path.test.ts (1)
185-191: Tests rely on specific repository structure.These tests assume
package.jsonexists at the repository root and useprocess.cwd()to construct paths. Similar to the previous test, ifprocess.cwd()differs fromgetCwd()(used internally by the implementation), these tests may produce unreliable results.Consider documenting this assumption or making the tests more resilient by checking for file/directory existence before asserting expected behavior.
Also applies to: 193-198
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/__tests__/path.test.ts` around lines 185 - 191, The test "returns parent directory for a known file" (and the similar test at 193-198) assumes package.json lives at process.cwd(), which can diverge from the implementation's getCwd() and break CI; update the tests that call getDirectoryForPath to first verify the target file/directory exists and use getCwd() (or mock getCwd/process.cwd consistently) to build the expected path, or skip/assert conditionally when the file is missing so tests don't rely on repository layout; specifically modify the tests referencing process.cwd(), getDirectoryForPath, and any use of getCwd to either mock/align the working directory or check fs.existsSync before asserting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils/__tests__/path.test.ts`:
- Around line 139-147: The test builds an absolute path using process.cwd() but
toRelativePath() reads the current working dir via getCwd(), which can differ
when runWithCwdOverride()/getCwdState() are used; update the test to import
getCwd() and use getCwd() instead of process.cwd() when constructing abs so the
input and implementation use the same cwd source (also add the getCwd import at
the top of src/utils/__tests__/path.test.ts).
---
Nitpick comments:
In `@src/utils/__tests__/path.test.ts`:
- Around line 185-191: The test "returns parent directory for a known file" (and
the similar test at 193-198) assumes package.json lives at process.cwd(), which
can diverge from the implementation's getCwd() and break CI; update the tests
that call getDirectoryForPath to first verify the target file/directory exists
and use getCwd() (or mock getCwd/process.cwd consistently) to build the expected
path, or skip/assert conditionally when the file is missing so tests don't rely
on repository layout; specifically modify the tests referencing process.cwd(),
getDirectoryForPath, and any use of getCwd to either mock/align the working
directory or check fs.existsSync before asserting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ab4505cc-adea-4c02-b972-3344adb0b82b
📒 Files selected for processing (1)
src/utils/__tests__/path.test.ts
| test("returns relative path for a child of cwd", () => { | ||
| // Build a path that is inside the current working directory. | ||
| // resolve() returns an absolute path, and toRelativePath should give | ||
| // back just the final segment (or relative form without ..). | ||
| const abs = resolve(process.cwd(), "package.json"); | ||
| const result = toRelativePath(abs); | ||
| expect(result).toBe("package.json"); | ||
| expect(result).not.toContain(".."); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the getCwd implementation to understand if it can differ from process.cwd()
# Find the getCwd implementation
ast-grep --pattern 'function getCwd() {
$$$
}'
ast-grep --pattern 'export function getCwd() {
$$$
}'
# Also search for getCwd definition with const/let
rg -n "(?:export\s+)?(?:const|let|var)\s+getCwd\s*=" -A 5Repository: claude-code-best/claude-code
Length of output: 54
🏁 Script executed:
# Search for getCwd in all files
rg "getCwd" -n --type ts --type tsxRepository: claude-code-best/claude-code
Length of output: 99
🏁 Script executed:
# Also check the test file structure
cat -n src/utils/__tests__/path.test.ts | head -50Repository: claude-code-best/claude-code
Length of output: 1809
🏁 Script executed:
# Find the implementation file for toRelativePath
fd -e ts -e tsx | xargs rg -l "toRelativePath" | grep -v testRepository: claude-code-best/claude-code
Length of output: 151
🏁 Script executed:
# Read the implementation file to see how getCwd is defined
cat -n src/utils/path.tsRepository: claude-code-best/claude-code
Length of output: 6856
🏁 Script executed:
# Read the cwd.js file to see how getCwd is implemented
cat -n src/utils/cwd.jsRepository: claude-code-best/claude-code
Length of output: 120
🏁 Script executed:
# List files in src/utils directory
ls -la src/utils/Repository: claude-code-best/claude-code
Length of output: 23384
🏁 Script executed:
# Search for any file with cwd in the name
fd "cwd" src/utils/Repository: claude-code-best/claude-code
Length of output: 154
🏁 Script executed:
# Search for getCwd definition anywhere in the codebase
rg "export.*getCwd|function getCwd|const getCwd" --type tsRepository: claude-code-best/claude-code
Length of output: 859
🏁 Script executed:
cat -n src/utils/cwd.tsRepository: claude-code-best/claude-code
Length of output: 1280
Use getCwd() consistently in tests instead of process.cwd().
The test constructs paths using process.cwd(), but toRelativePath() internally uses getCwd(). These can return different values if an async context override is active via runWithCwdOverride(), or if getCwdState() differs from process.cwd(). This creates a mismatch that may cause tests to pass with false confidence or fail unexpectedly. Import and use getCwd() from the same source for accurate test validation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/__tests__/path.test.ts` around lines 139 - 147, The test builds an
absolute path using process.cwd() but toRelativePath() reads the current working
dir via getCwd(), which can differ when runWithCwdOverride()/getCwdState() are
used; update the test to import getCwd() and use getCwd() instead of
process.cwd() when constructing abs so the input and implementation use the same
cwd source (also add the getCwd import at the top of
src/utils/__tests__/path.test.ts).
2 participants
Summary
toRelativePathandgetDirectoryForPathare exported fromsrc/utils/path.tsand used throughout the codebase but had no unit teststoRelativePath: returns a relative path for children of CWD, keeps the absolute path when the target is outside CWD (relative would start with..), returns""for CWD itself, and type-safety invariantgetDirectoryForPath: returns the path itself for an existing directory, returns the parent directory for a known file, and returns the parent directory for a non-existent pathTest plan
bun test src/utils/__tests__/path.test.ts— all existing tests continue to pass, new tests passprocess.cwd()andresolve()so they work against the actual repo structure without any mocking (thesrc/directory andpackage.jsonare guaranteed to exist at the repo root)🤖 Generated with Claude Code
Summary by CodeRabbit