Skip to content

bugfix(ci): run rust mock + live tests in separate processes (#241 unblock)#253

Merged
gerchowl merged 1 commit intodevfrom
bugfix/241-rust-live-isolation
Apr 30, 2026
Merged

bugfix(ci): run rust mock + live tests in separate processes (#241 unblock)#253
gerchowl merged 1 commit intodevfrom
bugfix/241-rust-live-isolation

Conversation

@gerchowl
Copy link
Copy Markdown
Contributor

Summary

The Rust client suite mixes httpmock-based unit tests (which set `MAT_VIS_HF_BASE` to a `127.0.0.1` server) with `@live` tests that need to hit real HF. Even with `--test-threads=1` the env var leaks across tests in the same process, so the live tests 404 on `http://127.0.0.1:/v2026.04.2/release-manifest.json` instead of `huggingface.co`.

Surfaced in CI when #248's manifest-driven coverage suite started actually firing the `@live` blocks (run 25173722342). All 4 Rust live tests fail with the localhost URL.

This is the root cause of #241 (originally reported as SIGABRT in live tests after http unit tests; the SIGABRT was an earlier symptom of the same shared-state problem).

Fix (CI-side unblock)

Run mock and live tests as two separate cargo invocations in the dagger `test_client_rust` fn, so env-var pollution from the mock tests can't bleed into live. Both invocations reuse the same cargo registry + target caches; second compile is incremental.

Proper fix (a `serial_test` annotation or `scopeguard` restoring `MAT_VIS_HF_BASE` after each mock test) still lives in #241.

Test plan

  • yamllint + pre-commit clean
  • Post-merge CI `client-tests` job's Rust live blocks pass against `v2026.04.2` (proves the isolation works)

…block)

The Rust client suite mixes httpmock-based unit tests (which set
MAT_VIS_HF_BASE to a 127.0.0.1 server) with @LiVe tests that need to
hit real HF. Even with --test-threads=1 the env var leaks across
tests in the same process, so the live tests 404 on
http://127.0.0.1:<port>/v2026.04.2/release-manifest.json instead of
huggingface.co.

This is the root cause of #241 (originally reported as SIGABRT in
live tests after http unit tests; the SIGABRT was an earlier
symptom of the same shared-state problem).

The proper fix (a serial_test annotation or scopeguard restoring
MAT_VIS_HF_BASE) belongs in the Rust test code; that's still on
#241. Until then, this is a CI-side unblock: run mock and live as
two separate cargo invocations so env-var pollution can't bleed.

Both invocations reuse the same cargo registry + target caches, so
the second compile is incremental — wallclock cost is minimal.
@gerchowl gerchowl enabled auto-merge (squash) April 30, 2026 15:25
@github-actions github-actions Bot added the area:baker Baker pipeline, Dagger, data fetchers label Apr 30, 2026
@gerchowl gerchowl merged commit 7cdc062 into dev Apr 30, 2026
4 checks passed
gerchowl added a commit that referenced this pull request Apr 30, 2026
…e state leak (#241) (#254)

The Rust client suite mixes httpmock-based unit tests (which set
MAT_VIS_HF_BASE to a 127.0.0.1 mock server) with @LiVe tests that
need to hit real HF. Even with `cargo test -- --test-threads=1` the
env var leaked across tests in the same process — live tests then
404'd on http://127.0.0.1:<port>/v2026.04.2/release-manifest.json.

Fix: an in-tree `EnvGuard` RAII helper (Option C — no new dev dep).
Each mock test stores the prior value of MAT_VIS_HF_BASE, sets the
mock URL, and restores on drop. Live tests then see the original
(unset) state regardless of test ordering.

`ENV_LOCK` stays so the suite is race-safe under default (parallel)
`cargo test` — `std::env::set_var` is `unsafe` in 2024 edition. The
existing hand-rolled restore in `e2e_fetch_color_png` and
`resolved_tag_falls_back_to_default` is replaced by `EnvGuard` /
panic-safe restoration.

Verified: `MAT_VIS_LIVE_TESTS=1 MAT_VIS_LIVE_TAG=v2026.04.2 cargo
test -- --test-threads=1` passes all 21 tests (4 live) cleanly in a
single process, no SIGABRT, no 127.0.0.1 leakage into live URLs.

Also rolls back the #253 dagger-side workaround
(`test_client_rust` dual cargo invocation): with proper test-side
isolation, the single-invocation form is correct again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:baker Baker pipeline, Dagger, data fetchers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant