Run the test suite on pull requests - #41
Merged
Merged
Conversation
The Tests workflow only triggered on pushes to main and a weekly cron, so no pull request ever had its tests run. A PR showed green from the pre-commit workflow alone, which checks formatting and linting and nothing else, and the suite first ran against the merged result once it was already on main. The pull_request trigger mirrors the paths-ignore list from the push trigger so docs, paper, and notebook only changes still skip the run. The existing concurrency group keys on github.ref, which is refs/pull/N/merge for a pull request, so successive pushes to one PR cancel the earlier run without affecting other PRs. This costs five conda based jobs per PR push, covering Python 3.10 to 3.14, which is the point: a broken PR should say so before it lands rather than after.
Replaces the hardcoded -n 2 with -n auto so the worker count comes from the machine rather than a number that has to be revisited when runner specs change. This is not a speedup. Measured across the full matrix it changes nothing: auto resolves to 2 workers here, the same as before, because psutil is in the CI environment and pytest-xdist's auto counts physical cores, of which the runner has two. Mean test step went 376s to 387s, which is noise. Only -n logical would reach the four logical CPUs, and whether hyperthread siblings help this mostly CPU bound suite is unmeasured, so that is left alone.
fbunt
force-pushed
the
ci-run-tests-on-prs
branch
from
July 29, 2026 01:10
4451e6b to
c7ed493
Compare
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.
Summary
Adds a
pull_requesttrigger to the Tests workflow.It previously ran only on pushes to
mainand a weekly cron, so no pull requestever had its tests run. The only check reporting on a PR was the pre-commit
workflow, which covers formatting and linting and nothing else, so a PR could show
green with a failing suite. The tests first ran against the merged result, once it
was already on
main.Details
The new trigger mirrors the
paths-ignorelist from thepushtrigger, so PRs thatonly touch
docs/,paper/, ornotebooks/still skip the run.No concurrency change was needed. The existing group keys on
github.ref, which isrefs/pull/N/mergefor a pull request, so it is already unique per PR --cancel-in-progresssupersedes earlier runs when a PR is updated without affectingother PRs.
Cost
Five conda-based jobs per PR push, covering Python 3.10 through 3.14. That is the
tradeoff being made deliberately: a PR that breaks the suite should say so before it
merges rather than after. If the cost turns out to be annoying in practice, the
matrix could be trimmed for PRs and left full for
mainand the cron, but startingwith full parity seems better than guessing which versions matter.
Verifying it works
This PR does not touch the ignored paths, so opening it should itself trigger the
workflow. If the Tests checks appear here, the change works.