fix(gnome-e2e): source the results summary from scripts/e2e_summary (#797)

[mendezr]

What

scripts/e2e_summary.py is the unit-tested source of truth for E2E scenario
counting and the pass/fail headline — e2e.yml imports it. The public
gnome-e2e composite action reimplemented the same logic inline and decided its
headline on failed == 0, so a behave run in which nothing passed rendered:

E2E PASSED: 0 passed / 0 failed / 0 skipped (total 0)

plus a green ✅ E2E Results job summary. That is the exact outcome
is_success() was written to prevent (tests/unit/test_e2e_summary.py::test_undefined_only_run_is_not_success).
Because gnome-e2e is a public entry point that no workflow in this repo uses,
only downstream consumers ever exercised the divergent copy.

How

• Add scripts to the action's _testsuite sparse checkout (cone mode, so the
  directory pattern is enough) and import count_scenarios, is_success and
  summary_icon from it in the Summarise results step. The inline counting and
  headline logic is deleted; the SLA-violation block (genuinely action-specific)
  stays inline.
• Headline: PASSED only when is_success holds — an undefined-only,
  untested-only or errored run now reports INCOMPLETE / ⚠️. The job-summary
  icon comes from summary_icon().
• Undefined / untested / other scenarios are counted, rendered as their own
  table columns, and included in Total instead of being dropped.
• The step fails closed: if scripts/e2e_summary.py is not importable it prints
  an error and writes a ⚠️ Summary unavailable summary rather than a green one.
  It still never exits non-zero, so the reporting step stays reporting-only.
• One sentence added to docs/skills/ci-ops/e2e-workflow/SKILL.md noting the
  composite action is now a consumer of the same module.

Tests

New tests/unit/test_gnome_e2e_action_summary.py executes the action step's
embedded script (extracted from action.yml, so the copy under test is the
copy that ships) against synthetic results.json payloads:

• undefined-only / untested-only / errored-with-zero-failures runs: no
  PASSED, no ## ✅ heading, and the undefined count is present in the total
• all-passed run: still E2E PASSED / ## ✅
• failed run: E2E FAILED / ## ❌
• missing scripts/e2e_summary.py: ⚠️ Summary unavailable, never green
• structural guards: the step imports the module (and checks out scripts/) and
  does not redefine the counting or the headline

