diff --git a/plans/opencode/pytest-qt-chart-ui-e2e.md b/plans/opencode/pytest-qt-chart-ui-e2e.md new file mode 100644 index 00000000..4ba4175c --- /dev/null +++ b/plans/opencode/pytest-qt-chart-ui-e2e.md @@ -0,0 +1,442 @@ +# `pytest-qt` chart UI E2E handoff + +## Purpose + +Build a layered, deterministic UI test system for Piker's PyQt6 and +PyQtGraph chart interface. The first goal is to audit existing behavior with +real Qt objects and synthetic data. Later phases compose the full chart UI, +Trio guest run and selected Tractor actors without making live brokers, +credentials or network access part of the default gate. + +This is a strategy and agent handoff, not an implementation record. Do not +change user-owned task markers while executing it. + +## Branch and PR context + +The chart-local gap stack is the immediate reference implementation: + +- Base: `2aa66c2cf6c76b8e270af9367d27ca16f63e7024` + (`ib_methproxy_refinery`). +- Head: `335190a4e3ff5712626288ac5e81693b9646c140` + (`chart_local_gapper`). +- Range: five commits, `2aa66c2c..335190a4`. +- Both cached `gitea/chart_local_gapper` and + `dev/chart_local_gapper` refs resolve to the head. +- The intended prospective PR base is `ib_methproxy_refinery`, not `main`. + +The root checkout has unrelated human changes in `.gitignore` and untracked +`.modden/`, `todo/` and `wkts/`. Future UI-test work should use an isolated +worktree and leave those paths untouched. + +## Current evidence + +`tests/test_gap_overlays.py` proves that small real-Qt integration tests can +find bugs below Piker's own API surface: + +- Real `PlotWidget`, `PlotItem`, `ViewBox` and `QGraphicsScene` objects found + stale PyQtGraph registries after direct scene removal + (`tests/test_gap_overlays.py:293-409`). +- Scene spatial-index lookups found that + `prepareGeometryChange()` ran after geometry mutation + (`tests/test_gap_overlays.py:412-482`). +- Duplicate FQMEs found that chart-local state must include concrete chart + identity (`tests/test_gap_overlays.py:485-540`). +- Startup and focus tests found unrelated cached charts being treated as + realtime targets (`tests/test_gap_overlays.py:543-648`). +- A real `QKeyEvent` traverses `EventRelay` and the async chart handler + (`tests/test_gap_overlays.py:651-735`). +- A real Tractor child tests endpoint/client correlation, cancellation and + owner cleanup (`tests/test_gap_overlays.py:738-1025`). That is cross-process + IPC integration, not full chart UI E2E, because rendering is stubbed. + +The reusable rule is: keep Qt and PyQtGraph real; fake the feed, SHM and +service edges at the narrowest useful boundary. + +## Adoption prerequisites + +### Dependency and pytest configuration + +1. Add `pytest-qt` to the `testing` dependency group beside `pytest` in + `pyproject.toml:150-152`, then regenerate `uv.lock`. +2. Resolve the release against the locked pytest version rather than guessing + a version constraint. +3. Remove or replace the comments-only `pytest.ini`. Its presence currently + disables the intended `[tool.pytest.ini_options]` configuration in + `pyproject.toml:169-180`. +4. Configure `qt_api = "pyqt6"`. Also set `PYTEST_QT_API=pyqt6` in the + targeted CI job as an explicit process-level guard. +5. Keep `-p no:xonsh` until collection behavior is deliberately changed. +6. Register a `qt` marker only if it will be used consistently for selection; + do not invent broad `gui`, `network` or `offline` markers. + +Official pytest-qt behavior to rely on: + +- `qtbot` owns application creation and widget tracking. +- `qtbot.addWidget()` closes registered widgets after each test. +- `qtbot.waitSignal()`, `waitSignals()` and `waitUntil()` provide bounded, + postcondition-oriented synchronization. +- `qtbot.waitExposed()` and `waitActive()` cover real focus/window tests. +- Qt virtual-method and slot exceptions are captured as test failures. +- `qtbot.screenshot()` is useful as a failure artifact, not an initial + pixel-golden assertion system. + +References: + +- +- +- + +### Headless process contract + +Set these before Python imports Qt or creates `QApplication`: + +- `QT_QPA_PLATFORM=offscreen` +- `PYTEST_QT_API=pyqt6` +- isolated `XDG_CONFIG_HOME` + +Do not set the platform only inside an individual test module. The current +assignment at `tests/test_gap_overlays.py:13` is too late for broad collection +and makes ordering significant. + +Use the current Nix/uv PyQt6 environment. `flake.nix:47-89` supplies Qt6 and +plugin paths; uv should supply pytest-qt. Do not also add a separate Nix +pytest-qt package, and do not update the obsolete Python 3.11/PyQt5 +`develop.nix` stack. + +The following legacy modules remain outside the initial audit because they +still import PyQt5: + +- `piker/ui/_signalling.py` +- `piker/ui/_orm.py` +- `piker/ui/quantdom/charts.py` +- `piker/ui/quantdom/_equity.py` + +### Minimal project fixtures + +Do not override pytest-qt's `qapp` fixture. Remove the module-local fixture at +`tests/test_gap_overlays.py:145-159` during migration. + +Start with non-autouse project fixtures: + +- `qapp_args`: stable application name only. +- `piker_qt`: set `setQuitOnLastWindowClosed(False)`, expose `qtbot`, process + only documented final events and verify no top-level widget leaks. +- `isolated_qsettings`: isolate `XDG_CONFIG_HOME`, clear test-owned settings + and restore process-global config references. +- `ohlcv_factory`: deterministic structured arrays with configurable indexes, + timestamps, gaps, duplicates, nulls and ordering defects. +- `pg_chart_factory`: typed harness around a real `pg.PlotWidget`, `PlotItem`, + `ViewBox`, scene and minimal Viz/SHM adapter. +- `display_state_factory`: paired realtime/history chart harnesses, including + duplicate-FQME cases. + +Prefer small dataclasses or protocols over nested `SimpleNamespace` graphs. +Fixture finalizers must fail on leaked widgets, graphics items, event filters, +SHM objects or child actors instead of merely trying to clean them. + +Do not hand a production `MainWindow` to `qtbot.addWidget()` until its close +path is test-safe. `MainWindow.closeEvent()` currently sends `SIGINT` to the +process (`piker/ui/_window.py:331-348`), so ordinary pytest-qt teardown can +interrupt pytest. Layer 4 must first separate widget closure from CLI process +signalling. + +## Test architecture + +### Layer 1: Qt and PyQtGraph primitives + +No Trio, Tractor, feeds, SHM or service actors. + +First audit targets: + +1. `CompleterView` section insertion, selection and navigation + (`piker/ui/_search.py:82-525`). +2. `SearchWidget` focus, cached results and dispatch to a stub GodWidget + (`piker/ui/_search.py:556-825`). +3. `MultiStatus` plain/grouped status creation and cleanup + (`piker/ui/_window.py:143-253`). +4. `GapAnnotations` and `SelectRect` geometry, attachment, repositioning and + removal (`piker/ui/_annotate.py:307-785`, + `piker/ui/_editors.py:377-710`). + +Acceptance gate: + +- repeated same-process runs leave no top-level widgets, scene items, + settings or signals behind; +- Qt warnings and virtual-method exceptions fail the test; +- no arbitrary sleeps or screenshot comparisons. + +### Layer 2: chart interaction and overlays + +Real `ChartView`, PyQtGraph scenes and Qt input; synthetic display state and +no service actors. + +First audit targets: + +1. `ChartView.open_async_input_handler()` for Ctrl-G, Ctrl-R, Escape and + search-focus input (`piker/ui/_interaction.py:147-548`, `:690-720`). +2. Wheel and drag behavior, fake-feed pause/resume and graphics cache restore + (`piker/ui/_interaction.py:650-688`, `:731-1012`). +3. `PlotItemOverlay` y-axis independence, x-linking, focus ownership and + resize geometry (`piker/ui/_overlay.py:301-523`). +4. Cursor activation, labels and rounded data coordinates + (`piker/ui/_cursor.py:341-639`). + +Use `qtbot.keyClick()`, mouse helpers, `waitExposed()` and `waitUntil()` for +user-visible behavior. `ChartView` is a PyQtGraph `ViewBox`, not a `QWidget`, +so deliver input to the owning `ChartPlotWidget` viewport, map scene/data +coordinates into viewport coordinates and let `GraphicsScene` synthesize the +PyQtGraph event. Keep low-level event dispatch for `EventRelay` conversion and +wheel behavior where QtBot has no suitable high-level helper. + +The existing `trio.run()` Ctrl-G test can remain as a focused relay test, but +it must not stand in for production Trio guest-mode integration. + +### Layer 3: composed charts with deterministic SHM + +Real `LinkedSplits`, `ChartPlotWidget`, `ChartView`, Viz objects, cursor, +stickies and graphics update cycles. No datad, broker, EMS, search-provider or +FSP actor. + +First audit targets: + +1. Build a main OHLC chart from deterministic realtime/history arrays + (`piker/ui/_chart.py:133-520`, `:555-1025`). +2. Compose history and realtime splitters and verify x-linking/region movement + (`piker/ui/_display.py:1124-1288`, `:1409-1416`). +3. Mutate the final SHM row, call `graphics_update_cycle()` and assert the last + bar, sticky labels, L1 labels, x tread and y range + (`piker/ui/_display.py:594-1121`). +4. Exercise local order lines/arrows with fake trackers and client state + (`piker/ui/order_mode.py:107-659`, `piker/ui/_lines.py:549-827`). + +Use a real disposable `ShmArray` because chart drawing asserts that interface +(`piker/ui/_chart.py:914-938`). A local Tractor root runtime is mandatory for +allocation (`piker/data/_sharedmem.py:146-159`); allocation without it raises +`NoRuntime` (`tests/test_shm_cleanup.py:18-46`). Reuse the exact SHM cleanup +discipline in `tests/conftest.py:97-239`. Nest chart and SHM lifetimes inside +the runtime, release Qt/Viz references first, then close SHM and the runtime. +Service actors are not required. + +### Layer 4: full in-process UI with fake services + +Real `MainWindow`, `GodWidget`, search, chart hierarchy, input handlers and +Trio guest run. Replace feed, FSP and EMS boundaries with deterministic async +harnesses. + +First audit targets: + +1. Initial symbol boot and history/realtime chart hierarchy + (`piker/ui/_widget.py:157-284`, `piker/ui/_display.py:1296-1711`). +2. Search-driven symbol switch and side-pane movement + (`piker/ui/_search.py:745-795`). +3. Cached A-to-B-to-A switch with widget reuse, feed resume and no duplicate + display task (`piker/ui/_widget.py:225-253`). +4. Overlapping uncached A-to-B startup: block A at a typed fake-feed barrier, + start B, release A and assert each display task retains its own linked + widgets, feed and cache identity. Exercise both completion and cancellation + ordering (`piker/ui/_widget.py:157-284`, + `piker/ui/_display.py:1296-1416`). +5. Real keyboard/mouse order staging plus fake open/fill/cancel EMS messages + (`piker/ui/_interaction.py:459-579`, + `piker/ui/order_mode.py:1058-1329`). +6. Graceful cancellation: filters removed, cached charts closed, fake + consumers exited and guest-run outcome observed. + +Patch names imported into `_display`, not their original definitions: + +- `open_feed` (`piker/ui/_display.py:44-50`, `:1384-1392`) +- `open_order_mode` (`piker/ui/_display.py:87-90`, `:1663-1673`) +- `start_fsp_displays` (`piker/ui/_display.py:72-76`, `:1522-1530`) +- `open_sample_stream` / `increment_history_view` + (`piker/ui/_display.py:214-320`) + +`_async_main()` also starts provider search independently of `_display` +(`piker/ui/_app.py:46-74`, `:125-173`). Inject or patch +`_app.load_provider_search` and isolate symcache/config paths. Add guards which +fail if real `maybe_spawn_datad`, backend client creation, credentials or +network entrypoints are reached. + +### Layer 5: offline actor-backed UI + +Use real Tractor contexts only after widget and in-process composition are +stable. Keep all default cases credential-free and network-free. + +Candidate coverage: + +1. `pikerd` plus a deterministic fake datad/feed endpoint. +2. Remote chart-control endpoint discovery and chart-local rendering. +3. Actor cancellation while the window is open. +4. Child failure propagation into the guest-run outcome and pytest failure. +5. UI teardown proving no actor, registry, stream, SHM or Qt object survives. + +Actor barriers must be typed messages or explicit events. Do not use logger +text, timing sleeps or implicit task scheduling as readiness evidence. + +### Layer 6: opt-in system and visual qualification + +Keep these outside the default deterministic gate: + +- live broker/feed sessions; +- real credentials and account state; +- compositor-specific Wayland/X11 behavior; +- multi-monitor DPI and persistent window placement; +- long-run rendering performance and visual fidelity. + +Use screenshots and logs as failure artifacts first. Introduce pixel or image +golden assertions only after fonts, DPI, styles and renderer versions are +controlled across supported environments. + +## Production seams likely required + +`run_qtractor()` currently creates global application/window state, starts +Trio guest mode, shows the window and calls blocking `app.exec_()` +(`piker/ui/_exec.py:86-211`). pytest-qt owns the Qt loop, so the full in-process +layer needs a nonblocking production seam rather than calling +`run_qtractor()` directly. + +Prefer a minimal extraction that separates: + +1. application policy and style setup; +2. `MainWindow` / main-widget construction; +3. Trio guest-run installation and observable completion; +4. blocking `app.exec_()` used only by the CLI path. + +The test seam should return typed handles for the window, GodWidget, guest-run +outcome and cancellation scope. Do not build a second test-only event-loop +implementation. + +Window closure is part of this extraction. Prefer a close-request signal or +injected shutdown callback owned by the guest-run harness. The CLI adapter may +translate that request into its existing SIGINT behavior; tests must cancel +and await the guest run, remove filters/connections and then close the window +without signalling the pytest process. + +Other likely extractions should follow the gap-overlay precedent: pull one +deterministic transition out of a long-running loop only when a real test +needs it. Avoid broad UI rewrites before behavior is captured. + +## Synchronization rules + +1. Set environment and binding before importing Qt. +2. Let pytest-qt own the single `QApplication`. +3. Register every test-owned widget immediately. +4. Show/focus widgets before user-input assertions. +5. Wait on a signal or postcondition, never a guessed delay. +6. Use bounded timeouts and assert the final state after every wait. +7. Treat `processEvents()` as a documented low-level exception, not a general + synchronization primitive. +8. Use deterministic typed barriers for Trio/Tractor transitions. +9. Assert both visible behavior and ownership registries for PyQtGraph items. +10. Assert cleanup after cancellation and failure paths, not only happy paths. + +For graphics geometry, preserve the dual test used by gap overlays: + +- white-box arrays/bounds have moved; +- black-box `QGraphicsScene.items(point)` finds the new location and no longer + finds the old one. + +## Global state and cleanup audit + +Every fixture touching these globals must snapshot, restore and assert: + +- `_window._qt_win`; +- `_remote_ctl._dss`, `_annots` and `_gapman`; +- `GodWidget` linked-chart caches, cursor and root nursery; +- Qt event filters and signal connections; +- `QApplication.topLevelWidgets()`; +- `QSettings`, config paths and module-global fonts; +- PyQtGraph `PlotItem.items`, `ViewBox.addedItems` and scene membership; +- SHM names/tokens and Tractor actor/registry identities. + +Use unique actor names and registry addresses per test session. Never reap Qt +objects, actors or SHM by broad process/name patterns. + +## Suggested test layout + +Keep protocol-only tests separate from Qt tests: + +- `tests/ui/conftest.py`: Qt/data/chart fixtures and leak assertions. +- `tests/ui/test_widgets.py`: search, completer, status and forms. +- `tests/ui/test_graphics_items.py`: annotations, selection and overlays. +- `tests/ui/test_chart_interaction.py`: keyboard, mouse, cursor and focus. +- `tests/ui/test_chart_composition.py`: linked charts, Viz and SHM updates. +- `tests/ui/test_godwidget.py`: symbol boot/switch/cache lifecycle. +- `tests/ui/test_order_mode.py`: fake EMS order UI transitions. +- `tests/ui/test_actor_runtime.py`: explicitly offline actor-backed UI. + +Retain schema/msgspec tests in their current non-Qt modules so they remain +fast and diagnosable. + +## CI progression + +The tracked `.github/workflows/ci.yml` is stale: it uses Python 3.10, +`setup.py`, missing requirements files and an uncontrolled full-suite command. +Do not claim UI CI coverage until a targeted job provisions the current +Python 3.13 uv/Nix-compatible stack. + +Initial job contract: + +- Python 3.13 and frozen uv environment; +- `QT_QPA_PLATFORM=offscreen`; +- `PYTEST_QT_API=pyqt6`; +- isolated `XDG_CONFIG_HOME`; +- explicit `tests/ui/` plus migrated gap/DPI paths; +- screenshots, Qt logs and pytest output retained on failure. + +Keep live feeds, account tests and compositor-specific tests out of this job. + +## Incremental PR boundaries + +1. Harness/migration PR: dependency/lock, pytest configuration, fixtures, + targeted CI command and migration of existing gap real-Qt cases onto + `qtbot` without changing their ownership/spatial-index assertions. +2. Primitive audit PR: search, completer, status and graphics-item behavior. +3. Interaction PR: input, cursor and overlay audit. +4. Composition PR: deterministic SHM, linked charts and graphics update cycle. +5. Full in-process PR: nonblocking `run_qtractor()` seam, safe window closure, + isolated provider search, overlapping startup, GodWidget switching and + fake EMS/feed lifecycle. +6. Actor-backed PR: offline service tree, failure propagation and teardown. + +Each PR should keep tests with the production seam or defect they prove. Do +not batch unrelated UI corrections simply because one audit pass found them. + +## Eventual repo-local skill + +Distill a `piker-ui-e2e` skill only after the harness and first component +suite provide empirical commands and failure modes. The skill should encode: + +1. environment and Qt-binding validation; +2. supported test tiers and escalation rules; +3. fixture ownership and global-state restoration; +4. signal/postcondition synchronization rules; +5. PyQtGraph attachment, detachment and scene-index assertions; +6. Trio guest-run and Tractor actor boundaries; +7. SHM/actor/widget leak checks; +8. headless CI commands and opt-in visual/live commands; +9. failure classification and artifact collection; +10. change-to-test mapping for UI modules. + +Do not bake speculative APIs into the skill. First record the real harness, +then turn repeated practice into instructions. + +## First-agent assignment + +The next agent should implement only the harness/migration PR and stop after +that foundation is demonstrably stable: + +1. open an isolated worktree at `335190a4`, stacking on the pushed + `chart_local_gapper` head; do not branch from `2aa66c2c`, where the gap + implementation and tests do not exist; +2. add pytest-qt and activate one pytest configuration source; +3. establish process-level offscreen/PyQt6/config isolation; +4. migrate the gap tests from the custom `qapp` to `qtbot`; +5. add typed chart/data fixtures and centralized PyQtGraph ownership helpers; +6. run repeated same-process tests and prove cleanup; +7. add the targeted CI command if the current workflow can provision it + without broad unrelated CI repair; +8. document exact commands and discovered failure modes for the future skill. + +The first agent should not yet launch live feeds, rewrite `run_qtractor()`, +add new widget audits, add screenshot goldens or attempt the complete +GodWidget/actor stack.