test(windows): make path assertions and chmod tests OS-aware#1378
Merged
Conversation
Six unit tests fail on Windows CI because they encode POSIX-only assumptions: - tests/unit/runtime/test_runtime_utils.py * test_skips_non_executable_apm_binary * test_apm_binary_without_execute_permission_falls_back Both rely on chmod(0o644)/chmod(0o600) making the APM-managed binary non-executable so find_runtime_binary() falls back to PATH. Windows ignores POSIX execute bits; os.access(path, os.X_OK) returns True for any readable file, so the fallback branch is unreachable. Skip both on win32 with a reason that documents the platform limitation. - tests/unit/integration/test_hook_integrator.py Four tests assert that rewritten hook commands contain forward-slash paths like '.claude/hooks/hookify/hooks/pretooluse.py'. When deploy_root is set, _rewrite_command_for_target() emits absolute paths via Path.resolve(), which uses native separators (backslash on Windows). This is the correct production behaviour -- Claude Code on Windows expects Windows paths -- so normalise the assertion side instead of the code under test by replacing backslashes before the substring/endswith check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Windows-only unit test failures by making test assertions OS-aware, without changing production code. It aligns runtime permission tests with Windows semantics and makes hook-integrator path assertions resilient to native path separators.
Changes:
- Skip two
chmod/execute-bit-based runtime tests onwin32, documenting why the branch is unreachable on Windows. - Normalize command-path separators in four hook-integrator tests by replacing backslashes before substring/
endswithassertions.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/runtime/test_runtime_utils.py | Skips POSIX execute-bit dependent tests on Windows to match os.access(X_OK) behavior. |
| tests/unit/integration/test_hook_integrator.py | Normalizes path separators in assertions so rewritten hook commands pass on Windows and POSIX. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 0
sergio-sisternes-epam
pushed a commit
that referenced
this pull request
May 19, 2026
Six unit tests fail on Windows CI because they encode POSIX-only assumptions: - tests/unit/runtime/test_runtime_utils.py * test_skips_non_executable_apm_binary * test_apm_binary_without_execute_permission_falls_back Both rely on chmod(0o644)/chmod(0o600) making the APM-managed binary non-executable so find_runtime_binary() falls back to PATH. Windows ignores POSIX execute bits; os.access(path, os.X_OK) returns True for any readable file, so the fallback branch is unreachable. Skip both on win32 with a reason that documents the platform limitation. - tests/unit/integration/test_hook_integrator.py Four tests assert that rewritten hook commands contain forward-slash paths like '.claude/hooks/hookify/hooks/pretooluse.py'. When deploy_root is set, _rewrite_command_for_target() emits absolute paths via Path.resolve(), which uses native separators (backslash on Windows). This is the correct production behaviour -- Claude Code on Windows expects Windows paths -- so normalise the assertion side instead of the code under test by replacing backslashes before the substring/endswith check. Co-authored-by: Daniel Meppiel <copilot-rework@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
Problem
Windows CI on
mainis red (run 26061585552). Linux and macOS pass — only the Windows job fails, with six unit-test failures that all encode POSIX-only assumptions:Both groups are test-only defects; production code behaves correctly on Windows.
Root cause
Runtime tests (2) — both call
chmod(0o644)/chmod(0o600)on the APM-managed binary expectingfind_runtime_binary()to fall back toshutil.which()because the file is "not executable". Windows ignores POSIX execute bits:os.access(path, os.X_OK)returnsTruefor any readable file, so the fallback branch is unreachable and the test asserts the wrong path.Hook integrator tests (4) — all assert forward-slash substrings such as
".claude/hooks/hookify/hooks/pretooluse.py" in cmd. Whendeploy_rootis set,_rewrite_command_for_target()emits absolute paths viaPath.resolve(), which uses native separators (backslash on Windows). This is the correct production behaviour — Claude Code / Cursor on Windows expect Windows paths — so the fix is on the assertion side.Fix
chmod-based runtime tests onwin32with a reason documenting the platform limitation. Other tests in the same class already use@pytest.mark.skipif(sys.platform != "win32", ...)for the inverse case, so the pattern is established.cmd.replace("\\", "/")) before the substring /endswithcheck in the four hook-integrator tests. The production rewriter is untouched.No
src/changes — six test-only edits, 17 lines added / 7 removed.Validation
Locally:
Both CI-mirror lint commands are silent per
.apm/instructions/linting.instructions.md. Windows CI on this branch will confirm the six previously-failing tests now pass / skip cleanly.