-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add github actions job to run unit tests for 3.8, 3.9 #188
Conversation
78c8470
to
bbacaed
Compare
bbacaed
to
859cfb9
Compare
This already exists in requirements.txt
Would like to merge this - in another branch I'm planning to add a CI test to validate the readthedocs build and I'd like to add it in github actions form |
No need to create a virtualenv, these tests always run in a fresh environment anyway.
This workflow will hold more than just unit tests in the future
We want to abort the run on any unit test failure to save compute time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, only question I have is the on: push trigger, does pull request make more sense as a trigger? Asking from ignorance:
https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request
You're right I think the behavior should be slightly different. The pull-request event isn't quite what we want because it wouldn't run on commits to an open pull request which we seem to rely on a lot. If we want to match the behavior of our circleci config we want it run on push events still but restrict it only to branches with open pull requests. I can make that change |
Never mind the pull_request event is exactly what we want - if you push a commit which changes the HEAD ref of a pull request, it counts as a pull_request event with the synchronize action. Perfect! |
This includes commits to branches with open PRs
Currently I think this runs against any commit even if no PR is open on the branch. Can change that if we want.