test: make auto-run-command and attachments tests OS-agnostic#46
Merged
chr1syy merged 1 commit intoJun 7, 2026
Merged
Conversation
Four tests in the suite failed on Windows but passed on Linux/macOS,
encoding platform assumptions in the test data and assertions:
- src/__tests__/auto-run-command.test.ts:77, 104, 131 — three tests
mocked autoRunFolderPath as '/agents/auto-run-docs' and asserted
the result of path.join(folder, doc). On Linux that yields
'/agents/auto-run-docs/plan.md'; on Windows, path.isAbsolute('/...')
is true so the resolved path is 'C:\\\\agents\\\\auto-run-docs\\\\...',
and path.join without resolve produces '\\\\agents\\\\...' (no
drive). Production calls path.resolve first, so the test's
expected value drifted from the actual on Windows.
- src/__tests__/attachments.test.ts:80 — checked
savedPath.includes(DEFAULT_FILES_SUBDIR) where the constant is the
forward-slash literal '.maestro/discord-files'. path.join on
Windows produces backslashes, so the substring never matches.
Fixes:
- Replace the mocked folder with a constant built from path.join,
giving a relative path that path.isAbsolute returns false for on
every platform. The absolute-path test inputs are built with
path.resolve against the same relative folder so they round-trip
identically.
- Compute expected docs values via resolveContainedDocPath, the same
OS-agnostic helper production uses, so the assertion is exact
regardless of how path.resolve normalizes the CWD prefix.
- The /etc/passwd test now uses path.resolve(os.tmpdir(), 'evil.txt')
for a portable 'guaranteed outside the folder' absolute path.
- For the DEFAULT_FILES_SUBDIR assertion, normalize the constant to
path.sep before the .includes() check.
Verified on Windows: 211 pass, 0 fail (was 207/4 on main). Production
code untouched.
|
PR changed again? Review this PR in Change Stack to compare snapshots and stay oriented. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR removes platform-specific path handling from two test files. The attachment test now normalizes path separators before assertions. The auto-run command tests replace hardcoded absolute paths with a mock folder constant and use production path-resolution helpers, ensuring consistent behavior across Windows and Unix-like systems. ChangesCross-Platform Test Path Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
This was referenced Jun 6, 2026
chr1syy
added a commit
that referenced
this pull request
Jun 7, 2026
Bundles four merged PRs since v0.2.0: - #46 OS-agnostic auto-run + attachments tests (Windows CI fix) - #47 typed bridge errors + `Retry-After` header on 429 - #48 leveled logger consolidation + `LOG_LEVEL` env var - #49 wallclock ISO timestamps on console output (macOS launchd gap) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@
Summary
Fixes 4 test failures that occur on Windows due to POSIX-only path assumptions in the test suite. No production code changes - tests only.
attachments.test.ts: normalize the expectedDEFAULT_FILES_SUBDIRsubstring to the platform separator (path.sep) before the containment check.auto-run-command.test.ts: build mock folders and fixtures withpath.join/os.tmpdir()instead of hardcoded/agents/...literals, so absolute/relative/traversal cases resolve consistently on every OS.Merge order
Merge this first. The other two open PRs (
fix/typed-bridge-errors,chore/logger-consolidation) still show these same 4 pre-existing failures on Windows; landing this branch makes the suite fully green for all three.Test plan
npm test-> 211 pass / 0 fail on Windows 11tsc)@
Summary by CodeRabbit