From 0e9888645f8d4f9e29e5719a2638b504e9e1e355 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Thu, 7 May 2026 04:00:35 +0000 Subject: [PATCH] CI(ui): STUDIO_UI_STRICT mode + theme cycle fix + Recents thread-match assertion The existing UI test was passing too easily: every "if button.count() == 0: log WARN" branch silently degraded into a green run. Three places this hid real bugs: 1. The theme toggle for-loop bailed after cycle 1 because the Radix Account-menu's data-state="open" lingered through the view-transition and the next acct.click() hit the still-open dropdown. The test went green observing only one polarity. 2. The regenerate button branch silently skipped when the assistant action bar didn't render (every CI run so far -- the locator was wrong, but no one noticed because it was a soft skip). 3. The Recents click accepted ANY non-nav sidebar entry, so a freshly deleted thread or an unrelated entry would still pass. Fixes: - Add STUDIO_UI_STRICT=1 env (default on in CI via workflow, default off locally). When on, every soft "if not visible: log WARN" branch hard-fails. The strict-skip pattern is centralised in a soft_fail() helper so the local-vs-CI split is one knob. - Theme toggle: wait for [role="menu"] to detach between cycles (the dropdown stay-open was the cycle-2 bail), assert the loop actually ran 3 times. - Model picker search: capture popover text after typing "qwen" vs "llama"; the two snapshots must DIFFER, proving the typeahead actually filters (a regression that rendered the picker but ignored input would silently pass before). - Recents click: after navigating to the clicked thread, the rendered turns must include at least one of our sent prompts ("hello", "world", "tree", "1+1", etc.) -- proves we landed on OUR thread, not a leftover from a previous run. - Use [data-tour="chat-model-selector"] as the primary selector for the model picker -- the guided-tour anchor is at least as stable as anything else in the codebase (the tour breaks if it moves), and there's no separate data-testid system to maintain. --- .github/workflows/studio-ui-smoke.yml | 4 + tests/studio/playwright_chat_ui.py | 142 +++++++++++++++++++++----- 2 files changed, 122 insertions(+), 24 deletions(-) diff --git a/.github/workflows/studio-ui-smoke.yml b/.github/workflows/studio-ui-smoke.yml index 1b7053b46a..c9e608dd2d 100644 --- a/.github/workflows/studio-ui-smoke.yml +++ b/.github/workflows/studio-ui-smoke.yml @@ -154,6 +154,10 @@ jobs: # against a freshly-installed Studio (BASE_URL=...; STUDIO_OLD_PW= # $(cat ~/.unsloth/studio/auth/.bootstrap_password); python ...). PW_ART_DIR: logs/playwright + # Strict mode: in CI a missing button / nav / dialog must + # FAIL the test. Locally the test still runs against partial + # Studio installs without STUDIO_UI_STRICT. + STUDIO_UI_STRICT: '1' run: | mkdir -p logs/playwright python tests/studio/playwright_chat_ui.py diff --git a/tests/studio/playwright_chat_ui.py b/tests/studio/playwright_chat_ui.py index 8b93718246..2d1d609d59 100644 --- a/tests/studio/playwright_chat_ui.py +++ b/tests/studio/playwright_chat_ui.py @@ -58,6 +58,12 @@ ART_DIR = os.environ.get("PW_ART_DIR", "logs/playwright") ART = Path(ART_DIR) ART.mkdir(parents = True, exist_ok = True) +# Strict mode -- when on (default in CI), the test fails loudly if any +# expected button / nav / dialog is missing instead of logging a WARN +# and continuing. Locally we leave it off so the test still runs against +# a partial Studio install. +STRICT = os.environ.get("STUDIO_UI_STRICT", "0") == "1" + _n = [0] @@ -73,6 +79,18 @@ def fail(m): raise AssertionError(f"[ui] FAIL: {m}") +def soft_fail(m): + """Hard fail in STRICT mode, info-warn otherwise. + + Use for "this button should exist but didn't" assertions where + a missing element is a regression in CI but acceptable when + running against a partial Studio locally. + """ + if STRICT: + fail(m) + info(f"WARN (strict-off): {m}") + + def login_via_api(pw): req = urllib.request.Request( f"{BASE}/api/auth/login", @@ -282,36 +300,59 @@ with sync_playwright() as p: # surface here. # ───────────────────────────────────────────────────── step("model picker: open + drive search bar") - picker_btn = page.locator( - 'button:has-text("gemma-3-270m"), ' - 'button:has-text("Gemma 3"), ' - 'button:has-text("Select model")' - ).first + # Stable selector first: [data-tour="chat-model-selector"] is the + # guided-tour anchor on the model picker button (app-sidebar.tsx). + # If the tour anchor moves the tour breaks, so this selector is at + # least as stable as anything else in the codebase. + picker_btn = page.locator('[data-tour="chat-model-selector"]').first if picker_btn.count() == 0: - # Fall back to any button mentioning the loaded model. - picker_btn = page.get_by_role( - "button", - name = re.compile(r"gemma-?3", re.I), + # Fall back to text-based locators for older Studio builds. + picker_btn = page.locator( + 'button:has-text("gemma-3-270m"), ' + 'button:has-text("Gemma 3"), ' + 'button:has-text("Select model")' ).first - if picker_btn.count() > 0: + if picker_btn.count() == 0: + soft_fail("model picker button not found") + else: picker_btn.click() page.wait_for_timeout(500) shoot("03c-model-picker-open") search = page.get_by_placeholder( re.compile(r"Search.*models?", re.I), ).first - if search.count() > 0: + if search.count() == 0: + soft_fail("model picker search input not found") + else: + # Type "qwen" -> capture popover text. Type "llama" -> capture + # again. The two text snapshots must DIFFER, proving the + # typeahead actually filters the list (a regression that + # rendered the picker but ignored input would silently pass + # the old version of this test). + def picker_visible_text(): + return page.evaluate("""() => { + const el = document.querySelector( + '[role="dialog"], [role="listbox"], [role="menu"]' + ); + return el ? (el.innerText || '').trim() : ''; + }""") search.fill("qwen") page.wait_for_timeout(800) + qwen_text = picker_visible_text() shoot("03d-model-picker-search-qwen") search.fill("") page.wait_for_timeout(300) search.fill("llama") page.wait_for_timeout(800) + llama_text = picker_visible_text() shoot("03e-model-picker-search-llama") - info("OK search bar filtered for qwen + llama") - else: - info("WARN model picker search input not found") + if qwen_text and llama_text and qwen_text == llama_text: + soft_fail( + "model picker text was identical for qwen + llama " + "queries -- typeahead may not be filtering" + ) + else: + info("OK search bar filtered (qwen text != llama text)") # Close picker without changing selection. page.keyboard.press("Escape") page.wait_for_timeout(300) @@ -387,7 +428,7 @@ with sync_playwright() as p: shoot("05-after-regenerate") info("regenerate completed") else: - info("regenerate button not visible (skip)") + soft_fail("regenerate button not visible") # ───────────────────────────────────────────────────── # 6. Add two more turns AFTER regenerate. @@ -499,23 +540,50 @@ with sync_playwright() as p: step("theme toggle x3 with computed-color assertion") observed = [] for cycle in range(3): + # Wait for any prior dropdown to fully detach. The Radix + # Account-menu sets data-state="open" while the view- + # transition is mid-flight; clicking it again before that + # clears would no-op silently and the for-loop bailed + # after cycle 1 in earlier runs. + try: + page.wait_for_function( + """() => !document.querySelector('[role="menu"]')""", + timeout = 3_000, + ) + except Exception: + pass + page.wait_for_timeout(150) try: acct.click(force = True) except Exception as exc: - info(f" cycle {cycle + 1}: account-menu click failed ({exc!r})") + soft_fail( + f"theme cycle {cycle + 1}: account-menu click failed " + f"({exc!r})" + ) + break + # Wait for the dropdown menu to actually render before + # querying its items. + try: + page.wait_for_selector('[role="menu"]', timeout = 3_000) + except Exception: + soft_fail(f"theme cycle {cycle + 1}: account menu didn't open") break - page.wait_for_timeout(400) theme_item = page.get_by_role( "menuitem", name = re.compile(r"^(Light Mode|Dark Mode)$", re.I), ).first if theme_item.count() == 0: page.keyboard.press("Escape") + soft_fail(f"theme cycle {cycle + 1}: theme menuitem missing") break try: theme_item.click(force = True) - except Exception: + except Exception as exc: page.keyboard.press("Escape") + soft_fail( + f"theme cycle {cycle + 1}: theme menuitem click failed " + f"({exc!r})" + ) break # Settle. The ".dark" class on is the ground # truth (theme-store toggles only that class); the @@ -541,9 +609,11 @@ with sync_playwright() as p: rgbs = [parse_rgb(o["bg"]) for o in observed if parse_rgb(o["bg"])] light_seen = any(min(r) > 220 for r in rgbs) dark_seen = any(max(r) < 60 for r in rgbs) + if len(observed) < 3: + soft_fail(f"theme toggle ran only {len(observed)} cycle(s), expected 3") if not (light_seen and dark_seen): - info( - f"WARN expected both light + dark backgrounds across " + soft_fail( + f"expected both light + dark backgrounds across " f"{len(rgbs)} cycles; light_seen={light_seen}, dark_seen={dark_seen}" ) else: @@ -557,13 +627,13 @@ with sync_playwright() as p: "button", name = re.compile(rf"^\s*{label}\s*$", re.I) ).first if btn.count() == 0: - info(f"nav '{label}' not found") + soft_fail(f"nav '{label}' not found") return False btn.click() page.wait_for_timeout(800) if expected_url_pat and not re.search(expected_url_pat, page.url): - info( - f"WARN clicking '{label}' didn't change url to /{expected_url_pat}; " + soft_fail( + f"clicking '{label}' didn't change url to /{expected_url_pat}; " f"current: {page.url}" ) return False @@ -681,6 +751,12 @@ with sync_playwright() as p: ) count_c = candidates.count() clicked_recent = False + # We sent the prompts ["Reply with exactly: hello", "What is 1+1?", + # "Reply with exactly: world", ...] above. The thread title that + # gets persisted is typically a snippet of the first user message + # (Studio summarises after a few turns). We accept either a literal + # word from one of our prompts OR a short Studio-summary heuristic. + PROMPT_KEYWORDS = ("hello", "world", "tree", "yes", "1+1", "2+2") for i in range(count_c): try: t = (candidates.nth(i).text_content() or "").strip() @@ -695,11 +771,29 @@ with sync_playwright() as p: shoot("15d-recent-clicked") info(f"OK clicked recent entry: {t[:60]!r}") clicked_recent = True + # Strict check: after clicking the Recents entry, the + # thread we land on must include at least one of our + # prompts in its rendered messages. Otherwise we + # navigated to a thread that wasn't ours. + turns_text = page.evaluate("""() => { + const els = document.querySelectorAll( + '[data-role="user"], [data-role="assistant"]' + ); + return Array.from(els).map(e => (e.innerText || '') + .toLowerCase()).join(' '); + }""") + if any(k in turns_text for k in PROMPT_KEYWORDS): + info("OK landed on a thread that includes our prompts") + else: + soft_fail( + "Recents-clicked thread doesn't contain any of our " + f"sent prompts; turns_text={turns_text[:120]!r}" + ) break except Exception: continue if not clicked_recent: - info("no Recents entry was clickable -- skipped") + soft_fail("no Recents entry was clickable") # Back to chat. page.goto(f"{BASE}/chat") composer = page.locator('textarea[aria-label="Message input"]')