Checked locally that the extracted pre-fix script fails the undefined-only case
(it prints E2E PASSED ... (total 0) and ## ✅ E2E Results) while the new
script passes.

Interface / breakage

No change to the action's inputs or outputs — internals plus one sparse-checkout
path only, so nothing for consumers to adapt to.

Fixes #797. Supersedes #812, whose inline except ImportError fallback
reintroduced the fork and whose embedded Python was syntactically broken
(literal newlines inside strings, duplicated PYEOF).

— hive: backend=goose model=deepseek/deepseek-v4.1-flash

[castrojo]

LGTM, single sources results summary logic and fixes coverage snapshot.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[castrojo]

Backlog triage pass. I have not approved and have not merged or enqueued anything: .github/copilot-instructions.md says agents never approve, merge, or enqueue, and AGENTS.md:136,163 + docs/skills/meta/human-gates/SKILL.md make merge a human gate. This is a review comment only.

Two things were blocking this that are now cleared.

First, the conflict. The only thing making this CONFLICTING was the docs/skills/test-authoring/suite-map/SKILL.md hunk, which proposed 526 ... 415 -> 413 active ... 111 -> 113 and common 101/20 -> 99/22 — scope creep on generated content that main has since moved past (it now reads 519 scenarios across 72 feature files: 406 active, 0 quarantined, 113 with common 121 | 99 | 0 | 22 already applied). Rebasing onto main and re-running python3 scripts/update_coverage_snapshot.py dropped that hunk to nothing, which is exactly the right resolution: the branch is now the action change, the e2e-workflow skill note, and the new test, and nothing else. Your other two hunks still applied cleanly — action.yml:442 and :469 on main still decide on failed == 0, and the _testsuite sparse checkout at :75-78 still lacks scripts.

Second, CI had never run: the workflow runs sat at conclusion: action_required because this is a fork PR. I approved the three pending runs, so the checks are executing now.

The change itself is the right shape. Making the public gnome-e2e action consume scripts/e2e_summary.py instead of reimplementing the headline removes a real duplicate, and tests/unit/test_gnome_e2e_action_summary.py extracting and executing the action's own embedded PYEOF script — rather than a paraphrase of it — is the only way that test would actually catch drift. Stripping COVERAGE_*/PYTEST_* from the child env and including a negative case (missing module => not green) are both right. The cone-mode claim checks out too: the _testsuite checkout sets sparse-checkout-cone-mode: true at action.yml:78, so the bare scripts pattern is sufficient — the "enumerate every file" rule at e2e-workflow/SKILL.md:156 applies to e2e.yml's checkout, not this one.

One coupling to sequence: #796 adds tests/unit/test_timing_contract.py, which parses entry.get("…") calls out of this same action's summariser. This PR leaves the SLA/timings block inline so that guard should still hold, but whoever lands second should re-run it.

Note castrojo's approval predates my push, so under require_last_push_approval it no longer counts toward the threshold.

Outstanding: two approving reviews, plus the repo-wide ghost-lab outage.

Gate status for this PR

• Ruleset main — merge queue requires 2 approving reviews (dismiss_stale_reviews_on_push: true, require_last_push_approval: true, empty bypass_actors). Required checks: Lint & syntax, Behave dry-run, pytest.
• I pushed to this branch, so under require_last_push_approval the two approvals must both come from someone other than me.
• ghost-lab is red repo-wide and that is not this PR's fault: every lab workflow in the Argo namespace is Failed/Error since 2026-09-15 across testsuite, common, knuckle and bluefin-lts; the last success anywhere was testsuite #790 on 2026-09-15. docs/skills/meta/human-gates/SKILL.md:80-107 still makes it a merge gate, so I am naming it rather than waiving it.
• On AGENTS.md:146 (matching docs/skills/** update): AGENTS.md calls it a mandatory gate, docs/skills/ci-ops/contributing/SKILL.md:78 calls it a review expectation that no CI job enforces. I filed #861 for that contradiction rather than guessing which reading binds.

[kubestellar-hive]

Reviewed — no findings from this perspective.

— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.78

[castrojo]

Reviewed — +1 of 2. A second independent reviewer is still required; not merging.

Replaces the reimplemented scenario counting in .github/actions/gnome-e2e/action.yml's Summarise results step with an import of scripts/e2e_summary.py — the copy that tests/unit/test_e2e_summary.py actually covers.

The bug this closes is the dangerous class. The action's inline counter treated "not background" as a scenario and only ever incremented on passed/failed/skipped. A run where every scenario came back undefined or untested therefore counted zero failures and rendered a ✅ headline — a green report for a run that proved nothing (#797). A test suite that reports success while silently executing nothing is the worst failure mode a QA repo has, and it was being produced by the summariser itself.

The fix is structurally right, not just a patch: the definition of "passed" now lives in exactly one place, and that place is unit-tested. INCOMPLETE as a distinct status word, and summary_icon() returning ⚠️ rather than ✅ or ❌, means an undefined-only run is now visibly neither.

The import-failure path is the detail I want to highlight as correct. except ImportError does not fall back to the old inline logic — it writes "Summary unavailable" to the job summary and exits. That is failing closed. Falling back would have quietly restored the bug this PR exists to remove, and it is the obvious tempting mistake here.

Sparse-checkout is handled, which is the thing that would otherwise make this fail at runtime while passing every check: the action's checkout is cone-mode and now includes scripts, so scripts/e2e_summary.py is actually present in _testsuite for the sys.path.insert to find.

Interaction to flag for the second reviewer: #854 changes the e2e.yml job-summary heading to drop the GNOME version, citing alignment with this action's heading format. The two are consistent in intent; neither blocks the other, but whoever lands the second should re-read the pair.

Outstanding: one more approval.

[Danathar]

The action change looks right to me: it imports count_scenarios, is_success and summary_icon from scripts/e2e_summary.py, adds scripts to the sparse checkout, fails closed on ImportError, and the new tests execute the embedded step script. The unit suite passes with this merged onto current main (1642 passed).

One blocker before I can approve. main's docs/skills/ci-ops/e2e-workflow/SKILL.md is already at exactly 500 lines, and this PR's edit there is a net +2, so after merging the file is 502 lines and python3 scripts/validate_docs.py fails with SKILL.md exceeds 500 lines (502). PR CI is green only because the branch is based on a 498-line version of the file. Could you rebase and reword that paragraph to fit in the same number of lines, or trim two lines elsewhere in the file? I will approve and enqueue once that is in.