fix: make logging spec path assertions cross-platform (Windows CI) - #3368
Conversation
The error-jsonl-hook tests hardcoded POSIX path literals
('/logs/errors.jsonl', '/logs/main.log') in assertions. The source
builds these paths with path.join(app.getPath('logs'), ...), which
emits backslash separators on Windows, so the literal-string matches
failed on Windows CI while passing on Linux/macOS.
Construct expected paths with path.join from a shared LOGS_DIR constant
so the separator matches the platform under test.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe test file ChangesLog path constant refactor in test suite
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Problem
src/logging/main/index.main.spec.ts(added in #3365) fails on Windows CI while passing on Linux/macOS:The spec hardcoded POSIX path literals (
/logs/errors.jsonl) in assertions. The source builds paths withpath.join(app.getPath('logs'), …), which emits\separators on Windows — so the exact-string match failed. Violates the repo's cross-platform test requirement (CLAUDE.md).Fix
Test-only. Derive expected paths with
path.joinfrom a sharedLOGS_DIRconstant so the test and source use the same primitive:MAIN_LOG_PATH = path.join('/logs', 'main.log'),ERRORS_JSONL_PATH = path.join('/logs', 'errors.jsonl').chmodSyncasserts, thewriteFileSynctruncation assert, and anexistsSyncmock comparison (the last would have silently skipped the truncation branch on Windows, masking the assertion).src/logging/index.tsunchanged — it was already correct.Platform-correct by construction: both sides now produce
\logs\errors.jsonlon Windows and/logs/errors.jsonlon POSIX.Verify
npx jest src/logging/main/index.main.spec.ts→ 35 pass. Lint clean.Summary by CodeRabbit