Repository navigation
tests-ok passes when workspace member discovery fails #18
Description
What's broken
tests-ok can pass when workspace member discovery fails and the dependent pytest matrix is skipped. This leaves the required pytest umbrella check green even though discovery failed and no matrix tests ran.
The workflow as a whole can still fail because members failed. The defect is the incorrect result of the required umbrella check; this report does not claim that every other merge requirement is bypassed.
Which layer
CI / dev loop: .github/workflows/tests.yml at upstream deb35662.
pytest needs members. tests-ok only directly needs pytest, uses if: always(), and accepts both success and skipped without checking whether discovery succeeded.
How to reproduce
In a checkout of upstream deb35662f299f4c3e931464eb2e94eebe5906f57, run this standard-library-only probe with Python 3.11+ and Bash:
python3 - <<'PY'
import os
import re
import subprocess
from pathlib import Path
from textwrap import dedent
workflow = Path('.github/workflows/tests.yml').read_text(encoding='utf-8')
def script(job):
body = re.search(r'(?ms)^ ' + job + r':\n(.*?)(?=^ \S|\Z)', workflow)[1]
return dedent(re.search(r'(?m)^ run: \|\n((?: .*\n|\n)+)', body)[1])
def run(text, **env):
result = subprocess.run(
['bash', '--noprofile', '--norc', '-e', '-o', 'pipefail', '-c', text],
env=dict(os.environ, **env), check=False,
)
print('exit:', result.returncode, flush=True)
return result.returncode
fault = 'python3() { if [ "$1" = "tools/dev/ci-pytest-batches.py" ]; then return 1; else command python3 "$@"; fi; }\n'
assert run(fault + script('members'), EVENT='push', GITHUB_OUTPUT=os.devnull) == 1
gate = script('tests-ok').replace('${{ needs.members.result }}', 'failure')
gate = gate.replace('${{ needs.pytest.result }}', 'skipped')
assert run(gate) == 0 # incorrect success on the original workflow
PYObserved output on the original workflow:
exit: 1
pytest gate satisfied (matrix result: skipped)
exit: 0
This executes the actual discovery and gate shell blocks locally. The pytest=skipped result is supplied to model GitHub's documented dependency semantics; this is not a hosted Actions fault-injection run.
Expected vs actual
Expected: successful discovery is required before tests-ok may pass. Discovery failure, cancelled, or skipped must fail the umbrella check. Successful discovery followed by a successful matrix, or a legitimately skipped empty matrix, must still pass.
Actual: the original gate returns 0 for members=failure, pytest=skipped.
The current regression module has 50 cases, covering all 16 job-result combinations, canonical empty-selection checks, Bash availability, exact failure diagnostics, equivalent literal dependency declarations, and the BATCHES output binding. Against the first-fix workflow d0e14d18, it reports 8 failed / 42 passed; against the revised workflow with Git Bash it reports 50 passed.
Surface area (optional)
.asf.yaml configures tests-ok as the required pytest-related context; the prek CI workflow skips workspace-pytest. The fix needs only the umbrella job and its regression tests, preserving the existing matrix and empty-selection behavior.
Duplicate searches on 2026-10-24 covered upstream and this fork, including open and closed issues/PRs, using tests-ok, members failure, and members gate. No same-root-cause report or fix was found. Related upstream PRs #341, #753, and #1717 introduce the umbrella, preserve empty-matrix skipping, and batch the matrix; they do not check discovery failure in the umbrella.
Environment (optional)
Verification distinguishes the shell environments explicitly:
- PowerShell with Git Bash prepended to PATH: 50 gate-module tests pass.
- Default PowerShell, where Bash resolves to the unusable WSL launcher: 23 tests pass and 27 Bash-dependent cases explicitly skip. The original module reproduced 2 failed / 15 passed there.
- Linux with Bash/Python 3.12: all 338 dev tests pass. Dev ruff, formatting, mypy, offline zizmor, full-tree prek (including lychee and workspace tests), and the real commit hook pass.
The earlier 17-pass Windows result used PowerShell with Git Bash explicitly prepended to PATH. It did not validate the default WSL launcher.
Review follow-up — 2026-10-25
The existing Draft PR #19 now includes follow-up commit 2c0beae49185760074282080f57ee77b0a513e1d. The confirmed rule is: successful discovery is required, and a skipped matrix is accepted only when its output is exactly []. BATCHES is supplied through the step environment; no JSON parser or new package dependency is added.
| Item | Disposition | Verification and resulting change |
|---|---|---|
| 1 | Partially accepted | The first fix accepts members=success, pytest=skipped with nonempty or missing batches. Require canonical [] for a skipped matrix. The proposed cancellation timing remains unverified; GitHub's cancellation reference does not establish that exact outcome. |
| 2 | Accepted | Default PowerShell reproduced 2 failed / 15 passed with the original module because bash resolved to the unusable WSL launcher. The fixture now probes the real arguments, with a five-second timeout, and skips missing, failed, or timed-out launchers. |
| 3 | Accepted | All 14 negative cases in that original WSL run passed without executing the shell. The revised cases assert exact status and the diagnostic for discovery, invalid selection, matrix failure, or success. |
| 4 | Accepted | Inline comments, quoted lists, block lists, and expression-wrapped always() were falsely rejected. A bounded standard-library field reader now accepts those forms; its docstring states the unsupported YAML features. Incorrect dependencies, conditions, and literal/misdirected BATCHES bindings still fail. |
| 5 | Covered by item 1 | Checking the actual output makes the gate independent of why pytest was skipped. No additional assertion freezes the pytest job's condition text. |
| 6 | Accepted | .asf.yaml now describes successful discovery plus a passing matrix or confirmed empty selection. Only the explanatory comment changes. |
| 7 | Accepted | Actual selector calls include .sh and .json changes and exclude .md / plugin manifests. The three old Python-only comments now defer to eligible files, linked sources, dependents, and shared-input selection. |
As of 2026-10-25 00:04:28 MYT (UTC+8), GitHub reports no workflow runs, commit statuses, or PR checks for the revised head. Hosted CI and the proposed cancellation timing remain unverified.
AI-generated with Codex (GPT-6). Filed in the contributor's fork for human audit; human review is pending.
Activity