Skip to content

bugfix(client/rust): isolate MAT_VIS_HF_BASE per-test (#241)#254

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

bugfix(client/rust): isolate MAT_VIS_HF_BASE per-test (#241)#254
gerchowl merged 1 commit intodevfrom
bugfix/241-rust-test-isolation

Conversation

@gerchowl
Copy link
Copy Markdown
Contributor

Summary

Closes #241.

The Rust client suite mixed 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, so live tests 404'd on http://127.0.0.1:<port>/v2026.04.2/release-manifest.json instead of huggingface.co — the underlying bug behind #241 (and the SIGABRT symptom that originally got reported).

Choice: Option C — in-tree EnvGuard RAII helper

Considered three options from the issue:

  • A — serial_test: doesn't restore env, just serializes execution; new dev dep.
  • B — scopeguard::defer!: correct, but requires a new dev dep.
  • C — in-tree EnvGuard: ~20 LOC, zero new deps, typed handle that's hard to forget. Picked C.

EnvGuard::set("MAT_VIS_HF_BASE", url) stores the prior value, sets the mock URL, and Drop restores (or removes if previously unset). The existing ENV_LOCK: Mutex<()> is kept so the suite stays race-safe under the default parallel cargo test runner — std::env::set_var is unsafe in the 2024 edition for that reason. Drop order: each test declares _lock before _env, so reverse-decl order drops _env (env restoration) while _lock is still held.

The hand-rolled env restore in e2e_fetch_color_png and resolved_tag_falls_back_to_default is also folded into the same helper / lock pattern — the e2e test now becomes panic-safe (was previously best-effort with manual unsafe blocks).

#253 rollback

Rolled back the .dagger/src/mat_vis_ci/main.py::test_client_rust dual-invocation workaround from #253. With proper test-side isolation, the single-invocation form is correct again — and avoids the second apt-get install + cargo test boot.

Before / after

Before (against bugfix/241-rust-live-isolation head, single cargo invocation): live tests after mock tests fail with 404 on http://127.0.0.1:<port>/v2026.04.2/release-manifest.json — env var leaked.

After (this branch, single cargo invocation):

$ MAT_VIS_LIVE_TESTS=1 MAT_VIS_LIVE_TAG=v2026.04.2 cargo test -- --test-threads=1
...
test result: ok. 21 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 12.95s

All 21 tests (including 4 live_*) pass cleanly in a single process — no SIGABRT, no 127.0.0.1 leakage in live test output.

Test plan

  • cargo build --tests clean
  • cargo test -- --test-threads=1 (no live) — 21 passed
  • MAT_VIS_LIVE_TESTS=1 MAT_VIS_LIVE_TAG=v2026.04.2 cargo test -- --test-threads=1 — 21 passed including 4 live tests
  • CI green on the rolled-back single-invocation test_client_rust

…e state leak (#241)

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.
@github-actions github-actions Bot added area:client Python/JS/Rust client packages area:baker Baker pipeline, Dagger, data fetchers labels Apr 30, 2026
@gerchowl gerchowl enabled auto-merge (squash) April 30, 2026 15:50
@gerchowl gerchowl merged commit 35dbaaa into dev Apr 30, 2026
4 checks passed
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 area:client Python/JS/Rust client packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(client/rust): SIGABRT in live tests when run after http unit tests, but passes in isolation

1 participant