randomparity / randomparity/kdive

Zero-proof guards match the whole pytest stream, so a skipped tier can still exit 0

Open
#2,584 0 comments 0 reactions 0 assignees View on GitHub
area:build-install effort:M priority:P1 risk:night-safe status:ready type:bug
Dominant language
Python
Stars
0
Forks
0
Avg merge
1h 26m
Merged PRs (30d)
311

Description

## Problem

Four zero-proof guards match ` passed` against the **whole captured pytest stream** instead of
the summary line, so any line anywhere in the output that contains the pattern satisfies the guard.
A run in which zero tests passed can exit 0 through a guard whose entire purpose is to make that
impossible. A fifth recipe, `just test-live-remote`, has no such guard at all.

This is the fourth occurrence of one class — a live tier reading green without proving anything —
and the first in which the *remedy* is what fails. #2048 closed it for the hosted TCG spine, #2517
for the `test-live-tcg` recipe, #2518 for the native spine's env contract, and #2540 for the native
`test-live` recipe. Each built the same expression, so the defect was copied forward four times.
Filing one sweep rather than a fifth occurrence.

Nothing is silently green today. This is latent: no skip reason currently in the tree contains the
pattern. It is one unlucky skip-reason wording away, and the wording lives in test files that
ordinary changes edit.

## Evidence

The guard expression, identical at all four sites:

```
if ! grep -Eq '(^|[[:space:],])[1-9][0-9]* passed' "$summary"; then
```

Merged on `main` today:

- `justfile:309` — `test-live-tcg` (added by #2517)
- `.github/workflows/live.yml:571` — hosted TCG spine, `$tcg_summary` (added by #2048)

Arriving with #2540 (PR #2580, open at time of filing; it clones the same expression):

- `justfile:311` — `test-live`
- `.github/workflows/live.yml:810` — native job, `$native_summary`

No guard at all:

- `justfile:330-343` — `test-live-remote` has only an `rc -eq 5` "collected nothing" check. A run
that collects tests and skips every one exits 0 with nothing proved, which is the exact hole
#2517 closed for the sibling recipe.

Why the whole stream is in scope, not just the summary line:

- `pyproject.toml:119` sets `addopts = "-ra"`, so pytest always prints a short summary of skip
**reasons** into the same captured stream the guard greps.
- The recipes capture via `| tee "$summary"`, so `$summary` holds all output, not the final line.

Confirmed by construction, not inferred. `just test-live` was fed a genuine all-skip stream —
pytest rc 0, `4 skipped`, zero passed — whose `-ra` skip reason read:

```
SKIPPED [1] tests/live_vm/test_kdump.py:31: needs a prior warm run where 1 passed
```

The recipe **exited 0**. The string satisfied the gate from a skip reason, off the summary line
entirely.

## Expected

A guard that reads as proof-of-execution is satisfied only by pytest's own summary line. An
all-skip, all-fail, or empty run is red at every one of the five sites. The two sibling recipes
(`test-live`, `test-live-tcg`, `test-live-remote`) and the two workflow jobs agree on one
expression, so the next tier added inherits a correct one rather than a fifth copy of a broken one.

## Proposed approach

1. Anchor the match to pytest's summary line rather than searching the whole stream — the summary
is the last line and is delimited by `=` runs, so anchoring on that shape rejects a skip reason
while still accepting `4 passed`, `4 passed, 1 skipped`, and `1 failed, 4 passed`. Decide the
exact expression against real pytest output for each of those cases plus the all-skip case;
`-p no:cacheprovider` and terminal width both affect the line, so check them.
2. Apply it at all four existing sites.
3. Give `test-live-remote` the same guard, keeping its existing `rc -eq 5` message.
4. Put the expression in one place both the `justfile` and `live.yml` call, so site five is not
another copy. A small script under `scripts/` is the repository's existing shape for this;
hooks and CI already invoke recipes rather than duplicating command strings.
5. Verify by controlled fault, since this issue exists because a guard was trusted rather than
tested: feed each site an all-skip stream whose skip reason contains `1 passed` and observe red,
then a genuine passing stream and observe green. A guard added without that pair of arms is the
same defect again.

## Out of scope

- Changing which tests any tier selects, or any marker.
- The native tier's env contract (#2518, closed) and the allocation cap (#2560).
- `just test-agent-smoke`'s `rc -eq 5` tolerance, which #2540 addresses.

## Provenance

Found during campaign work on #2540: its implementer built the all-skip stream above while
verifying its own new guard, and the failure it exposed was in the two guards already merged rather
than in the new one. Independently re-verified against `pyproject.toml:119` and the shipped pattern
before filing. Filed at the repository operator's explicit request.

Contributor guide

Open the contributing guide

Research direction

Read the guards at justfile:309-343 and .github/workflows/live.yml:571 and :810, then check pyproject.toml:119 to understand the captured pytest output. Exercise the recipes and workflow guard with the documented all-skip stream, including a skip reason containing “1 passed,” plus genuine passing and failing summaries. Done means all five live sites reject unproved runs and share one verified expression.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, python, shell
Domain
ci-cd, devops, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.