Skip to content

CI tests via GitHub actions and conda #504

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.

Sign up for GitHub

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

Merged
murrayrm merged 10 commits into from
Jan 12, 2021
Merged

CI tests via GitHub actions and conda #504

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3f87c33
Create python-package-conda.yml
murrayrm Jan 10, 2021
21e47a6
remove flake8 lint; add dependencies
murrayrm Jan 10, 2021
d83e67d
skip X11 tests if no DISPLAY env variable
murrayrm Jan 10, 2021
627754f
expand test matrix to match Travis CI
murrayrm Jan 10, 2021
47251c6
simplify GitHub actions workflow
murrayrm Jan 11, 2021
6ff2e9b
pytest with slycot from source
murrayrm Jan 11, 2021
5bb9de4
update xvfb configuration; remove X11 marks
murrayrm Jan 12, 2021
36ecea1
update xvfb configuration; remove X11 marks
murrayrm Jan 12, 2021
b7b696f
add coveralls
murrayrm Jan 12, 2021
40aac67
Merge branch 'github-actions-conda_revised' into github-actions-conda
murrayrm Jan 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions .github/workflows/python-package-conda.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
name: Conda-based pytest

on: [push, pull_request]

jobs:
build-linux:
runs-on: ubuntu-latest

strategy:
max-parallel: 5
matrix:
python-version: [2.7, 3.6, 3.9]
slycot: ["", "conda"]
array-and-matrix: [0]
include:
- python-version: 3.9
slycot: conda
array-and-matrix: 1

steps:
- uses: actions/checkout@v2
- name: Set up Python
uses: actions/setup-python@v2

@bnavigator bnavigator Jan 10, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conflicts with the conda setup

@murrayrm murrayrm Jan 10, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

- name: Set up conda
run: |
echo $CONDA/bin >> $GITHUB_PATH
conda create -q -n test-environment python=${{matrix.python-version}}
Comment thread
murrayrm marked this conversation as resolved.
Outdated
- name: Install dependencies
run: |
source $CONDA/bin/activate test-environment
conda install pip coverage pytest
conda install numpy matplotlib scipy
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

@bnavigator bnavigator Jan 10, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why still bother with Python2? Time to ditch it for the next release.

Also, pytest-xvfb is the way to go.

@murrayrm murrayrm Jan 10, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

@bnavigator bnavigator Jan 10, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

- name: Install slycot via conda (optional)
if: ${{ matrix.slycot == 'conda' }}
run: |
source $CONDA/bin/activate test-environment
conda install -c conda-forge slycot
- name: Test with pytest
env:
PYTHON_CONTROL_ARRAY_AND_MATRIX: ${{ matrix.array-and-matrix }}
run: |
source $CONDA/bin/activate test-environment
if [[ '${{matrix.python-version}}' == '2.7' ]]; then
# Run within (virtual) X11 environment for Python 2.7/matplotlib
xvfb-run pytest control/tests
else
coverage run -m pytest control/tests
fi
Comment thread
murrayrm marked this conversation as resolved.
Outdated
3 changes: 2 additions & 1 deletion control/tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
"PendingDeprecationWarning")
matrixerrorfilter = pytest.mark.filterwarnings("error:.*matrix subclass:"
"PendingDeprecationWarning")

X11only = pytest.mark.skipif(os.getenv("DISPLAY") is None,
reason="requires X11")
Comment thread
murrayrm marked this conversation as resolved.
Outdated

@pytest.fixture(scope="session", autouse=True)
def control_defaults():
Expand Down
3 changes: 3 additions & 0 deletions control/tests/rlocus_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
from control.statesp import StateSpace
from control.bdalg import feedback

from control.tests.conftest import X11only


class TestRootLocus:
"""These are tests for the feedback function in rlocus.py."""
Expand Down Expand Up @@ -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

@bnavigator bnavigator Jan 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@murrayrm murrayrm Jan 10, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@murrayrm murrayrm Jan 11, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

def test_root_locus_zoom(self):
"""Check the zooming functionality of the Root locus plot"""
system = TransferFunction([1000], [1, 25, 100, 0])
Expand Down
2 changes: 2 additions & 0 deletions control/tests/sisotool_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from control.rlocus import _RLClickDispatcher
from control.xferfcn import TransferFunction

from control.tests.conftest import X11only

@pytest.mark.usefixtures("mplcleanup")
class TestSisotool:
Expand All @@ -19,6 +20,7 @@ def sys(self):
"""Return a generic SISO transfer function"""
return TransferFunction([1000], [1, 25, 100, 0])

@X11only
def test_sisotool(self, sys):
sisotool(sys, Hz=False)
fig = plt.gcf()
Expand Down