CI tests via GitHub actions and conda #504
Conversation
|
I am not done with Slycot yet (coming closer), but see https://github.com/bnavigator/Slycot/blob/3c0cb727d5371996c038c94e9bd68621a5430b8a/.github/workflows/slycot-build-and-test.yml for an example how to deal with coveralls. |
control/tests/rlocus_test.py
Outdated
| @@ -48,6 +50,7 @@ def test_without_gains(self, sys): | |||
| roots, kvect = root_locus(sys, plot=False) | |||
| self.check_cl_poles(sys, roots, kvect) | |||
|
|
|||
| @X11only | |||
There was a problem hiding this comment.
The failures are not due to X11 but because the figure manager has no toolbar. I guess the matplotlib version/setup is different than on the Travis runners.
Same problem has always been present for the openSUSE rpm package, which I maintain. Forcing the PyQt5 backend instead of Agg helped there, but my tries (short) for Slycot on GitHub worklfows did not succeed. I just skip the two tests in python-control/Slycot#140
There was a problem hiding this comment.
Thanks for the extra info. I'd prefer not to have to factor these out. Let me see if I can use pytest-xvfb or something similar to get it to work.
There was a problem hiding this comment.
I ended up leaving in these marks since they are useful if you are testing without X11, but also got pytest-xvfb working, so these tests aren't skipped in the GitHub Actions CI testing.
| - uses: actions/checkout@v2 | ||
| - name: Set up Python | ||
| uses: actions/setup-python@v2 |
There was a problem hiding this comment.
Conflicts with the conda setup
There was a problem hiding this comment.
I think I might be able to remove this completely. I meant to do that at one point but lost track of it while playing around to just get something to work. I'll see if I can remove it and if things still work correctly.
| if [[ '${{matrix.python-version}}' == '2.7' ]]; then | ||
| # matplotlib for Python 2.7 seems to require X11 to run correctly | ||
| sudo apt install -y xvfb | ||
| fi |
There was a problem hiding this comment.
Why still bother with Python2? Time to ditch it for the next release.
Also, pytest-xvfb is the way to go.
There was a problem hiding this comment.
Thanks for the suggestion. Will try shifting to pytest-xvfb. I figure we may as well test against Python 2.7 for as long as it continues to work. Amazingly, it is still the default Python on MacOS 11 (Big Sur).
There was a problem hiding this comment.
If we don't actively remove it, every PR with a failing Py2 test will try to workaround it. I think we should save contributors from this torture.
| conda install pip coverage pytest | ||
| conda install -c conda-forge pytest-xvfb pytest-cov |
There was a problem hiding this comment.
coverage not reported to coveralls yet
There was a problem hiding this comment.
I think we need to merge the PR before this is run and (hopefully) at that point things start to show up in coveralls?
There was a problem hiding this comment.
coverage != coveralls.
See https://coveralls-python.readthedocs.io/en/latest/usage/configuration.html#github-actions-gotcha and my previously linked workflow draft for Slycot. (Much more complicated than yours! :( )
| - name: Test with pytest | ||
| env: | ||
| PYTHON_CONTROL_ARRAY_AND_MATRIX: ${{ matrix.array-and-matrix }} | ||
| run: | | ||
| source $CONDA/bin/activate test-environment | ||
| # Use xvfb-run instead of pytest-xvfb to get proper mpl backend | ||
| # Use coverage instead of pytest-cov to get .coverage file |
There was a problem hiding this comment.
pytest-cov produces one. See python-control/Slycot#140. Use the --cov parameter.
| @@ -31,18 +32,36 @@ jobs: | |||
|
|
|||
| # Install test tools | |||
| conda install pip coverage pytest | |||
| conda install -c conda-forge pytest-cov | |||
| pip install coveralls | |||
There was a problem hiding this comment.
coveralls is on conda-forge nowadays
|
I think this PR is good to go. There are some small changes that @bnavigator suggests, but I don't these these substantially add/enhance functionality => we can probably stick with this for now (and see if it all works when merged). |
|
Sorry, for being so pedantic here. I went through many of these hoops for Slycot and the openSUSE rpm packages so I want to share the "proper" solutions.
|
|
I'll move these comments to issue #446 so that we can continue to track these improvements. I mainly wanted to get shifted over to some new test infrastructure since Travis CI is getting so frustrating. (GitHub actions clears commits in 2-3 minutes, versus what was often 30+ min for Travis). I think we can leave Travis and GitHub Actions running for a few days, as we see whether new PRs and things are properly handled, but then I think we shift away from Travis completely. |
This is a fairly minimal version of continuous integration (CI) tests using GitHub actions consisting of two workflows:
python-package-conda: This workflow uses a conda-based install for everything and tests only against the latest scipy and Python 3.6 and 3.9. So compared with our current Travis CI testing, we lose scipy-0.19.1 testing and Python 2.7. Coverage is implemented, but not sure (yet) whether it properly connects to coveralls.control-slycot-src: This workflow installs slycot from source, which allows testing against the latest version of slycot (in case we need it). No coverage testing.This workflow users coveralls and pushes results up to coveralls.io.
Note: I think that as soon as this PR is merged then we should see the workflows show up in the Actions tab on GitHub. This is working correctly in my personal fork.