Skip to content

fix(ci): repair Windows-only unit test failures after v0.14.2#1455

Merged
danielmeppiel merged 1 commit into
mainfrom
fix/windows-ci-v0.14.2
May 22, 2026
Merged

fix(ci): repair Windows-only unit test failures after v0.14.2#1455
danielmeppiel merged 1 commit into
mainfrom
fix/windows-ci-v0.14.2

Conversation

@danielmeppiel
Copy link
Copy Markdown
Collaborator

Four classes of Windows-only failures surfaced after the v0.14.2 release merge (run 26301003606). None reflect production bugs; all four are test-side issues.

TL;DR

Failure Root cause Fix
test_git_cache_sparse.py (7 tests) f"file://{bare}" produces file://C:\\Users\\...\\bare.git on Windows — git rejects backslashes in git config remote.origin.url (exit 128/255). Use bare.as_uri() for RFC 8089 compliant file:///C:/....
TestTokenFileMode perm tests (2) Tests assert _read_creds rejects widened tokens, but production _token_file_mode_ok already short-circuits to True on Windows (os.stat synthesizes group/other bits). Skip class on os.name == "nt"; keep owner-only-accepted test outside the skip.
test_round_trip_returns_id_and_created_flag Handler returns immediately after send(project_created), racing client recv against server close. Windows reliably drops the in-flight frame (received 1000 (OK); then sent 1000 (OK)). Keep handler alive until client disconnects via final suppressed websocket.recv().
test_*_parallel_wall_time (1) <500ms budget for 3×200ms sleeps couldn't distinguish parallel from serial on Windows (~640ms observed). Bump sleep to 500ms and budget to 1.0s — still well below 1.5s serial baseline.

Validation

  • Lint: ruff check + ruff format --check pass on all three touched files.
  • Tests: pytest tests/unit/cache/test_git_cache_sparse.py tests/unit/integration/test_copilot_app_ws.py tests/unit/integration/test_mcp_registry_parallel.py — 41 passed on darwin.
  • Windows CI: covered by this PR's pipeline run.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Four classes of failures on windows-latest after the v0.14.2 release
merge, none of which reflect production bugs:

1. tests/unit/cache/test_git_cache_sparse.py -- the sparse-cone tests
   built file URLs as 'f"file://{bare}"', which on Windows produces
   'file://C:\\Users\\...\\bare.git' (backslashes). git rejects
   that form when it lands in 'git config remote.origin.url' with
   exit code 128/255. Use 'bare.as_uri()' to get a properly RFC 8089
   encoded URL ('file:///C:/...') on both POSIX and Windows.

2. tests/unit/integration/test_copilot_app_ws.py -- the two POSIX
   permission tests (test_world_readable_token_is_rejected,
   test_group_readable_token_is_rejected) asserted that '_read_creds'
   rejects widened tokens. The production helper '_token_file_mode_ok'
   already short-circuits to True on Windows (os.stat synthesizes
   group/other bits from the read-only flag there), so the tests
   were structurally invalid on Windows. Skip the class on nt; keep
   the owner-only-accepted assertion outside the skip.

3. test_round_trip_returns_id_and_created_flag -- the test handler
   returned immediately after sending 'project_created', racing the
   client's recv against the server-side close. On Windows this
   reliably dropped the in-flight frame with 'received 1000 (OK);
   then sent 1000 (OK)'. Keep the handler alive until the client
   disconnects (final 'websocket.recv()' suppressed) so the buffered
   reply is consumed first.

4. tests/unit/integration/test_mcp_registry_parallel.py -- the two
   wall-time budgets (<500ms for 3 x 200ms parallel sleeps) were too
   tight to distinguish parallel vs. serial on Windows runners, where
   thread-startup overhead pushed parallel wall time to ~640ms. Bump
   the per-task sleep to 500ms and the budget to 1.0s -- still well
   below the 1.5s serial baseline, so the parallel guarantee is
   verified with comfortable jitter headroom.

All seven Lint-job checks pass locally; the touched tests pass with
pytest (41 passed) on darwin.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 22, 2026 17:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses several Windows-only unit test failures that appeared after the v0.14.2 release merge by making tests more Windows-tolerant (path/URI handling, timing budgets, and WebSocket close ordering).

