diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 59b44804..8e2c370a 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -402,6 +402,11 @@ on: required: false type: string default: "[]" + e2e-skip-blocking: + description: "Make the E2E skip-discipline check a hard failure for this app. It reads the RUN's Playwright report — the only place a runtime `test.skip(cond, reason)` is visible — and reports three things: a spec file that executed ZERO tests, a skip deferring to a deploy state CI itself decides ('not deployed', 'deploy drift', 'predates'), and a skip carrying no reason. A skip naming a genuinely absent optional app or external service is allowed. Default false because eight apps carry findings today (measured 2026-08-24: 298/3210 tests skipped fleet-wide, 27 zero-test spec files) and a gate that turns them all red at once is a gate nobody can turn on — same reasoning as check-code-blocking. Flip it per app once that app is worked down; it can then only stay clean." + required: false + type: boolean + default: false enable-coverage-guard: description: "Enforce the coverage ratchet — READ-ONLY, never commits. On a PULL REQUEST the merge base is measured in the phpunit job and the change must not reduce coverage against it; the floor is measured, not typed, so it cannot go stale and there is nothing for an author to keep up to date. On a PUSH the committed .coverage-baseline is enforced as a conservative fail-safe floor (below it fails; above it is fine). PRs may raise that floor but never lower it. Requires scripts/coverage-guard.php supporting --against; the workflow probes for it and fails loudly rather than silently downgrading the check." required: false @@ -4856,6 +4861,70 @@ jobs: SPEC_COVERAGE_JS node /tmp/spec-coverage.js + # ── e2e skip discipline: read the REPORT, not the source ─────────────── + # + # Every other e2e check in this file reads `tests/e2e/**` source. gate-19 + # parses `test.skip(...)` statically and is careful about it. What no + # source reader can settle is the form the fleet actually uses: + # + # const present = await someLiveQuery(page) + # test.skip(!present, 'Members tab not deployed on this instance') + # + # At source level that is indistinguishable from a test that runs. Only + # the report of an actual run knows. Nothing read the report, so measured + # 2026-08-24 across 20 apps' latest GREEN `development` runs: 298 of 3210 + # tests skipped (9.3%), 27 spec files executing ZERO tests, 61 skips + # deferring to a deploy state CI itself decides, and 142 carrying no + # reason at all. decidiq 29.8%, dossiq 27.7%, buildiq 26.3%. + # + # A zero-test spec file is still accepted by gate-19 as the `@e2e` anchor + # for an openspec scenario, so a scenario can be "covered" by a file that + # never runs. + # + # `!cancelled()` and not `always()`: a cancelled run has no verdict to + # explain, and a capture hung off a cancelled job produces nothing exactly + # when you need it. It deliberately runs when the suite PASSED — a green + # suite is where skips hide, and running only on failure would reproduce + # the blindness this step exists to remove. + # + # REPORT, NOT FAIL, on day one, exactly as app:check-code above: eight + # apps carry findings today, and turning them all red at once makes it a + # gate nobody can turn on. `e2e-skip-blocking` flips it per app as that + # app is worked down. + - name: E2E skip discipline + if: ${{ !cancelled() }} + continue-on-error: ${{ !inputs.e2e-skip-blocking }} + run: | + set -euo pipefail + GATE="" + for candidate in \ + "server/apps/${{ inputs.app-name }}/vendor/conduction/hydra-gates/scripts/lib/check_e2e_skips.py" \ + "hydra-gates/scripts/lib/check_e2e_skips.py"; do + if [ -f "${candidate}" ]; then GATE="${candidate}"; break; fi + done + if [ -z "${GATE}" ]; then + echo "::warning::check_e2e_skips.py not found (hydra-gates not vendored here yet) — skip discipline NOT measured on this run." + exit 0 + fi + + REPORT="" + for candidate in \ + "server/apps/${{ inputs.app-name }}/playwright-report" \ + "server/apps/${{ inputs.app-name }}/tests/e2e/playwright-report"; do + if [ -f "${candidate}/index.html" ]; then REPORT="${candidate}"; break; fi + done + if [ -z "${REPORT}" ]; then + # An absent report is an absent measurement, never a pass. Said + # loudly, because this is the one failure mode that would quietly + # turn the whole gate into a no-op. + echo "::error::no Playwright HTML report found — skip discipline could not be measured. Ensure the 'html' reporter is configured." + exit 1 + fi + + MODE=report + if [ "${{ inputs.e2e-skip-blocking }}" = "true" ]; then MODE=enforce; fi + python3 "${GATE}" --report "${REPORT}" --mode "${MODE}" --summary "${GITHUB_STEP_SUMMARY}" + - name: Upload Playwright report if: always() # `tests/e2e/…` is included because that is where the scaffolded config diff --git a/hydra-gates/scripts/lib/check_e2e_skips.py b/hydra-gates/scripts/lib/check_e2e_skips.py new file mode 100644 index 00000000..1deee693 --- /dev/null +++ b/hydra-gates/scripts/lib/check_e2e_skips.py @@ -0,0 +1,387 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: EUPL-1.2 +"""E2E skip discipline — reads the RUN's Playwright report, not the source. + +Every other e2e gate in this repo reads `tests/e2e/**` source. That is exactly +what this gate cannot do, and why it exists. + +`check_e2e_coverage.py` (gate-19) already parses `test.skip(...)` statically and +is careful about it: it separates an unconditional `test.skip(true)` from a +conditional `test.skip(cond, reason)`, and issue #239 tightened the case of an +unconditional skip hidden inside an `if` guard. What no source reader can settle +is the form the fleet actually uses: + + const present = await someLiveQuery(page) + test.skip(!present, 'Members tab not deployed on this instance') + +At source level that is indistinguishable from a test that runs. Only the report +of an actual run knows whether it executed. Nothing read the report, so the +fleet accumulated 298 skipped tests across 3210 (9.3%) and 27 spec files that +execute ZERO tests — every one of them on a GREEN run. + +MEASURED 2026-08-24 by this script, against the latest green `development` run +of each of 20 fleet apps. Fleet totals: 298/3210 skipped, V1=27, V2=61, V3=142. +The eight apps carrying it: + + buildiq 68/259 (26.3%) 6 zero-test specs + decidiq 57/191 (29.8%) 1 + dossiq 38/137 (27.7%) 5 + pipelinq 40/324 (12.3%) 3 + learniq 27/457 (5.9%) 6 + shillinq 21/345 (6.1%) 3 + integriq 16/191 (8.4%) 2 + filinq 9/124 (7.3%) 1 + +(An earlier hand-rolled count of mine read double these, by summing the +report's own `report.json` roll-up alongside the per-file entries. This script +skips `report.json` for that reason — the per-file entries are the whole truth +and the roll-up repeats them.) + +Three findings, in the order they matter. + +**V1 — a spec file that executed zero tests.** It contributes nothing and reads +as coverage. Worse, gate-19 accepts it as the `@e2e` anchor for an openspec +scenario, so a scenario can be "covered" by a file that never runs. + +**V2 — a skip that defers to a deploy state CI fully determines.** These are +real, copied verbatim from green runs: + + "decidiq not deployed on the shared instance — live-run deferred" + "Members tab not deployed on this instance" + "Deploy drift: the deployed decidiq predates minutes-ui-v1" + "Sub-cases tab not present in the deployed build (deploy mismatch)" + "leaves PR not deployed yet" + +Every one was written for a SHARED DEV INSTANCE, where the deployed build really +can lag the test. CI is not that, and the claim fails there in one of two ways: + +* About the app under test, it is **impossible**. The app IS the head commit, so + "not deployed" and "deploy drift" cannot happen, and the same sentence can + only mean the feature is missing or broken. +* About a dependency (decidiq's 46 all say "leaves PR not deployed yet", meaning + the pinned `nextcloud-vue`), it is **deterministic**. The lockfile decides it, + identically on every run. A test that quietly stands down because the pinned + dependency is too old is not waiting for anything: either bump the dependency + or drop the test. + +Either way the guard is an unconditional escape hatch in the one environment +where the condition is not a matter of luck. Note the shape of decidiq's worst +case — `partial registry: 27/29 providers` skips an exact-count assertion, so a +registry that is 93% correct reads as "not deployed" rather than as a diff. + +**V3 — a skip with no reason.** 142 of the 298 carry no description at all. A +reason is already required by gate-16 and gate-19 for their exclusions; a skip +is an exclusion decided at run time and the same rule applies. Without one there +is nothing to review and nothing to expire. + +NOT a finding: a skip that names an optional Nextcloud app or external service +the CI instance genuinely does not have ("No chat backend reachable", "requires +a TaskProcessing LLM provider"). Those describe a real absence the app does not +control. They must still carry a reason (V3), but they are allowed to skip. + +Modes, following the `app:check-code` precedent in quality.yml: `report` always +exits 0 and prints the table, `enforce` exits 1 on any violation. Day one is +report-only per app, because a gate that turns eight apps red at once is a gate +nobody can turn on. +""" + +from __future__ import annotations + +import argparse +import base64 +import io +import json +import os +import re +import sys +import zipfile +from pathlib import Path + +# Reasons that defer to a deploy state CI fully determines — impossible for the +# app under test, deterministic for a pinned dependency. Each entry was taken +# from a real skip annotation on a green fleet run, not invented. Matched +# case-insensitively against the whole reason string. +DEPLOY_STATE = ( + "not deployed", + "not yet deployed", + "deploy drift", + "deploy mismatch", + "not present in the deployed build", + "predates", + "not yet wired", + "deployed build", + "on the shared instance", + "in target instance", + "on this instance", +) + +# A reason may name a genuinely absent optional dependency even though it also +# trips a phrase above ("no Talk backend reachable on this instance"). These +# phrases mark that shape and are checked FIRST, so a real environment gap is +# never reported as a deploy claim. +ENVIRONMENT_ABSENCE = ( + "not reachable", + "no chat backend", + "unreachable", + "credential", + "llm provider", + "not provisioned", + "not installed", + "session does not survive", +) + +# Tolerate further attributes between the id and the closing bracket. Playwright +# writes `' + "" +) +io.open(os.path.join(out, "index.html"), "w", encoding="utf-8").write(html) +PY +} + +# expect