ci: extract the shell-suite runner and guard that CI keeps calling it - #127
Merged
Conversation
Issue #32 (CI collecting only pytest, so tests/*.sh went unexecuted) was already fixed on master by inlining a loop into the shell job of .github/workflows/test.yml. Two gaps remained: the loop was reachable only by pushing (no local equivalent), and nothing asserted CI kept calling it -- deleting the step would silently restore the original silence. Extracts the loop to scripts/run-shell-suites.sh, so CI and a local run execute identical code, and adds tests/test_ci_runs_shell_suites.py (pytest, so the guard itself is guaranteed to run) asserting the workflow still invokes it and still runs pytest, that the runner covers every suite and accumulates rather than short-circuits on failure, and that a deliberately failing suite makes it exit non-zero. Verified the guard actually fails, not just that it exists: temporarily replaced the workflow's `bash scripts/run-shell-suites.sh` step with a no-op and confirmed test_ci_invokes_the_shell_runner goes red, then restored it. One correction to the ported original: master gained tests/run-shell-suite.sh (a meta-runner that loops over every test_*.sh and prints one aggregate summary for regression-test.sh's parser) after this PR was opened. The runner's original tests/*.sh glob would have picked that file up as if it were a leaf suite, running everything a second time nested inside it every CI run. Narrowed the glob to tests/test_*.sh, which the meta-runner's name doesn't match, and added a test pinning that exclusion so a future rename can't quietly reintroduce the nesting. Reapplies the one real commit from PR #119, which carried this fix 26 commits behind master (CONFLICTING) with unrelated already-landed history.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #119, which carried this fix 26 commits behind master (CONFLICTING) with unrelated already-landed history. Reapplies the one real commit cleanly.
Scope
Not a fix for #32 -- that's already fixed on master (inline loop in the
shelljob). This closes what the inline fix left open: the loop was reachable only by pushing, and nothing asserted CI kept calling it.Changes
scripts/run-shell-suites.sh-- the inline loop, extracted, so CI and a local run execute the same code..github/workflows/test.yml-- the shell job's inline script becomesrun: bash scripts/run-shell-suites.sh.tests/test_ci_runs_shell_suites.py-- 8 pytest tests guarding the workflow and the runner.Verified the guard actually fails, not just exists
Temporarily replaced the workflow step with a no-op and confirmed
test_ci_invokes_the_shell_runnergoes red, then restored it.One correction to the ported original
Master gained
tests/run-shell-suite.sh(a meta-runner that loops over everytest_*.shand prints one aggregate summary forregression-test.sh's parser) after PR #119 was opened. The originaltests/*.shglob would pick that file up as a leaf suite, running everything a second time nested inside it on every CI run. Narrowed the glob totests/test_*.sh, which excludes it structurally, and addedtest_runner_excludes_the_meta_runnerpinning the intent.Verification
The one failure is pre-existing on master, unrelated to this change (tracked as #125).