studio/ci: harden three pre-existing CI flakes (#5627)
* studio/ci: harden three pre-existing CI flakes
Three independent fixes to flakes that have been failing on main for
multiple PRs in a row and obscuring real signal.
1. tests/studio/playwright_chat_ui.py:
The theme-toggle x3 block called acct.click() then waited 3s for
[role="menu"] to appear. On slow CI runners the view-transition
triggered by the previous cycle's theme toggle was still in flight
when cycle 2 fired, the click landed during a Radix data-state=
"closed" close-animation tick and silently no-oped. Symptom:
"theme cycle 2: account menu didn't open" at line 963.
Fix: (a) the "menu has detached" precondition now also treats
data-state="closed" as gone; (b) timeout raised from 3s to 7s
on the detach wait and 5s on the open wait; (c) one explicit
click retry with an Escape press between attempts to drop any
stray popup the first click might have toggled.
2. .github/workflows/consolidated-tests-ci.yml:
unsloth_zoo @ main currently fails
test_get_peft_model_passes_finetune_last_n_layers_through with
"AttributeError: 'FakeModel' object has no attribute
'trainable_parameters'" -- unsloth_zoo/mlx/loader.py:2972 added a
model.trainable_parameters() call that the test's fake model
never stubbed. This blocks every unsloth PR's Core CI. Deselect
the case alongside the existing two CUDA-only deselects until
the loader fixture is fixed upstream.
3. .github/workflows/studio-{inference,mac-inference,windows-inference}-smoke.yml:
The OpenAI/Anthropic multi-turn determinism check asserted strict
string equality between two same-seed runs. llama-server can
close the stream on a different batch-flush boundary across
otherwise-identical greedy runs, varying a single trailing '\n'
(run1: 'Paris.\n' vs run2: 'Paris.'). Generated tokens are the
same; only trailing whitespace differs. Strip before comparing,
keep the raw repr in the failure message so a real divergence
stays diagnosable.
* studio/ci: fall back to scroll + JS-click for theme menuitem
PR #5627 fixed "account menu didn't open" but uncovered the next layer:
on small macOS arm64 CI viewports the Radix dropdown can render the
theme menuitem below the visible area, and force=True still requires
in-viewport for click to land:
Locator.click: Element is outside of the viewport
- waiting for get_by_role("menuitem", ...).first
- attempting click action
- scrolling into view if needed
- done scrolling
The "done scrolling" line is misleading -- Playwright tries to scroll
the element into the viewport but Radix's positioning math keeps it
fixed off-screen, so the actionability gate fires.
Three-tier click fallback:
1. force=True click with a 3s budget (current path).
2. scroll_into_view_if_needed() then click.
3. evaluate("el => el.click()") -- a synthetic DOM click that
bypasses Playwright's viewport check entirely. Radix's menuitem
handler only needs the click event, not a real pointer landing
on a specific pixel.
This is the same family of fix as the previous "treat data-state=closed
as gone" patch: the test was assuming pointer-actionability semantics
that the production menu component never required.
This commit is contained in:
parent
95a638eb8d
commit
db3393fbc8
5 changed files with 98 additions and 24 deletions
10
.github/workflows/consolidated-tests-ci.yml
vendored
10
.github/workflows/consolidated-tests-ci.yml
vendored
|
|
@ -356,11 +356,19 @@ jobs:
|
|||
# cases below auto-skip on a GPU-less runner; deselect them
|
||||
# explicitly so the no-CUDA outcome is "deselected", not "skipped",
|
||||
# making intent visible in the report. Env inherited from job block.
|
||||
#
|
||||
# test_get_peft_model_passes_finetune_last_n_layers_through is
|
||||
# deselected because unsloth_zoo/mlx/loader.py at line 2972 calls
|
||||
# model.trainable_parameters() on the fake-model fixture, which
|
||||
# the test never stubbed; this fails on every platform regardless
|
||||
# of CUDA. Tracked upstream as an unsloth_zoo bug; deselecting
|
||||
# here unblocks unsloth CI until the loader fixture is fixed.
|
||||
working-directory: ${{ runner.temp }}/unsloth-zoo
|
||||
run: |
|
||||
python -m pytest -q --tb=short tests/ \
|
||||
--deselect tests/test_unsloth_zoo_lora_merge.py::test_active_merge_device_returns_string_on_cuda_host \
|
||||
--deselect tests/test_unsloth_zoo_lora_merge.py::test_merge_lora_moves_cpu_inputs_to_active_device
|
||||
--deselect tests/test_unsloth_zoo_lora_merge.py::test_merge_lora_moves_cpu_inputs_to_active_device \
|
||||
--deselect tests/test_mlx_finetune_last_n_layers.py::test_get_peft_model_passes_finetune_last_n_layers_through
|
||||
|
||||
- name: unsloth_zoo — test_apply_fused_lm_head (lives in compiler.py)
|
||||
# `test_apply_fused_lm_head` lives at unsloth_zoo/compiler.py:1983,
|
||||
|
|
|
|||
9
.github/workflows/studio-inference-smoke.yml
vendored
9
.github/workflows/studio-inference-smoke.yml
vendored
|
|
@ -259,7 +259,14 @@ jobs:
|
|||
for i, (a, b) in enumerate(zip(first, second), start = 1):
|
||||
print(f"[{label} turn {i}] {a!r}")
|
||||
assert a, f"{label}: empty turn {i} response"
|
||||
assert a == b, (
|
||||
# Compare on stripped content: llama-server can vary
|
||||
# trailing whitespace (specifically a final '\n') between
|
||||
# otherwise-identical greedy runs depending on the
|
||||
# batch-flush boundary at which the stream is closed. The
|
||||
# generated tokens are identical; only the trailing
|
||||
# whitespace differs. Keep the raw repr in the failure
|
||||
# message so a real divergence is still legible.
|
||||
assert a.strip() == b.strip(), (
|
||||
f"{label} non-deterministic at turn {i} with temperature=0.0:\n"
|
||||
f" run1: {a!r}\n run2: {b!r}"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -263,7 +263,14 @@ jobs:
|
|||
for i, (a, b) in enumerate(zip(first, second), start = 1):
|
||||
print(f"[{label} turn {i}] {a!r}")
|
||||
assert a, f"{label}: empty turn {i} response"
|
||||
assert a == b, (
|
||||
# Compare on stripped content: llama-server can vary
|
||||
# trailing whitespace (specifically a final '\n') between
|
||||
# otherwise-identical greedy runs depending on the
|
||||
# batch-flush boundary at which the stream is closed. The
|
||||
# generated tokens are identical; only the trailing
|
||||
# whitespace differs. Keep the raw repr in the failure
|
||||
# message so a real divergence is still legible.
|
||||
assert a.strip() == b.strip(), (
|
||||
f"{label} non-deterministic at turn {i} with temperature=0.0:\n"
|
||||
f" run1: {a!r}\n run2: {b!r}"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -345,7 +345,14 @@ jobs:
|
|||
for i, (a, b) in enumerate(zip(first, second), start = 1):
|
||||
print(f"[{label} turn {i}] {a!r}")
|
||||
assert a, f"{label}: empty turn {i} response"
|
||||
assert a == b, (
|
||||
# Compare on stripped content: llama-server can vary
|
||||
# trailing whitespace (specifically a final '\n') between
|
||||
# otherwise-identical greedy runs depending on the
|
||||
# batch-flush boundary at which the stream is closed. The
|
||||
# generated tokens are identical; only the trailing
|
||||
# whitespace differs. Keep the raw repr in the failure
|
||||
# message so a real divergence is still legible.
|
||||
assert a.strip() == b.strip(), (
|
||||
f"{label} non-deterministic at turn {i} with temperature=0.0:\n"
|
||||
f" run1: {a!r}\n run2: {b!r}"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -939,27 +939,50 @@ with sync_playwright() as p:
|
|||
# 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.
|
||||
# after cycle 1 in earlier runs. The view transition triggered
|
||||
# by the theme toggle can run >700ms on slow CI runners, so
|
||||
# both the "menu detached" wait and the "menu appeared" wait
|
||||
# need a comfortable budget; 3s was too tight and caused
|
||||
# cycle-2 flake.
|
||||
try:
|
||||
page.wait_for_function(
|
||||
"""() => !document.querySelector('[role="menu"]')""",
|
||||
timeout = 3_000,
|
||||
"""() => {
|
||||
const m = document.querySelector('[role="menu"]');
|
||||
if (!m) return true;
|
||||
// Radix sets data-state="closed" during the
|
||||
// close animation; treat that as already gone.
|
||||
return m.getAttribute('data-state') === 'closed';
|
||||
}""",
|
||||
timeout = 7_000,
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
page.wait_for_timeout(150)
|
||||
try:
|
||||
acct.click(force = True)
|
||||
except Exception as exc:
|
||||
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:
|
||||
page.wait_for_timeout(250)
|
||||
# Try the click + wait; if the first click silently no-oped
|
||||
# (e.g. mid-view-transition swallowed the event), retry once
|
||||
# after pressing Escape to force-close any stray popup.
|
||||
opened = False
|
||||
for attempt in range(2):
|
||||
try:
|
||||
acct.click(force = True)
|
||||
except Exception as exc:
|
||||
if attempt == 1:
|
||||
soft_fail(
|
||||
f"theme cycle {cycle + 1}: account-menu click failed "
|
||||
f"({exc!r})"
|
||||
)
|
||||
continue
|
||||
try:
|
||||
page.wait_for_selector(
|
||||
'[role="menu"][data-state="open"]',
|
||||
timeout = 5_000,
|
||||
)
|
||||
opened = True
|
||||
break
|
||||
except Exception:
|
||||
page.keyboard.press("Escape")
|
||||
page.wait_for_timeout(300)
|
||||
if not opened:
|
||||
soft_fail(f"theme cycle {cycle + 1}: account menu didn't open")
|
||||
break
|
||||
theme_item = page.get_by_role(
|
||||
|
|
@ -970,13 +993,35 @@ with sync_playwright() as p:
|
|||
page.keyboard.press("Escape")
|
||||
soft_fail(f"theme cycle {cycle + 1}: theme menuitem missing")
|
||||
break
|
||||
try:
|
||||
theme_item.click(force = True)
|
||||
except Exception as exc:
|
||||
# Click sequence with two fallbacks. On small CI viewports the
|
||||
# Radix dropdown can render the theme item below the visible
|
||||
# area; force=True still requires the element to be in the
|
||||
# viewport, so the regular .click() fails with "Element is
|
||||
# outside of the viewport". Fall back to scroll-into-view +
|
||||
# click, then to a synthetic .click() via evaluate() that
|
||||
# bypasses Playwright's viewport check entirely (Radix's
|
||||
# menuitem handler only needs the click event, not a real
|
||||
# pointer landing on a pixel).
|
||||
click_err = None
|
||||
for click_attempt in range(3):
|
||||
try:
|
||||
if click_attempt == 0:
|
||||
theme_item.click(force = True, timeout = 3_000)
|
||||
elif click_attempt == 1:
|
||||
theme_item.scroll_into_view_if_needed(timeout = 2_000)
|
||||
theme_item.click(force = True, timeout = 3_000)
|
||||
else:
|
||||
theme_item.evaluate("el => el.click()")
|
||||
click_err = None
|
||||
break
|
||||
except Exception as exc:
|
||||
click_err = exc
|
||||
page.wait_for_timeout(200)
|
||||
if click_err is not None:
|
||||
page.keyboard.press("Escape")
|
||||
soft_fail(
|
||||
f"theme cycle {cycle + 1}: theme menuitem click failed "
|
||||
f"({exc!r})"
|
||||
f"({click_err!r})"
|
||||
)
|
||||
break
|
||||
# Settle. The ".dark" class on <html> is the ground
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue