Feat/backend smoke gate - #5
Merged
Merged
Conversation
added 30 commits
August 12, 2026 10:32
8 of 12 authenticated baselines captured something other than the page they named: a 404 (the route /evaluation/288 does not exist), stuck "Loading ..." curtains, a false-empty list, and four byte-identical copies of the logged-out marketing homepage (bug #15). Most sat under hard pixel-diff assertions, so they passed *because* the app was broken - ai_maker_dashboard would have gone red the day bug #14 was fixed. The root cause was the wait, not the missing check: captures settled a flat 2.5s after load while lessons.md documents curtains at 15-30s+, so they landed mid-curtain by design. Adds wait_for_render_settled (45s) plus a gate that refuses to save or compare a capture that is a 404, a curtain, logged-out chrome on an auth route, or blank. It skips rather than fails: an invalid capture means "did the pixels change?" was never asked, so the honest result is no signal. Also fixes the evaluation-detail route to use the completed_eval_id fixture, and masks the account name/initials that every authenticated baseline was embedding.
Four defects in the generated TEST_REPORT.md: - Duration read from summary["duration"], but pytest-json-report puts it at the top level, so every report claimed "0.00s". Per-test durations were 0.00s for the same reason - they live on the setup/call/teardown phases, not on the test object. - Pass rate counted skips in the denominator, so a run that skipped almost everything rendered as "0.0%" - a skip is the absence of a result, not a failed one. Now computed over executed tests, "n/a" when none ran. - A run where nothing executed reported "PASSED". An environment outage that skipped every test looked identical to a clean pass; it now reads "NO TESTS EXECUTED". - xfailed/xpassed were never counted, so on any xfail-using suite the rows silently failed to add up to the total.
The summary table printed needs.<job>.result per suite, which cannot express a test failure here: every suite runs pytest with continue-on-error: true, so the job succeeds no matter how many tests failed. The most-read signal in the run always said "success". Each suite now writes a named JSON report and uploads it; the summary aggregates them into per-suite pass/fail/skip/xfail counts, flags a suite where nothing executed, and keeps the job-status table in a collapsed block labelled as plumbing. Also bumps the visual baseline cache to a v2- generation. Editing tests/visual/** changes the primary key, but restore-keys would still fall back to the newest old cache - which holds the poisoned baselines, so CI would have diffed correct captures against 404s and homepage clones.
Phase 11 log plus per-baseline outcomes. Files #24 (auditor assignments hangs past 45s on "Loading your assignments...", a route neither #13 nor #14 covered). Flags a caveat worth acting on: #13/#14 did not reproduce this session - both the AI-Maker overview and the evaluations list loaded fully within 45s. That may be the merged backend perf work, or intermittency, but either way the visual suite's evidence for those rows was unreliable because it was capturing at 2.5s. Re-verify before removing their xfails.
Moves DEFAULT_MASKS / AUTH_PII_MASKS into utils/visual_guards so sibling visual suites use one definition - a new module that forgot the identity masks would quietly start baking the test account's name back into baselines. Also updates app_bugs.md #15: two consecutive runs hit the logged-out fallback on 12 of 24 authenticated captures (~50%, vs the 17-25% recorded in July), and the fallback now renders logged-out chrome (LOGIN / SIGN UP) rather than authenticated shell with wrong content. Both samples are small and conditions differed, so it is noted as materially worse under current conditions rather than a measured regression.
Responsive: mobile/tablet baselines for 4 high-traffic auth routes (role selector, AI-Maker overview, evaluations list, models list) plus a no-horizontal-overflow assertion at mobile width - a deterministic, non-pixel-diff check that catches the most common responsive bug directly. Verified: 10 passed, sensible skip/xfail, all 4 overflow checks passed. Surfaced a real bug: the evaluations table overflows horizontally at 768px, cutting off the Model column and the row-count control. Modal: element-scoped captures for the Know More info dialog (90f38dc) and the Start Evaluation modal's two steps, targeting the ModelSelectionModal refactor (130b5ac). Two interaction steps (evaluation-method radio, evaluator-type radio) intermittently time out against the live dev UI - wrapped to skip with a clear reason rather than fail, same pattern as completed_eval_id's UI-seeding fallback. Masks are shared via utils.visual_guards (DEFAULT_MASKS / AUTH_PII_MASKS) rather than redefined, so this coverage can't accidentally omit the identity masking that visual_regression.py relies on.
Targeted repro (poll body text until the loading-curtain regex clears or a 60s budget expires - the disambiguation procedure this file already prescribes) shows both routes rendering real content well within budget: the AI-Maker overview in 28.2s with populated stat cards, the evaluations list in 8.1s with real table headers. Second independent confirmation after Phase 11's visual-suite runs also failed to reproduce either hang the same day. Xfails referencing #13/#14 in the e2e/accessibility suites are not yet removed - that cleanup is a follow-up, per this file's own convention of re-running the linked test before dropping the xfail.
Applies to e2e-tests, accessibility-tests, visual-tests. The visual suite alone ran 11.5 min on a cache-cold run today (every baseline saving for the first time under the new v2- cache generation); 30 min was tight headroom for a slow or retry-heavy run. test-summary's separate 10-minute timeout is untouched - it does no browser work.
Both visual suites hit the same problem: the AI-Maker overview's stat cards (Evaluations Completed, etc.) and Recent Evaluations table change during normal platform use, so a hard pixel-diff assertion flapped red on ordinary re-runs. The responsive suite had already worked around this by adding the page to its non-deterministic xfail set - which silences diffs for the WHOLE page, including the static layout and model-card grid a real regression would land in. Adds route_masks() to utils.visual_guards: a per-route mask lookup, verified against the live DOM logged in as TEST_EMAIL_2 so the selectors aren't tied to one account's data. Masks only .metric-card-value and the live table rows; the four stat labels, headings, and model cards keep a real hard assertion. Wired into both the desktop and responsive suites, and ai_maker_dashboard is removed from the responsive suite's non-deterministic set now that it's genuinely stable - confirmed 4/4 passed on a same-session re-run with zero skips.
The health endpoints are now deployed to dev, so test_rejects_non_get
ran for real for the first time and failed: assumed 405 from
@require_GET, but Django's CsrfViewMiddleware intercepts a POST with
no CSRF cookie before that decorator ever runs, returning 403
('CSRF verification failed') instead. Confirmed live via direct curl.
Accept either code - the test's concern is that non-GET is rejected,
not which middleware rejects it first.
Experiment: both now depend only on lint instead of chaining accessibility-tests after api-tests. The serial chain existed because concurrent load against the shared dev backend used to produce pool exhaustion and widespread ReadTimeouts (docs/app_bugs.md #3/#13/#14). Two things changed since: the backend moved from runserver to Docker, and #13/#14 were independently re-verified fixed today. A local 2-suite concurrency smoke test today was inconclusive - it showed no pool-exhaustion symptoms, but was confounded by a second, unrelated CI run hitting the same backend at the same time (this branch has an open PR into main, so every push auto-triggers a run). Testing it in CI directly avoids that confound. visual-tests and e2e-tests are deliberately not included yet - widen gradually and watch for the ReadTimeout/stuck-curtain signature before going further.
The concurrent experiment (previous commit) started failing in CI within ~13 minutes of the real run. Reverting accessibility-tests to needs: api-tests, restoring the full serial chain. The run was cancelled rather than left to finish, so there's no captured job log of the actual failure mode yet - documented in docs/ci_workflow_notes.md along with what to gather before trying again.
The prior note said the concurrent run "started failing" without a captured log. Pulled the cancelled API Tests job's partial log (GitHub retains whatever uploaded before cancellation) and found a real, diagnostic failure: tests/api/test_audit_detail_api.py failed in a RERUN/RERUN/FAILED cluster with ~60s between each attempt - a timeout signature matching bug #3's fingerprint, not an assertion bug. Also corrects an overstatement: accessibility-tests and visual-tests both completed successfully in the same run despite running concurrently with the struggling api-tests job, so the slowdown was concentrated in api-tests' fixture-heavy queries, not universal across suites.
Visual Regression has always shown "no report found" regardless of whether the run passed - the summary script was looking for suite-artifacts/visual.json, but that artifact's upload step spans three top-level folders (reports/, screenshots/, snapshots/), so GitHub doesn't strip the reports/ prefix the way it does for api-test-report and accessibility-report (each a single common folder). Confirmed by downloading the actual artifact and inspecting its real layout: reports/visual.json.
Rapid pushes to an open PR were each auto-triggering a full run, and every run shares one dev backend - overlapping runs were contending with each other (api-tests hit 42m26s vs a 6m31s baseline while two runs overlapped, docs/ci_workflow_notes.md). Groups by PR number (or ref for push/workflow_dispatch), cancel-in-progress so a new push supersedes a stale run's results instead of letting it keep running against the shared backend.
Verified live: a superseded run's api-tests correctly cancelled via the new concurrency group, but accessibility-tests (if: always()) started right after anyway and kept running against the shared backend, ignoring the cancellation entirely - the exact caveat flagged when the concurrency group was added. Removed always() from accessibility-tests and visual-tests. Every test step already uses continue-on-error, so the upstream job's conclusion is "success" even when its tests fail - default success()-gating only actually changes behavior on a genuine infra failure or cancellation, both cases where skipping is correct. test-summary keeps always() (paired with !cancelled()) since it's a reporting job that must tolerate e2e-tests' deliberate skip while paused, not a test job. Not pushed yet - holding per instruction until current runs are stopped or told to push.
DOM value attrs on the step-2 evaluator-type radios changed from plain Technical/Domain/Cultural to the backend enum form (TECHNICAL_AUDIT/ DOMAIN_AUDIT/CULTURAL_AUDIT), so select_evaluator_type_in_modal timed out 30s on every call. Updated the locators and made get_checked_evaluator_type() map the raw enum back to the display label so existing assertions keep reading Technical/Domain/Cultural.
Root-caused in ParakhAI-frontend (origin/dev, NewEvaluationContent.tsx getEvaluatorLabel): the switch compares the raw AUDIT_TYPE enum against AUDIT_TYPE_LABELS values, which can never match, so every selection falls through to the default label. Introduced in commit 34f221c. xfail the three e2e assertions that hit it.
DOM value attr for the Playground radio changed from 'manual' to 'playground' (bulk unchanged). select_evaluation_method/is_method_selected kept the public 'bulk'/'manual' API used across ~20 call sites and translate internally to the real DOM values.
Both tests navigate with the unauthenticated page fixture, which redirects to Keycloak login before the editor's JS bundle (and its SyntaxError) ever loads -- so they were passing without observing anything, which fails loudly under xfail(strict=True) as a false XPASS. Added the same reachability skip guard the CDS-005 tests in this file already use, keyed on landing on the opub-kc auth domain.
The tab bar was replaced by a per-column 'Filter Status' popover on the DataTable in the evaluation-table-listing redesign (~2026-08); all STATUS_TAB_* locators now permanently match 0 elements. Added the new FILTER_STATUS_BUTTON/FILTER_DIALOG locators and rewrote EvaluationsPage.click_status_tab/get_status_filter_options/ is_status_filter_available to drive the popover. Updated all of TestStatusFilterTabs plus the two pagination tests that referenced the old tabs. Also filed bug #27: the Pending Review filter option shows its chip as applied but doesn't actually filter rows (Draft/Completed filtering works correctly), and xfailed the one test that catches it.
The list can render its first (cached) page of rows before a freshly-created draft has synced in -- waiting on any EVAL_TABLE_ROW match returns too early. Wait on the draft's own href instead.
Phase 12 marked it fixed after one clean check, but a group-2 e2e run today genuinely XFAILed two of the four related tests while the other two XPASSed in the same run -- and an immediate isolated re-run of the XFAILing pair then XPASSed cleanly. That's inconsistent both ways, not the bug #3 concurrency-noise shape. Keeping all four strict=False xfails in place rather than removing them.
test_evaluations_list_controls.py still assumed the removed StatusFilterTabs '(N)' count badges -- they have no equivalent in the redesigned Filter Status popover, so narrowed those assertions the same way the sibling test_evaluations.py fixes did. Also found: (1) bug #28 -- the first click on a sortable column header is a no-op (aria-sort never updates, rows don't reorder); only the second click engages sorting. (2) Broadened bug #27 -- the Filter Status popover's intermittent no-op isn't Pending-Review-specific, a same-day run reproduced it for Completed/Draft/Failed too. xfailed the affected tests referencing both rows.
test_new_evaluation_regression.py still used the removed STATUS_TAB_* tab-bar locators directly; migrated to EvaluationsPage.click_status_tab. While fixing it, found click_status_tab itself only checked new checkboxes without unchecking previous ones -- since the popover is a real multi-select group, calling it twice (Draft then Completed) would silently accumulate both filters instead of switching. Fixed to clear all checked boxes before checking the target one. Also xfailed a new bug #26 recurrence in TestEvaluationTypeRadio and extended that row's test list.
Live repro (open Add Evaluator dialog, click Cancel) confirmed no phantom success toast appears anymore. Was XPASS(strict)-failing the suite; verified genuinely fixed rather than trusting the single run.
Three locators mixed a text=/regex/flags selector with plain CSS in one comma-joined string, which Playwright can't parse (documented pattern, tasks/lessons.md 2026-05-18/2026-07-28) -- rewrote all three using get_by_text(re.compile(...)).or_(css_locator) instead. Also fixed _nav_to_eval_detail's fixed 3.5s wait, which was far too short for the documented 15-30s+ curtain and only survived because most callers here soft-retry-then-xfail on empty results; now polls for curtain-absence AND a positive 'Evaluation Overview' content marker (content can flash empty in between, same caveat as wait_for_wizard_loaded).
pytest.ini sets xfail_strict=true globally. This test's xfail for bug #15 (documented as ~50% intermittent, not always-reproducing) had no strict=False override, so a lucky run's XPASS reported as a suite failure -- exactly the problem bug #14's xfails were already fixed to tolerate. Confirmed no other e2e test references bug #15 by id.
added 5 commits
August 14, 2026 13:22
XPASSed (strict) in the 2026-08-14 CI run, failing the suite over an intermittent a11y improvement rather than a regression. Had no app_bugs.md row at all, violating this file's own convention. Filed #29 and added strict=False, matching how #14/#15/CDS-001 were already handled today for the same reason.
1.085% diff in the 2026-08-14 CI run, vs a 0.2% threshold. Root cause: completed_eval_id picks whichever completed audit it discovers/seeds each run, so the page's actual content (evaluation name, scores, sample-issue text) varies run to run - most of the page, not a few numbers a mask could absorb. xfail-on-diff (matching the existing pattern for other live-data pages) is the right tool here, not masking.
The job summary showed real counts like 176 as "***76" - some repo secret's value (unknown, secrets are write-only) collides with digit substrings in real test counts, and GitHub masks any exact substring match anywhere in workflow output, not just where the secret was meant to be used. Confirmed cosmetic-only earlier (real numbers always recoverable from the JSON artifacts), but the summary table is the whole point of this script, so it should render correctly on its own. Joins every printed count and duration with a zero-width space (chr(0x200B), not a literal invisible character in source - that version got caught in review since it's unverifiable in a diff and fragile to whitespace-trimming tools) between characters. Breaks the contiguous-substring match masking relies on; renders identically to a human in both the terminal and GitHub's Markdown-rendered summary.
This reverts commit 20b3ab9.
Confirmed with the user: its value is "1" (CivicDataLab's org id, not sensitive). GitHub was masking every literal "1" digit anywhere in the job summary because it exactly matched this secret's value - that's what caused ***76/***5/***0 etc regardless of what actually ran. Scoped narrowly to this file only: added a new repo Variable (coexists with the existing secret under the same name, no conflict) and changed just this one reference from secrets. to vars.. The actual secret is untouched and still used as-is by run-smoke.yml (the reusable workflow ParakhAI-frontend's CD pipeline calls) and scheduled.yml - no reason to touch either for a fix scoped to this file's own job summary rendering. A prior attempt to defeat the masking with zero-width spaces between digits (20b3ab9) did not work and was reverted (68d29a6) - GitHub evidently normalizes/strips invisible characters before matching secrets, which is the correct hardening on their end. This is the real fix, not a workaround.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.