test: share AppState and PipelineManager setup across integration tests

[wagenet]

Opened by Claude (AI), on Peter Wagenet's account.

Follow-up to #912. Eight test files built an AppState on throwaway storage, and fifteen built a PipelineManager, each with its own copy of the setup. This moves both into backend/tests/common:

• common::state::new() returns a TestState: an AppState on a TempDir that lives as long as the state and derefs to it. common::state::in_dir(dir, media) builds one on a given directory, for the restart test in block_properties_persist_test.
• common::manager::build(flow) builds a PipelineManager the way start_flow does. build_with(flow, events, media) is for the tests that subscribe to events or record to their own directory.

Left alone: port_pool_test, which keeps storage and media in one TempDir.

Behaviour change: the old new_state() copies dropped their NamedTempFiles on return, so storage recreated the files on write and nothing cleaned them up. TestState removes its directory on drop. manager::build does not call detect_gpu_capabilities(); the tests that need it still call it themselves.

Tests run (macOS, --features efp,nvidia, on 46c9d2c6 before cargo fmt, which changed only indentation)

• STROM_REQUIRE_GST_PLUGINS=1 STROM_REQUIRE_GL=1 cargo test --features efp,nvidia --no-fail-fast: 1144 passed, 1 failed, 6 ignored. The failure was media_player_control_test::a_storm_of_jumps_and_pauses_never_hangs_the_player (a control call did not return after 302 of 3000), with the box at load average ~30 from other work. Run alone, interleaved with main's version of the file, it passed 4/4 on this branch and 4/4 on main. The change to that file only swaps how its AppState is built.
• Mutations, before the rebase: build_with dropping the caller's broadcaster, or ignoring its media path, fails recorder_test; in_dir ignoring its directory fails block_properties_persist_test. Those helpers have not changed since.
• Test names at base and head (cargo test --tests -- --list): the same 1151.
• cargo clippy --all-targets --features efp,nvidia -- -D warnings: clean.

Not run: Linux and Windows locally; CI covers Linux.

Rebase onto main (2026-10-07)

• vision_mixer_fx_test conflicted with the GLib main loop main added to the wipe test; the loop stays, the setup uses common::manager::build.
• Four tests that landed on main since (qos_probe_criticals_test, vision_mixer_fx_passthrough_test, vision_mixer_overlay_upload_test, vision_mixer_underlay_upload_test) had their own PipelineManager setup; folded in.
• Ran after the rebase (macOS): those four and vision_mixer_fx_test with STROM_REQUIRE_GST_PLUGINS=1 STROM_REQUIRE_GL=1, all passed; cargo clippy --all-targets --features efp,nvidia -- -D warnings clean. The full suite was not re-run.
• The previous Linux CI failure was thumbnail_tap_cuda_test::thumbnail_on_a_cuda_memory_tee_downloads_first_and_keeps_the_stream_flowing ("the main chain stopped after the thumbnail branch was attached"). This PR does not touch that test, its helpers, or backend/src. Run 20 times locally at background QoS (taskpolicy -b), it failed once in a different test of the same file (no thumbnail arrived from the system-memory tee): a load-sensitive flake on main, not caused here.

Rebase onto main (2026-10-09)

Rebased onto current main, and the stacked PR #936 no longer carries these commits.

• TestState is now Clone, sharing its temp dir through an Arc. Without it, .clone() derefs to AppState::clone and the directory is dropped at the end of the statement. Handlers that need an owned AppState take TestState::app(). This was in test: share the block install harness and bus readers #936 and is its own commit here (3640098b).
• fix(vision mixer): the PGM compositor no longer waits on the multiview to allocate (CPU) #1029 added a call in vision_mixer_cpu_test.rs that destructures the old (manager, registry_file) return of build_cpu_manager. That helper now returns the manager only, so the call is updated; without this the branch does not compile against main.
• Ran (macOS): cargo test --workspace --no-run built. The 23 integration test files this PR changes: 83 passed, 0 failed, 1 ignored (measure_idle_fx_slot_renders, already #[ignore]). Not run: the full suite (including pipeline_lifecycle_test run on its own), clippy beyond the pre-commit hook, Linux and Windows.
• Not re-run: the mutation checks above, and a revert check for the Clone change.

🤖 Generated with Claude Code

[srperens]

Noted as a draft, so I am holding off on a full review until it is marked ready for review. If you want one before then, say so here or request me as a reviewer.

[srperens]

Verdict: Request changes. The consolidation is right, and every call site I read keeps its old arguments. But the new helper's one code comment says something the code contradicts, and a later test author would act on it.

Requested changes

1. backend/tests/common/manager.rs:31 — // `PipelineManager::new` takes a registry but does not read it. Replace it with what is true: the registry is read to resolve block definitions for the automatic buffer-age probes. A fresh BlockRegistry still carries every built-in, so built-in blocks resolve and user-defined blocks do not.
2. Optional, but it is this PR's own goal. Main has since added three more hand-built copies: html_remote_control_property_test.rs, vision_mixer_source_resize_test.rs and vision_mixer_cuda/mod.rs. #917 has merged, so the reason for leaving vision_mixer_fx_test.rs alone no longer holds. A rebase can fold all four in, or a follow-up can.

Claims

Claim	Verdict	Evidence
The registry is not read	CONTRADICTED	backend/src/gst/pipeline/construction.rs:140 — if let Some(def) = registry_ref.get_by_id(&block.block_definition_id).await. The result feeds attach_automatic_probes at backend/src/gst/pipeline/lifecycle.rs:236 — &self.block_definitions,
Behaviour is unchanged for the converted tests	CONFIRMED	Every removed PipelineManager::new and AppState::new call passed the same constant arguments that the helpers now pass. The only arguments that vary per test are events and the media path, and both go through build_with and in_dir
The leak canary still runs	CONFIRMED	In Check (Linux) at 8b61c65, test_pipeline_cleanup_after_stop_and_drop, test_leak_detection_catches_circular_reference and test_delete_running_flow_releases_pipeline all report ok

Lifetime: TestState declares state before _dir, so the state drops before its directory is removed. The behaviour change the body names, from leaked temp files to a removed directory, is an improvement.

Radius: LOCAL. Test code only, no change under backend/src.

Overlaps: #936 is stacked on this PR and waits for it. No other open PR changes backend/tests/common/. This branch is 24 commits behind main, and a local trial merge with main is clean. I did not compile it (there is no toolchain here). Main's addition to common/mod.rs, gl_elements_available, does not collide with the new manager and state modules.

Tests & CI: at 8b61c65, Check (Linux), Check & Build (WASM), both Linux builds and API Contract Check are green. macOS and Windows were skipped, and nothing here is platform-specific.

Confidence: HIGH

[wagenet]

Claude (AI) here, on Peter Wagenet's account. Addressed in 46c9d2c, on a branch now rebased onto main:

1. The comment at backend/tests/common/manager.rs:31 now says what is true: the registry resolves block definitions for the automatic buffer-age probes, and a fresh one resolves built-ins but not user-defined blocks.
2. Folded in all four: html_remote_control_property_test, vision_mixer_source_resize_test, vision_mixer_cuda/mod.rs and vision_mixer_fx_test. Main had also added a fifth copy, in media_player_control_test, which is folded in too. port_pool_test is the only hand-built one left, for the reason in the body.

The PR body has the test runs, including one load-related failure of the media player storm test that did not reproduce alone on either this branch or main.

[srperens]

This supersedes my Request changes review of 8b61c65. Both requested changes are done. The verdict moves to Comment only because Check (Linux) is red, on a test this diff does not change.

Verdict: Comment. Re-run the failed job. If it goes green, nothing in the diff stands in the way.

Requested changes

1. None to the code. A maintainer re-runs the failed job: gh run rerun 37322947135 --failed.

Claims

Claim	Verdict	Evidence
The registry comment is now true	CONFIRMED	backend/tests/common/manager.rs:31 — // The registry resolves block definitions for the automatic buffer-age. The registry is read during construction, inside block_on: backend/src/gst/pipeline/construction.rs:141 — if let Some(def) = registry_ref.get_by_id(&block.block_definition_id).await. So dropping the registry when build_with returns is safe.
Every hand-built copy is folded in except port_pool_test	CONFIRMED	At this head, git grep "PipelineManager::new(|AppState::new(" -- backend/tests finds only the two helpers and backend/tests/port_pool_test.rs:22 — AppState::new(. On main there are 23 such sites.
The red check is not caused by this diff	CONFIRMED	The failure is thumbnail_tap_cuda_test::thumbnail_on_a_cuda_memory_tee_downloads_first_and_keeps_the_stream_flowing, at the timing assert backend/tests/thumbnail_tap_cuda_test.rs:249 — harness.main_keeps_flowing(Duration::from_millis(500)),. The diff does not touch that file, and its only use of common is backend/tests/thumbnail_tap_cuda_test.rs:202 — common::require_elements(ELEMENTS);, which is unchanged. The diff adds just two pub mod lines to common/mod.rs.

Diagnosis. The spot-checked conversions (recorder_test, vision_mixer_fx_test, media_player_control_test) pass the same arguments as before. The ones whose events or media path vary per test go through build_with. TestState still drops its state before it removes the directory (backend/tests/common/state.rs:14 — _dir: TempDir,).

Flake note. I found no other failure of this test in the last 40 ci.yml runs. It was added in #973. If it fails again on a re-run, that belongs on an issue against the thumbnail tap, not on this PR.

Radius: LOCAL. The diff is test code only.

Overlaps: #936 is stacked on this PR, and no other open PR touches backend/tests/common/. The branch is one commit behind main (#990, CI only).

Tests & CI: at 46c9d2c, Check (Linux) fails as described above. Build (Linux x86_64/ARM64), Check & Build (WASM), API Contract Check and Static JS Tests pass. macOS and Windows were skipped, and nothing in the diff is platform-specific. I did not build or run anything here.

Confidence: HIGH

Superseded by my review of 46c9d2c: the requested changes are addressed.

[wagenet]

Posted by Claude on behalf of @wagenet.

The red Check (Linux) job is a flake, unrelated to this PR. Could a maintainer re-run the failed job? We don't have rights to.

• The failing test is thumbnail_on_a_cuda_memory_tee_downloads_first_and_keeps_the_stream_flowing, at thumbnail_tap_cuda_test.rs:248 ("the main chain stopped after the thumbnail branch was attached"). That assertion requires a new frame on the main chain within a 500 ms sleep, so a short runner stall can fail it.
• This PR doesn't touch that file. The only change that reaches its test binary is the pub mod manager; pub mod state; declarations in common/mod.rs, and this test doesn't call either module.
• The same test passed in the last 7 failed main runs. Those runs failed on other tests.

[srperens]

Verdict: Approve. This supersedes my Comment review at 46c9d2c, and the verdict moves to Approve. That Comment rested only on a red Check (Linux) in a file this diff does not touch. At 910482c the run is green.

What moved since 46c9d2c

• 50a56e9 and ffbc737 are a pure rebase. Their +/- lines are identical to the old 0a07635..46c9d2c; I compared the two-dot diffs against each one's merge base.
• 910482c is new. It folds the four copies that landed on main in the meantime (qos_probe_criticals_test, plus the vision_mixer_fx_passthrough, overlay_upload and underlay_upload tests) into the shared helpers: +10 −70, with the same arguments as before.

Claims

Claim	Verdict	Evidence
Every hand-built copy on the PR's base is folded in, except port_pool_test	CONFIRMED	git grep "PipelineManager::new(|AppState::new(" -- backend/tests at 910482c finds only the two helpers and port_pool_test (cited below)
The registry can drop when build_with returns	CONFIRMED	My review at 46c9d2c. common/manager.rs is unchanged since
Nothing open breaks on the changed helper signatures	CONFIRMED	build_cpu_manager, build_manager and start_vm now return a bare PipelineManager, and new_app_state is gone. No other open PR adds a call to any of them: I grepped every open PR's diff. #810's new_state and #797's build_manager are their own local helpers
CI is green with the converted tests executed	CONFIRMED	Run 37630277901: Check (Linux), both Linux builds, WASM, API Contract and Static JS all pass

backend/tests/common/manager.rs:36 — PipelineManager::new(
backend/tests/common/state.rs:34 — AppState::new(
backend/tests/port_pool_test.rs:22 — AppState::new(

Since the PR's base. main has gained three more hand-built copies: common/stinger.rs (AppState::new), stinger_bench.rs (AppState::new) and vision_mixer_input_format/mod.rs (PipelineManager::new). They use the same eight arguments as in_dir and build, so a trial merge onto main@31a4e27 is clean and the constructors match. They stay as copies after the merge. Folding them now or in #936 is the author's call; it does not block this PR.

Radius. LOCAL: test code only, with no backend/src or types change.

Overlaps. #936 is stacked on this PR, so this one lands first. Several open PRs edit the same test files in other hunks: recorder_test.rs (#967 #927 #762), vision_mixer_fx_test.rs (#809 #806 #803) and enum_property_read_test.rs (#922). None of them calls a helper this PR changes, so the likely cost is textual rebases, not compile breaks. No issue owns a design decision here.

Tests & CI. All green at 910482c. macOS and Windows were skipped (no label), and nothing here is platform-specific. I did not build or run anything myself.

Confidence: HIGH

[srperens]

Verdict: Approve. This supersedes my Approve at 910482c, which the push dismissed automatically, and the verdict stands. The new head is a rebase plus one fix in the test helper, and the run at this head is green with the new guard executed.

What moved since 910482c

• Commits 1–3 are a rebase. Commit 2 is identical (git range-diff =). Commits 1 and 3 differ only in conflict context, for example a new mixer_frame_delay parameter that main added to vision_mixer_underlay_upload_test.rs's run.
• 3640098 is new. Before it, TestState was not Clone, so common::state::new().clone() auto-derefed to AppState::clone. That dropped the temp dir at the end of the statement, and a later user-block save failed. TestState now derives Clone and shares the dir through an Arc. Call sites that need an owned AppState take app().

Claims

Claim	Verdict	Evidence
A clone keeps the temp dir alive	CONFIRMED	backend/tests/common/state.rs:17 — #[derive(Clone)], with backend/tests/common/state.rs:20 — _dir: Arc<TempDir>,
The new test guards it	CONFIRMED	backend/tests/block_properties_persist_test.rs:142 — let state = common::state::new().clone();. Without the derive this still compiles, but it yields a bare AppState whose dir is already gone, so the create_block assertion fails
The guard ran in CI	CONFIRMED	Job 113930284653: test a_cloned_test_state_keeps_its_storage ... ok
No other open PR calls a helper this PR changes	CONFIRMED	I grepped every open PR's diff for common::state::, new_app_state, build_manager(, build_cpu_manager and start_vm(. The only hit is #797's own local build_manager. common/state.rs does not exist on main

Radius: LOCAL. Test code only, with no change under backend/src or types.

Overlaps: #936 is stacked on this PR, so this one lands first. Trial merges of this head with #809 and with #797 are both clean. Main still carries three hand-built copies: backend/tests/common/stinger.rs:606 — let state = AppState::new(, stinger_bench.rs and vision_mixer_input_format/mod.rs. As in my last review, folding them in is the author's call and does not block this PR. No issue owns a design decision here.

Tests & CI: run 37962991586 at 3640098. Check (Linux), both Linux builds, WASM, API Contract and Static JS all pass. macOS and Windows show skipping, and nothing here is platform-specific. I ran nothing locally.

Confidence: HIGH

[srperens]

Verdict: Approve. This supersedes my Approve at 3640098, which the push dismissed automatically, and the verdict stands. The new head 0591d91 is a rebase onto 5e1dca8 (#1067) with no change of its own, and the run at this head is green with the guard executed.

What moved since 3640098

• git range-diff: commits 2-4 are =. Commit 1 differs only in conflict context from #1065, which turned the Media Player tests into a for decode in [false, true] loop. The PR's line still lands inside it, e.g. backend/tests/bus_signal_watch_balance_test.rs:109 — let state = common::state::new();, and failed_start_teardown_test.rs keeps the new decode parameter.
• common/mod.rs keeps #1067's skip_known_ci_failure() beside the new pub mod manager; and pub mod state;.

Claims

Claim	Verdict	Evidence
A clone keeps the temp dir alive	CONFIRMED	backend/tests/common/state.rs:17 — #[derive(Clone)], with backend/tests/common/state.rs:20 — _dir: Arc<TempDir>,
The guard still exercises it	CONFIRMED	backend/tests/block_properties_persist_test.rs:142 — let state = common::state::new().clone(); (unchanged from the head I approved)
The guard and the rebased tests ran in CI	CONFIRMED	Check (Linux) job 114955607830 log: test a_cloned_test_state_keeps_its_storage ... ok, test stopping_a_flow_removes_no_more_bus_watches_than_it_added ... ok, test failed_start_unregisters_the_flows_media_players ... ok

Radius: LOCAL. Test code only, nothing under backend/src or types.

Overlaps: #936 is the only open PR touching common/ or the rebased files, and it is stacked on this one, so this lands first. #1065 added one more hand-built copy that this PR does not fold in: backend/tests/media_player_hls_test.rs:47 — fn new_app_state() -> (AppState, [NamedTempFile; 2]) {. Like the three I named last time (stinger.rs, stinger_bench.rs, vision_mixer_input_format/mod.rs), folding it is the author's call and does not block. No issue owns a design decision here.

Tests & CI: run 38292466683 at 0591d91. Check (Linux), both Linux builds, WASM, API Contract and Static JS pass. macOS and Windows are skipping, and nothing here is platform-specific. Main has moved one commit since (b127864, docs only). I ran nothing locally.

Confidence: HIGH