Skip to content

Additional pipeline tests - #32

Merged
HuguesSib merged 9 commits into
mainfrom
pipeline-tests
Aug 21, 2026
Merged

Additional pipeline tests#32
HuguesSib merged 9 commits into
mainfrom
pipeline-tests

Conversation

@evanjt

@evanjt evanjt commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Tests for the features added in #25: device selection, orchestrator lifecycle (cancel, pause, viewer protocol), mapping progress callbacks, LoGeR progress counting, and resume with uncompressed npz.

@evanjt
evanjt requested a review from HuguesSib August 20, 2026 09:35
@evanjt evanjt self-assigned this Aug 20, 2026
@evanjt evanjt mentioned this pull request Aug 21, 2026
@HuguesSib

Copy link
Copy Markdown
Collaborator

Review

Ran the suite locally on this branch: 139 passed. Good coverage of the #25 surface, and the byte-identical / reference-comparison style is the right call. One test is broken, though.

test_failed_run_stops_the_sampler_thread is vacuous 🔴

It counts threads named "drm-resource-sampler", which exists nowhere in the repo except that test — the only threads the package ever creates are viser-render and simple-viser-render. So _sampler_threads() returns [] before and after, and the assertion is 0 == 0.

Verified rather than inferred — asserting the counted set is empty on both sides passes, with only MainThread alive through three failing runs:

LIVE THREADS: ['MainThread']
1 passed

It would stay green through any real thread leak. The docstring describes a sampler that presumably lives in the GUI repo; the assumption seems to have leaked in.

Fix: drop it, or retarget it at a thread that actually exists (viser-render) with a real viser app in play.

The rest — solid ✅

  • test_device.py — nice short-circuit check: capability 7.5 → float16 without running the flash probe, enforced via an AssertionError side-effect. Dynamo globals restored in finally.
  • test_resume_npz.py — the strongest one here: saves the npz exactly as the orchestrator does, reloads, proves the cloud is byte-identical fresh-vs-resumed. Locks the Add support for GUI, type fixes, dependency updates #25 local_points change down properly.
  • test_orchestrator_lifecycle.py (rest) — owned-vs-injected viewer semantics are exactly right, and the signature-position guard protects GUI callers from enable_viser drift.
  • test_loger_progress.py_count_windows expectations check out against the implementation; the block-progress test also re-asserts the transform against a single-shot reference, so it can't silently accept a broken re-anchor.

Minor

  • _single_shot_reanchor_world is duplicated here and in test_loger_reanchor.py — a shared helper would avoid drift.
  • _run_expecting_failure uses a broad pytest.raises(Exception); tightening it would catch a test failing for the wrong reason.

Verdict: approve once the sampler test is dropped or retargeted.

@HuguesSib
HuguesSib merged commit d646a9f into main Aug 21, 2026
15 checks passed
@HuguesSib
HuguesSib deleted the pipeline-tests branch August 21, 2026 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants