-
Notifications
You must be signed in to change notification settings - Fork 683
Move tests to GitHub Actions #1551
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
Move tests to GitHub Actions #1551
Conversation
The previous method treated any non-empty value as truth-y. This adds '0' and some Python-like values ('false', 'none') to the false-y values to allow scripts to be more explicit.
This moves our entire testing apparatus over to GitHub Actions, since Travis has changed its pricing model and we can no longer rely on it. This actually has several advantages for us right now: - better integration with other GitHub Actions workflows - more concurrent jobs - more access to Windows and macOS machines - easier job dependency graphs within our resources This first commit does not add in any Windows tests because they're currently known to fail, but the intention is to turn them on. The blocking issue is currently a failure of the multiprocessing capabilities, which also affects mcsolve. We can now reasonably run coverage reporting on every test run, and upload all of them to Coveralls at the end, so we get accurate stats on what lines were actually tested.
939d706
to
51dcddb
Compare
@hodgestar shall we attempt to sort out the caching now, or get things moved over ahead of Unitary Hack and sort it out later? I don't think it's particularly a blocking issue - the tests still all run fine, after all. There's an argument against using |
Definitely leave for later.
I don't think we need to pin to a particular version to benefit from the pip caching, but let's see later. |
This moves our entire testing apparatus over to GitHub Actions, since Travis has changed its pricing model and we can no longer rely on it. This actually has several advantages for us right now:
This first commit does not add in any Windows tests because they're currently known to fail, but the intention is to turn them on. The blocking issue is currently a failure of the multiprocessing capabilities, which also affects mcsolve.
We can now reasonably run coverage reporting on every test run, and upload all of them to Coveralls at the end, so we get accurate stats on what lines were actually tested. I've also turned pytest's timer, so it reports all tests that took longer than a second to run. This should really help identify where the real problems in our testing are - right now you should see that there's a
correlation
test that takes around 2 minutes to run, which is a very sizable chunk of the entire test suite. We can concentrate on rewriting the longest tests to make sure we're still testing all the behaviour, without excessively long runtimes.I've been fairly conservative with the number of tests I set in motion. On the free GitHub plan we can have 20 concurrent VMs running across the whole QuTiP organisation, which I think spreads across all repositories and all test runs. With no Windows tests currently active this is 6 test runs plus the documentation build. If we want more, we could consider spending some money ($4/user/month) to convert into a Teams account which gets us 60 concurrent VMs, but I don't see any need to do this right now.
All the test runners now start up almost instantly, which really solves a lot of the slowdown we were getting on Travis. The actual speeds of the Linux VMs seem to be similar to what we had on Travis - the walltimes are slightly longer because I now install all optional dependencies in most runners, so slightly more gets tested than before. Currently there's also a slowdown on several machines because
cvxpy
doesn't build many wheels, and none at all for Linux, onpip
. I install all our dependencies except BLAS/LAPACK stuff that way, since it's easiest to read out the dependencies like that. We can potentially save some build time (maybe ~3 minutes?) if we install that from conda-forge instead.I also fixed a minor point of how environment variables are read within
setup.py
(in the end I didn't actually use the new behaviour in the action, but I did during testing), and added a warning filter on dnorm tests to account for a deprecated Numpy alias being used withincvxpy
code.Also of note: this should allow us to have commits that skip the testing stages (though possibly not codeclimate). You should be able to put
[skip ci]
in the commit message to avoid it.