-
Notifications
You must be signed in to change notification settings - Fork 14
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
automated testing #68
Comments
Thanks for preparing a continuous integration script. My main concern is that running the full test suite currently requires at least 15 minutes on a high-end system (excluding setting up the environment). GitHub will likely take longer. Currently, the test suite is run as part of version releases, see #64 for example. However, I can see the benefit of additionally running CI on (a subset of) the full test suite. This will also help identify issues when dependencies are making changes to their API. I have opened a new branch for the PR. |
List of test durations:
|
I think it's fine to not test everything, but there should be some critical group of tests that get run on auto- otherwise it's too easy, especially if someone is running away with a fork of their own, to just start slowly breaking everything without realizing it. |
I took the liberty of writing a CI workflow based on a standard GitHub workflow: .github/workflows/auto-test-package.yml. This also checks against different OS and python versions (currently limited to 3.10 and 3.11 due to limitations in the A Pull Request is no longer required. |
Great but if I recall Python 3.11 should throw you some problems, and Python 3.12 definitely did when I tested it. Look very carefully at each package, and the requirements before adding Python 3.11. |
Python 3.12 likely throws errors because its not supported by The auto-ci test suite also includes a test that uses parallel processing. |
To do:
|
I suggest you will be better of with testing in your CI. I wrote an example for your package (https://github.com/drcandacemakedamoore/reviewmirp/blob/master/.github/workflows/on-commit.yml) if you like it we can make a PR from this fork; if you envision a different approach go for it; but I see no more normative and efficient way to handle testing that putting it in the CI.
The text was updated successfully, but these errors were encountered: