Skip to content

Fix Windows nightly Maven ~/.m2 fallback test - #1962

Closed
Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
mainfrom
ci-janitor/maven-m2-fallback-userprofile
Closed

Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
mainfrom
ci-janitor/maven-m2-fallback-userprofile

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Nightly CI on main has been red three nights running, and one of the three failing jobs has no open fix: test (windows-latest, 1) fails in get_maven_repo_paths_home_dot_m2_fallback:

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\\.tmpvzSbL2\\.m2\\repository"]

The other two nightly failures already have open PRs: macOS test (macos-latest, 2) GOWORK is #1894, and e2e_bun_lockb is #1905.

Root cause

#1801 (ca6dc05) fixed Maven's user.home on Windows to read USERPROFILE before HOME, the way the JVM does. The test only moved HOME, so on Windows the crawler looked in the runner's real profile, found no .m2/repository there, and returned nothing. Linux and macOS read HOME first, so PR and merge-queue legs stayed green. The Windows test legs run only nightly or on dispatch.

Fix

This changes only the test. NoJvmCaches gets a with_home(path) constructor that points both HOME and USERPROFILE at the test's directory. Like new(), it clears the other Maven and JVM variables (MAVEN_OPTS, MAVEN_ARGS, M2_HOME, MAVEN_HOME, …) and restores them on drop, including when an assertion panics. Before, the hand-written restore was skipped on panic and leaked HOME into later #[serial] tests.

Both HOME-fallback tests now use it:

  • get_maven_repo_paths_home_dot_m2_fallback: this is the failing test.
  • get_maven_repo_paths_global_mode_no_m2_returns_empty: it had the same bug. It passed on the runner only because the real profile has no .m2, so it would fail on a developer's Windows machine.

No assertion changed, and no production code changed.

Proof

  • cargo test -p socket-patch-core --test crawler_maven_e2e on Linux: 28/28 pass.
  • cargo clippy -p socket-patch-core --test crawler_maven_e2e -- -D warnings is clean.
  • Windows dispatch run https://github.com/SocketDev/socket-patch/actions/runs/41363575371 succeeded. In test (windows-latest, 1), get_maven_repo_paths_home_dot_m2_fallback ... ok and get_maven_repo_paths_global_mode_no_m2_returns_empty ... ok.
  • PR checks: 55 passed, 43 skipped, 0 failed. Bugbot found no issues.

Where tests run

No test was removed or moved.

https://claude.ai/code/session_011f9aeRXDcg7ce8xkkpQdxf


Generated by Claude Code

Nightly CI on main has failed test (windows-latest, 1) every night
since 2026-10-30: get_maven_repo_paths_home_dot_m2_fallback returns
[] instead of <tmp>/.m2/repository. #1801 made Maven's user.home on
Windows read USERPROFILE before HOME, as the JVM does, but the test
only moved HOME, so the crawler looked in the runner's real profile.

Run both HOME-fallback tests under NoJvmCaches::with_home, which
points HOME and USERPROFILE at the test's directory, clears the
other Maven/JVM variables and restores everything on drop, also
when an assertion panics. The no-m2 test passed on the runner only
because its real profile has no .m2.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011f9aeRXDcg7ce8xkkpQdxf
@mikolalysenko Mikola Lysenko (mikolalysenko) added the ci-janitor Opened by the CI janitor routine (flakes, redundant tests, CI perf) label Nov 1, 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.

✅ 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 5003425. Configure here.

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

Copy link
Copy Markdown
Collaborator Author

Ready for review at head 50034252.

  • CI: 55/55 non-skipped checks green (43 skipped), 0 failed. The Windows legs were checked separately in the dispatch run linked in the description.
  • Bugbot reviewed 50034252: no findings.
  • Test-only change: NoJvmCaches::with_home sets both HOME and USERPROFILE and restores them on drop. No production code changes.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does: Test-only fix for the Windows nightly failure in get_maven_repo_paths_home_dot_m2_fallback. Since #1801 the Maven crawler reads USERPROFILE before HOME on Windows (as the JVM does), but the test only moved HOME. A new NoJvmCaches::with_home guard moves both and restores every variable on drop, panics included.

Risk: low. Only crates/socket-patch-core/tests/crawler_maven_e2e.rs changes, no production code and no assertion changes. The second test (global_mode_no_m2_returns_empty) now also clears the Coursier/Ivy cache variables, which makes it more hermetic, not less.

Look here

Verified: read the full diff and the guard's key list. cargo test -p socket-patch-core --test crawler_maven_e2e: 28/28 pass locally. The PR's Windows dispatch run (41363575371) passes both tests. CI 55 passed / 43 skipped / 0 failed; Bugbot passed.

Changes I made: none.

Open questions: none blocking. Nit: MAVEN_REPO_LOCAL is still saved and restored by hand, so a panic skips that restore. It could go in the guard's key list in a follow-up.

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


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

On the MAVEN_REPO_LOCAL nit: I agree. I left it out of this PR on purpose. Adding it to the NoJvmCaches key list also changes the three other tests that use the guard, and the fix here is meant to touch only what the Windows failure needs. It is a good follow-up and does not block this PR, so I am not pushing again here; a new commit would re-run CI and drop the review.


Generated by Claude Code

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[agent] Reviewed 5003425210b63af9fd9d88d151439ccc8c1e3af0. Checked the HOME/USERPROFILE guard and restoration against Maven home resolution. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.

@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Nov 2, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Nov 2, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Closing as superseded by #1852, which merged at 15:49 UTC and fixes the same Windows nightly failure in the same way: both home-fallback tests now set HOME and USERPROFILE. That is why the merge queue removed this PR with a merge conflict.

What this PR still adds on top is minor: the variables are restored even when an assertion fails, and the other JVM variables are cleared. It is not worth a separate merge. I missed #1852 when checking for duplicate open PRs because I read only the first 50 open PRs.


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.

3 participants