Skip to content

Fix Maven home tests failing on Windows nightly - #1852

Open
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
mainfrom
ci-janitor/maven-home-windows-test
Open

Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
mainfrom
ci-janitor/maven-home-windows-test

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The Windows test shard 1 is red on main. The 2026-10-29 nightly failed both Windows shards; #1814 and #1816 fixed what was failing then, so I re-ran a Windows-only dispatch on main at c07af61 to check: run 40942139555. Shard 2 and every Windows e2e leg pass now. Shard 1 still fails, on a new test:

---- get_maven_repo_paths_home_dot_m2_fallback stdout ----
panicked at crates\socket-patch-core\tests\crawler_maven_e2e.rs:297:5:
assertion `left == right` failed: HOME/.m2/repository fallback must be the sole discovered repo
  left: []
 right: ["C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\.tmpSAbxln\\.m2\\repository"]

Without this fix the next nightly on main will be red on Windows again.

Root cause

#1801 (merged 13:26 UTC today) changed the Maven local-repository lookup to read user.home the way Java does: USERPROFILE first on Windows, HOME elsewhere. Two home-fallback tests in crawler_maven_e2e.rs still set only HOME to their temp dir. On Windows the crawler therefore read the runner's real profile directory, which has no .m2. get_maven_repo_paths_global_mode_no_m2_returns_empty has the same bug but passes only because that runner has no real ~/.m2.

This is the same class of bug as #1814 (cargo home tests after #1779).

Fix

Point both HOME and USERPROFILE at the temp dir in both home tests, and restore both afterwards. Maven's user.home prefers USERPROFILE on Windows, while the Coursier and Ivy lookups that --global also walks prefer HOME everywhere. Bugbot pointed out that redirecting only the platform's variable, as #1814 did for cargo, would leave the second one leaking. Only test code changes; no assertion is weakened.

Proof

  • Linux: cargo test -p socket-patch-core --test crawler_maven_e2e gives 28/28 passing, and the same 28/28 on 30 looped runs. cargo clippy -p socket-patch-core --test crawler_maven_e2e -- -D warnings is clean, and so is rustfmt --check on the touched file.
  • Windows: Windows-only dispatches on this branch are green, including test (windows-latest, 1), which failed on main: run 40946507476 (first commit) and run 40952032589 (final commit).
  • PR CI is green on the final head. Both commits lost Linux e2e legs to a Depot runner outage ("self-hosted runner lost communication", see Depot runners lost mid-job evicted 2 merge-queue entries in 27h #1660). One re-run of the failed jobs passed each time.

Where tests run

No test is removed or moved.


Generated by Claude Code


Note

Low Risk
Test-only environment setup in Maven e2e tests; no production crawler or assertion changes.

Overview
Fixes Windows CI failures in Maven crawler e2e tests after production started resolving user.home like Java (USERPROFILE before HOME on Windows).

Two serial tests in crawler_maven_e2e.rs that isolate home-based .m2/repository discovery previously set only HOME to a temp directory. On Windows the crawler still read the real profile, so get_maven_repo_paths_home_dot_m2_fallback could see an empty repo list instead of the staged path.

The change introduces a shared HOME_VARS list and sets and restores both HOME and USERPROFILE in those tests, matching the existing NoJvmCaches helper and avoiding leaks from Coursier/Ivy paths that still use HOME everywhere. Assertions are unchanged; this is test harness only.

Reviewed by Cursor Bugbot for commit c8f2314. Configure here.


Generated by Claude Code

#1801 made the Maven local-repository lookup read user.home the way
Java does: USERPROFILE on Windows, HOME elsewhere. The two
crawler_maven_e2e home-fallback tests still pointed only HOME at
their temp dir, so on Windows the crawler looked in the runner's real
profile and get_maven_repo_paths_home_dot_m2_fallback failed in the
Windows test shard on main. The no-.m2 test passed there only because
the runner has no real ~/.m2.

Point the platform's home variable at the temp dir instead, as #1814
did for the cargo home tests.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011f9aePZNoNmNU3zNg5Latx
@mikolalysenko Mikola Lysenko (mikolalysenko) added the ci-janitor Opened by the CI janitor routine (flakes, redundant tests, CI perf) label Oct 29, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

The global-mode test also walks the Coursier and Ivy caches, which
read HOME first on every OS. Redirecting only USERPROFILE on Windows
would let the runner's real HOME leak into its empty-list assertion.
Point both variables at the temp dir in both home tests, and restore
both afterwards.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011f9aePZNoNmNU3zNg5Latx
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit c8f2314. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 29, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review at c8f23147814707ad374990fe5160b11ae9a245af.

  • CI: 55/55 run checks green (43 skipped), mergeable, no conflicts.
  • Bugbot: the earlier finding on 10bac33 was fixed and its thread resolved; reviewed c8f2314 with no new findings.
  • Reviewer note: test-only change in crates/socket-patch-core/tests/crawler_maven_e2e.rs to make the Maven home tests pass on Windows nightly.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Labeled Ready for review at c8f2314.

  • CI: all 98 checks on c8f2314 passed or were skipped (55 success, 43 skipped), including the Windows dispatch run.
  • Bugbot: reviewed c8f2314. Its one earlier finding was fixed and the thread is resolved, so no threads are open.
  • Reviewer note: this changes only tests. The two ~/.m2 fallback tests in crawler_maven_e2e.rs now point both HOME and USERPROFILE at the temp dir and restore both afterwards. On Windows, Maven's user.home reads USERPROFILE.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does: A test-only fix for the Windows nightly. #1801 made Maven's user.home prefer USERPROFILE on Windows, but two #[serial] home-fallback tests in crawler_maven_e2e.rs still redirected only HOME. They now set both variables to the temp dir and restore both afterwards, removing either one if it was unset before.

Risk: low. Only test code changes. Restore is correct for previously-unset variables, and there is no race: the other tests in the file are #[serial_test::parallel] and never overlap a #[serial] test.

Look here:

Verified: I read the diff. CHANGELOG.md is untouched. CI is green on c8f2314 (55 passed, 43 skipped, ci-ok and clippy both green). Bugbot found no new issues.

Changes I made: none.

Open questions: none. Optional follow-up: the existing NoJvmCaches drop guard in the same file (lines 31-71) could replace the hand-written restore. It would also clear LOCALAPPDATA and COURSIER_CACHE, and it would restore the environment on panic.

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-janitor Opened by the CI janitor routine (flakes, redundant tests, CI perf) Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants