Skip to content

Fix uv settings test failing on Windows nightly - #1663

Merged
Mikola Lysenko (mikolalysenko) merged 2 commits into
mainfrom
ci-janitor/uv-settings-test-windows
Oct 22, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 2 commits into
mainfrom
ci-janitor/uv-settings-test-windows

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

Problem

The nightly CI run on main is red. Run 39730006970, job test (windows-latest, 1):

---- patch::redirect::upstream::uv::tests::global_uv_settings_reads_the_user_then_first_system_file stdout ----
panicked at crates\socket-patch-core\src\patch\redirect\upstream\uv.rs:2389:9:
assertion `left == right` failed
  left: ["C:\\...\\.tmp1nilbo\\xdg\\uv\\uv.toml"]
 right: ["C:\\...\\.tmp1nilbo\\xdg/uv/uv.toml", "C:\\...\\.tmp1nilbo\\dirs2/uv/uv.toml"]
test result: FAILED. 6220 passed; 1 failed

The test landed yesterday in #1614. Windows unit tests run only on the nightly schedule, so PR and merge-queue CI never ran it. The failure is deterministic, not a flake, and it will fail every nightly until fixed.

Root cause

The test joins its XDG_CONFIG_DIRS entries with std::env::join_paths, which uses ; on Windows. global_uv_settings(.., windows = false) models uv's Unix lookup and splits on :. On Windows the joined value also contains drive letters (C:\...), so no entry resolves to the fixture directory and only the user file is found. The production code is correct: on Windows it calls global_uv_settings(.., cfg!(windows)), which reads %APPDATA% and %PROGRAMDATA% and never takes the XDG path.

Fix

The fix is in the test only:

  • It builds the list with :, the separator the code under test uses.
  • It sets XDG_CONFIG_DIRS, and expects the dirs2 system file, only on Unix. Unix is the only place that lookup is used, and a : list can't hold Windows paths.
  • On Windows the test still checks the XDG_CONFIG_HOME user file and the %APPDATA%/%PROGRAMDATA% branch.

No assertion is weakened on Unix, and the test keeps running everywhere it ran before (PR/merge-queue Linux, nightly Windows/macOS).

Proof

  • On Linux, cargo test -p socket-patch-core --lib global_uv_settings passes.
  • rustfmt was run on the touched file only.
  • Bugbot found that the expected paths were display strings of root.join("xdg/uv/uv.toml"). On Windows those keep their /, while the function builds paths with native \ separators, so the assertion would still fail. It was right; the original failure output shows the same mismatch. 481872b builds every expected path one segment at a time, the same way the function does.
  • On Windows, XDG_CONFIG_DIRS is no longer set. The function falls back to the default /etc/xdg, which is root-relative on the runner's D: drive and doesn't exist, so the result is the user file alone. That matches expected. A local Windows run isn't possible here, so the next nightly, or a workflow_dispatch of CI on this branch, confirms it.

Generated by Claude Code


Note

Low Risk
Test-only changes to env setup and path expectations; no production uv settings lookup logic is modified.

Overview
Fixes the nightly Windows failure in global_uv_settings_reads_the_user_then_first_system_file without changing production behavior.

The test no longer builds XDG_CONFIG_DIRS with join_paths (; on Windows) while asserting Unix-style global_uv_settings(.., false) lookup, which splits on :. Unix-only cases now use colon-separated dirs and still expect the first system uv.toml under dirs2. On Windows the test skips that XDG system branch and keeps checking user config plus %APPDATA% / %PROGRAMDATA%.

Expected paths are built via a new uv_toml helper that joins path segments like the implementation, so assertions use native separators instead of mixed / in root.join("…") display strings.

Reviewed by Cursor Bugbot for commit 481872b. Configure here.


Generated by Claude Code

global_uv_settings_reads_the_user_then_first_system_file joined its
XDG_CONFIG_DIRS entries with std::env::join_paths, which uses `;` on
Windows, while global_uv_settings(.., windows = false) models uv's
Unix lookup and splits on `:`. On Windows the joined list also holds
drive letters (`C:\...`), so no entry resolved and the test found
only the user file. Windows unit tests run only nightly, so the test
landed green in #1614 and has turned the nightly CI on main red.

Build the list with `:` and only add it, and expect the system file,
on Unix, where that lookup is used. Windows keeps checking the
XDG_CONFIG_HOME user file and the %APPDATA%/%PROGRAMDATA% branch.

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

Comment thread crates/socket-patch-core/src/patch/redirect/upstream/uv.rs
The test compared display strings of root.join("xdg/uv/uv.toml")
against paths global_uv_settings builds one segment at a time, so on
Windows the expected side kept its forward slashes and every
assertion of a found path would still fail (Bugbot on #1663). Build
the expected paths segment by segment too.

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

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@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 481872b. Configure here.

@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 481872b. Configure here.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

test-release (2) failed in e2e_redirect_cargo_shapes::cargo_hosted_refuses_a_lockless_project_with_other_dependencies (job). This PR changes only a unit test in uv.rs, so the failure is not from this PR.

The test's setup step cargo update -p cfg-if@1.0.5 --precise 1.0.4 exited non-zero. The helper discards cargo's stderr, so the log doesn't say why. Its sibling cargo_hosted_refuses_a_crate_another_crate_depends_on ran the same pin step in the same run and passed, which points to a crates.io or index-fetch blip. #1654 raises CARGO_NET_RETRY for this exact helper (cargo_e2e_matrix::cargo_command) on CI. I'm not porting it here: the cause isn't confirmed, and porting it would touch #1654's file. If this recurs, the next step is to include cargo update's stderr in that assert. I'm re-running the failed job once.


Generated by Claude Code

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

Copy link
Copy Markdown
Collaborator Author

Ready for review at 481872b.

  • CI: 53/53 checks green (34 success, 19 skipped). No main-wide failures.
  • Bugbot: reviewed 481872b, no new findings. Its one earlier finding (expected paths kept / separators, so the Windows comparison would still fail) was fixed in 481872b and the thread is resolved.
  • Mergeable, no conflicts; no CHANGELOG.md change. Test-only diff in upstream/uv.rs; cargo test -p socket-patch-core --lib global_uv_settings passes on Linux.
  • Reviewer note: PR CI doesn't run Windows unit tests, so the Windows leg this fixes is unproven until the next nightly (or a workflow_dispatch of CI on this branch).
  • Slack: not announced. This run had no Slack send tool, so the next run will retry.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does: Fixes the nightly Windows failure in global_uv_settings_reads_the_user_then_first_system_file (added in #1614). The test built XDG_CONFIG_DIRS with join_paths (; on Windows), but the Unix lookup splits on :. It also compared against root.join("xdg/uv/uv.toml") strings, which keep / on Windows. The change is test-only. Production code is untouched.

Risk: low. One test in one file changed, and the Unix assertions are unchanged in strength. Windows unit tests run only nightly, so the Windows leg is unproven until the next nightly.

Look here:

  • uv.rs:2383:`` uv_toml helper builds expected paths one segment at a time, as the function does at uv.rs:867
  • uv.rs:2397-2407:```` XDG_CONFIG_DIRS and the `dirs2` expectation now apply on Unix only
  • uv.rs:876-884:````` the : split in the code under test. On Windows the default `/etc/xdg` is drive-relative and never exists, so expecting the user file alone is correct

Verified:

  • Read the full diff. Only uv.rs changes, and CHANGELOG.md is untouched.
  • Walked through the rest of the test on Windows by hand. The dirs4:dirs1::dirs2 case still passes there: splitting on the drive letter's : yields only paths that don't exist, and the empty entry stops the list.
  • cargo test -p socket-patch-core --lib global_uv_settings passes on Linux.
  • CI: ci-ok and clippy are green on 481872b, and Cursor Bugbot passes. Its one thread (native separators) is fixed and resolved.

Changes I made: none.

Open questions: none. If you want the Windows leg proven before merging, run a workflow_dispatch of CI on this branch.

Auto-merge is armed: approving sends this PR straight to the merge queue.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

The Windows leg is now confirmed. In the workflow_dispatch CI run on this branch (39745825228), test (windows-latest, 1) and test (windows-latest, 2) both passed. test-release (2) also passed on its one re-run, so its earlier failure was the crates.io blip.

That run's ci-ok is still red. The only cause is e2e-docker (sbt), which was cancelled at its 45-minute timeout on both attempts. This PR is not the cause:

  • That job runs only on schedule and dispatch, never on PR or merge-queue CI.
  • In both attempts it spent the whole budget building the sbt Docker image with no cache. The last log lines before the cancel are sbt 1.2.8 warming at Updating ProjectRef(... warm-1-2-8-build), stalled for about 18 minutes.
  • On main's nightly, with a warm cache, the same job takes about 6 minutes (2026-10-21: 05:52 to 05:59).

This PR touches nothing Docker or sbt related, and no fix for the cold-cache stall exists yet. I'm recording it as a CI reliability item. The pull_request run's ci-ok on this head is green (39739005695).


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final reviewer: disarmed auto-merge on 481872b. ci-ok on this head is now red. The cause is the workflow_dispatch CI run 39745825228, attempt 2. Its e2e-docker (sbt) job hung in "Build sbt image" for 45 minutes (09:02 to 09:47 UTC) and was cancelled. This PR only changes a test in upstream/uv.rs, so that hang is infra, not this PR. The pull_request run on the same head is green (ci-ok and clippy).

That run was already re-run once (attempt 2), so I haven't re-run it again. CI janitor: the sbt image build hang needs a look. Once ci-ok is green on this head again, the next final-review pass re-arms auto-merge. The review brief above still applies, since the code is unchanged.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Auto-merge was turned off, apparently because the newest ci-ok on this head was the red one from the manual workflow_dispatch run. That run's only failure is the nightly-only e2e-docker (sbt) timeout, explained above. I've re-run the ci-ok job of this PR's own pull_request run (39739005695). That job's result was already green, and the re-run makes the PR gate the newest ci-ok again. Once it's green, the final-review routine can re-arm auto-merge.


Generated by Claude Code

Merged via the queue into main with commit 55d52e0 Oct 22, 2026
588 of 594 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the ci-janitor/uv-settings-test-windows branch October 22, 2026 19:29
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