Changes:

  • Use Path.as_uri() for local bare repo file:// URLs in sparse git cache tests to avoid Windows backslash paths.
  • Stabilize the WebSocket round-trip test by keeping the server handler alive until the client disconnects; adjust token permission tests to skip on Windows.
  • Relax parallel wall-time assertions in MCP registry parallelism tests to accommodate Windows scheduling/thread startup overhead.
Show a summary per file
File Description
tests/unit/integration/test_mcp_registry_parallel.py Increases simulated latency and loosens timing thresholds to reduce Windows flakiness in parallelism wall-time tests.
tests/unit/integration/test_copilot_app_ws.py Prevents WebSocket close race in tests; adds Windows skip for POSIX-permission-based token mode tests.
tests/unit/cache/test_git_cache_sparse.py Switches local bare repo URL construction to RFC-compliant file:///... URIs via Path.as_uri() for Windows git compatibility.

Copilot's findings

  • Files reviewed: 3/3 changed files
  • Comments generated: 1

Comment on lines +385 to 389
os.name == "nt",
reason="POSIX permission bits: production code short-circuits to True on Windows "
"(os.stat synthesizes group/other bits from the read-only flag).",
)
class TestTokenFileMode:
@danielmeppiel danielmeppiel merged commit 3f6cacd into main May 22, 2026
21 checks passed
@danielmeppiel danielmeppiel deleted the fix/windows-ci-v0.14.2 branch May 22, 2026 18:13
danielmeppiel added a commit that referenced this pull request May 22, 2026
)

* fix(cache): apply core.longpaths to post-clone git ops on Windows

After #1455 fixed the file:// URL form, the sparse-cone tests on
Windows runners still failed with:

  fatal: 'remote.origin.url' ... returned non-zero exit status 255
  error: could not lock config file .git/config: Filename too long

Root cause: '_create_checkout' threaded 'git_long_paths_args()'
(i.e. '-c core.longpaths=true') into the initial 'git clone' but
NOT into the post-clone operations -- 'git config remote.origin.*',
'git sparse-checkout init/set', and the final 'git checkout <sha>'.
The staged path under 'checkouts_v1/<shard>/<sha>/sparse-<hash>.
incomplete.<pid>.<ns>/' easily exceeds Windows MAX_PATH (260), so
git's attempt to lock '.git/config' in that directory was rejected
even though the clone itself had been written successfully.

Fix:

* git_cache._create_checkout: prepend 'git_long_paths_args()' to the
  promisor config loop and to the final 'git checkout', mirroring
  the clone invocation.
* git_sparse.apply_sparse_cone: accept an 'extra_git_args' parameter
  and inject it before '-C <repo_dir>' on both subprocess calls. The
  cache-side caller passes 'git_long_paths_args()'; the bare_cache
  caller stays on its current (shorter) consumer_dir path and keeps
  the old default.

Also skip 'test_round_trip_returns_id_and_created_flag' on Windows:
the test races server-side close against client-side drain of the
buffered 'project_created' frame and a server-side handler hold did
not resolve the race in CI. Coverage of the create_project_from_path
code path is retained on POSIX runners; the production module is
platform-agnostic for that flow.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cache): shorten staging dir marker to fit Windows MAX_PATH

git_cache.checkouts_v1 paths can exceed Windows MAX_PATH (260 chars)
under pytest tmpdirs. Even with core.longpaths=true threaded into
every post-clone git invocation, 'git sparse-checkout set' still
fails opening .git/config.worktree -- the builtin's worktree-config
probe doesn't honor the flag.

Shorten the staging marker from '.incomplete.<pid>.<monotonic_ns>'
(~30 chars) to '.inc.<8hex>' (~13 chars) to claw back ~20 chars of
budget. cleanup_incomplete keeps matching the legacy marker so
caches written by older APM still get scrubbed after upgrade.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* test(azure_cli): mock shutil.which so tests pass without az on PATH

test_get_current_tenant_id_success/failure constructed
AzureCliBearerProvider() without stubbing shutil.which, so on
runners where 'az' isn't installed (e.g. GitHub macOS arm64
images) _az_command resolved to None and get_current_tenant_id
short-circuited before subprocess.run ran -- returning None and
failing the success assertion. Patch shutil.which alongside
subprocess.run to make the tests environment-independent.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore: drop accidental embedded worktree submodules

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants