Repository navigation
tests-ok passes when workspace member discovery fails #1806
Description
What's broken
tests-ok succeeds when workspace member discovery fails and the dependent pytest matrix is skipped. The required pytest umbrella check can therefore be green while discovery has failed and no matrix ran.
The workflow can still fail overall because members failed. The incorrect result is in the required umbrella check; other merge requirements may still block the pull request.
Which layer
CI: .github/workflows/tests.yml at dcfe0ad5.
pytest needs members, but tests-ok directly needs only pytest. Its unconditional assertion accepts skipped without checking discovery.
How to reproduce
Check out dcfe0ad562b55d4df41bc69aee1f0f8289ee205b, then run this Python 3.11+ / Bash probe from the repository root. It uses only the Python standard library and executes the actual workflow shell blocks.
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), capture_output=True, text=True, check=False,
)
print('exit:', result.returncode, result.stdout.strip(), 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.pytest.result }}', 'skipped')
assert run(gate) == 0 # the incorrect success on the baseline
PYObserved:
exit: 1
exit: 0 pytest gate satisfied (matrix result: skipped)
This is local shell fault injection. The supplied pytest=skipped result models GitHub's failed-dependency semantics.
Expected vs actual
Expected: discovery must succeed before the umbrella may pass. A successful matrix passes; a skipped matrix passes only when discovery explicitly produced canonical []. Discovery failures and matrix failures/cancellations fail the gate.
Actual: the baseline returns 0 for failed discovery followed by skipped pytest.
Surface area (optional)
.asf.yaml requires tests-ok as the pytest context, and the prek workflow skips workspace-pytest. The repair needs the workflow, regression tests, and the explanatory .asf.yaml comment; the required check name and branch-protection values remain the same.
Related #753 establishes legitimate empty-selection skipping. #1783 adds a generic skipped-gate audit rule; it does not repair this workflow. Searches of open/closed issues and PRs found no report or fix for this specific discovery-result omission.
Environment (optional)
Verified on upstream dcfe0ad5 in Linux with Python 3.12 and Bash. The final 47-case regression module reports 24 failed / 23 passed against the baseline and 47 passed against the fix; the complete dev suite reports 338 passed. Ruff, formatting, mypy, offline zizmor, whole-tree non-test hooks (including lychee), the affected workspace pytest hook, the real commit hook, and Magpie's contribution checklist pass.
Proposed fix: PR #1807, now ready for review. Upstream matrix CI and prek pass on d83db100, including the Ready-event reruns. Discovery selects 52 members, all eight pytest steps succeed, and the hosted dev run records 338 passing tests.
Suggested classification: bug, family:ci, capability:fix; label assignment requires maintainer permissions.
Prior public audit: fork issue #18, fork Draft PR #19.
AI-generated with Codex (GPT-6); submitted for maintainer review.
Activity