From aae8b3785e9ba05dc99c594df0a59c7a69b0bedd Mon Sep 17 00:00:00 2001 From: Boris Batkin Date: Sat, 6 Jun 2026 13:05:35 -0700 Subject: [PATCH 1/2] playwright: fix wait_for_hover false-pass on missing widget + cover both #186 primitives Copilot review on PR #186: - wait_for_hover(ident, expected=false) false-passed when the widget was missing or had no `hover` field: the predicate `find_widget(...)?["hover"] ?? false == expected` coalesced a null hover to false, so a typo'd / unregistered ident read as a successful "not hovered" and masked a real sync failure. Guard with widget_exists (the same load-bearing check widget_rendered uses for the identical daslib/json ?[] quirk): the widget must actually be present before its hover state is compared. - Add tests/integration/test_recording_sync.das covering both primitives #186 added (no dasImgui-side test exercised either): wait_for_hover registers on move-onto / clears on move-away / does NOT false-pass a missing ident (the regression above), and record_check returns its `ok` and accumulates-without-panicking on false (the teardown-panic surfacing is exercised transitively by the record_* driver suite). Verified: test_recording_sync green headless; lint + format clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/integration/test_recording_sync.das | 50 +++++++++++++++++++++++ widgets/imgui_playwright.das | 6 ++- 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 tests/integration/test_recording_sync.das diff --git a/tests/integration/test_recording_sync.das b/tests/integration/test_recording_sync.das new file mode 100644 index 0000000..8364004 --- /dev/null +++ b/tests/integration/test_recording_sync.das @@ -0,0 +1,50 @@ +options gen2 +options indenting = 4 +options no_unused_block_arguments = false +options no_unused_function_arguments = false + +require dastest/testing_boost public +require imgui/imgui_playwright public +require daslib/json public +require daslib/json_boost public + +//! Integration coverage for the two recording-sync primitives added in dasImgui PR #186: +//! `wait_for_hover` and `record_check`. Uses the `color_button_hover` feature — the CBH_RED +//! swatch carries the per-widget IsItemHovered telemetry (the `hover` field) that wait_for_hover +//! polls. One app spawn covers both (record_check's `app` arg is unused). + +let FEATURE = "modules/dasImgui/examples/features/color_button_hover.das" +let TARGET = "CBH_WIN/CBH_RED" + +[test] +def test_recording_sync(t : T?) { + with_imgui_app(FEATURE) $(d) { + var snap = wait_for_widget(d, TARGET, 15.0f) + t |> success(snap != null, "{TARGET} renders") + if (snap == null) return + + // ---- wait_for_hover ---- + // Move onto the swatch — it must report hovered (the move -> wait_for_hover -> click gate). + move_to(d, widget_click_point(snap, TARGET)) + t |> success(wait_for_hover(d, TARGET) != null, "wait_for_hover registers hover after move_to") + + // Move off — hover must clear (expected=false on a PRESENT widget). + move_to(d, (4.0f, 4.0f)) + t |> success(wait_for_hover(d, TARGET, false) != null, "wait_for_hover sees hover clear after moving away") + + // Regression for the #186-A false-pass: a missing / misspelled ident must NOT pass as + // "not hovered". The old predicate coalesced a null hover to false and returned the first + // snapshot; the widget_exists guard makes it time out instead. Short timeout — guaranteed miss. + t |> success(wait_for_hover(d, "CBH_WIN/__no_such_widget__", false, 2.0f) == null, + "missing widget does not false-pass wait_for_hover(expected=false)") + + // ---- record_check ---- + // Returns its `ok` argument, and accumulates (does NOT panic) on false — the + // accumulate-don't-panic discipline; the teardown-panic surfacing is exercised transitively + // by every record_* driver in the suite. Reaching the line after the false call proves it + // didn't panic mid-body. + t |> success(record_check(d, "ok branch returns true", true), "record_check returns true when ok") + let failed = record_check(d, "intentional miss (accumulated, not panicked)", false) + t |> success(!failed, "record_check returns false and accumulates instead of panicking") + } +} diff --git a/widgets/imgui_playwright.das b/widgets/imgui_playwright.das index 74a1182..95ce66c 100644 --- a/widgets/imgui_playwright.das +++ b/widgets/imgui_playwright.das @@ -358,8 +358,12 @@ def public wait_for_hover(app : ImguiApp; widget_ident : string; expected : bool //! race a real continuous-hover mouse never hits (synthetic != real). Bounded by ``timeout_sec``; //! returns the matching snapshot or null on timeout. ``null`` is a real failure — gate it with //! ``record_check`` (recording) or ``success`` (test); never proceed to the click silently. + //! The ``widget_exists`` guard is load-bearing for ``expected=false``: an absent / misspelled + //! ident yields a null ``hover`` that ``?? false`` would coalesce to ``false``, so a missing + //! widget would false-pass as "not hovered" and mask a sync failure — same guard as + //! ``widget_rendered``. return wait_until_sec(app, timeout_sec) $(var snap) { - return (find_widget(snap, widget_ident)?["hover"] ?? false) == expected + return widget_exists(snap, widget_ident) && (find_widget(snap, widget_ident)?["hover"] ?? false) == expected } } From 444f93598744ee2ed57fe1e8363966fe2df87a82 Mon Sep 17 00:00:00 2001 From: Boris Batkin Date: Sat, 6 Jun 2026 13:14:34 -0700 Subject: [PATCH 2/2] test_recording_sync: shrink the missing-ident timeout 2.0s -> 0.3s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review on PR #187: the negative-case regression check waited out a full 2s every run (the widget is guaranteed missing, so it always times out). A tiny timeout still separates the two implementations — the old false-pass returned non-null on the first poll, the fixed version times out — without spending the wall-clock. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/integration/test_recording_sync.das | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_recording_sync.das b/tests/integration/test_recording_sync.das index 8364004..1b2d74d 100644 --- a/tests/integration/test_recording_sync.das +++ b/tests/integration/test_recording_sync.das @@ -33,9 +33,10 @@ def test_recording_sync(t : T?) { t |> success(wait_for_hover(d, TARGET, false) != null, "wait_for_hover sees hover clear after moving away") // Regression for the #186-A false-pass: a missing / misspelled ident must NOT pass as - // "not hovered". The old predicate coalesced a null hover to false and returned the first - // snapshot; the widget_exists guard makes it time out instead. Short timeout — guaranteed miss. - t |> success(wait_for_hover(d, "CBH_WIN/__no_such_widget__", false, 2.0f) == null, + // "not hovered". The old predicate coalesced a null hover to false and returned on the FIRST + // poll; the widget_exists guard makes it time out instead. A tiny timeout still separates the + // two (old=immediate non-null, fixed=times out) without spending the wall-clock to wait it out. + t |> success(wait_for_hover(d, "CBH_WIN/__no_such_widget__", false, 0.3f) == null, "missing widget does not false-pass wait_for_hover(expected=false)") // ---- record_check ----