ci: make validate.sh run the consumer suite - #35
codeforester merged 2 commits into
Conversation
| exit 1 | ||
| } | ||
|
|
||
| python - <<'PY' |
There was a problem hiding this comment.
Bug: the new distribution-version gate never actually blocks the script.
tests/validate.sh has no set -e, and nothing checks the exit status of the python - <<'PY' ... PY block. If a required distribution (base-cli-demo, base-cli, or pytest) is missing, the heredoc raises SystemExit and prints the intended diagnostic to stderr, but the script falls straight through to printf 'Running the complete consumer and documentation-command suite.\n' and exec python -m pytest -q anyway — i.e. the 'authoritative' gate is advisory only.
I verified this pattern in isolation:
before
boom
after (should NOT print if set -e worked, but WILL print without set -e)
exit code: 0
Suggest adding set -euo pipefail near the top of the script (or an explicit || exit 1 after the heredoc) so a missing distribution actually stops the run with the clear message, instead of silently falling through to pytest.
| PY | ||
|
|
||
| printf 'Running the complete consumer and documentation-command suite.\n' | ||
| exec python -m pytest -q |
There was a problem hiding this comment.
Cross-file impact: folding pytest into validate.sh makes two other workflows run the full suite twice.
This PR only updates .github/workflows/tests.yml, but ./tests/validate.sh is also invoked (unchanged) as a preliminary 'baseline' step in:
.github/workflows/compatibility.yml(supportedjob): 'Validate the repository baseline' (./tests/validate.sh) is immediately followed by 'Run installed-wheel compatibility tests' (python -m pytest -q)..github/workflows/release-package.yml(verifyjob): 'Validate repository baseline' (./tests/validate.sh) runs before./tests/package.shand the wheel build, and is followed later by 'Run the installed package tests' (python -m pytest -q) against the built wheel.
With this exec python -m pytest -q now inside validate.sh, both jobs run the entire pytest suite twice per invocation (4x across the compatibility matrix). In release-package.yml it's worse than just wasted time: the full suite now also runs prematurely against the source/editable install, before the release wheel is even built, where the step used to be a near-instant file-existence check.
Worth updating those two workflows (drop their now-redundant explicit pytest step) as part of this change, or keeping validate.sh's pytest invocation opt-in so existing call sites aren't silently doubled.
| - name: Run consumer tests | ||
| run: python -m pytest | ||
| - name: Run authoritative consumer validation | ||
| run: ./tests/validate.sh |
There was a problem hiding this comment.
Minor: lost fail-fast behavior for the baseline check.
Previously 'Validate repository baseline' ran as the very first step, before 'Set up Python' — a missing required file (e.g. LICENSE, CHANGELOG.md) failed within seconds. Now ./tests/validate.sh only runs after Python setup and pip install ".[dev]" complete, and (per the change in tests/validate.sh) it now also runs the entire pytest suite at the end. A trivial baseline failure that used to surface almost instantly now waits through the full environment setup first.
Fixes #22