hypermodern-python icon indicating copy to clipboard operation
hypermodern-python copied to clipboard

Redundant matrix strategy in `.github/workflows/tests.yml`

Open gustavgransbo opened this issue 4 years ago • 3 comments

Thanks for a great blog-series and repo!

I noticed that the matrix strategy python-version: ['3.7', '3.8'] in .github/workflows/tests.yml is redundant, since nox will take care of testing both versions anyway.

gustavgransbo avatar Jul 05 '20 18:07 gustavgransbo

Thank you for the feedback! Yes, I have observed this as well.

I would avoid relying on additional Python versions being available in the runner image. The ubuntu-latest image is a moving target, and may not have the same Python versions installed in the future. Even if you were to pin the image to a specific release, it is better to be explicit about the Python versions your jobs should run on, and use only the version specified with the setup-python action.

Since Nox 2020.5.24, there is a --python option which lets you restrict runs to a specific Python version. So in theory, you could do this:

- run: nox --python=${{ matrix.python-version }}

Unfortunately, this will run all sessions that have the specified Python version, even when they are not listed in nox.options.sessions. So it makes sense to include the Nox session in the matrix as well. You can see an example of this in the Tests workflow of the Hypermodern Python Cookiecutter.

cjolowicz avatar Jul 08 '20 11:07 cjolowicz

Thank you for the elaborate answer!

Including the session in the matrix is a great idea, which I will most likely opt for in my personal project. Thanks!

gustavgransbo avatar Jul 08 '20 12:07 gustavgransbo

Unfortunately, this will run all sessions that have the specified Python version, even when they are not listed in nox.options.sessions

I've just checked with Nox 2022.11.21 (on Windows); nox -p 3.8 runs only sessions listed in nox.options.sessions which have 3.8 specified

I think it can also help with missing interpreters on CI. Nox 2022.11.21 gives error on CI in that case (it was a warning earlier). See https://github.com/wntrblm/nox/pull/567 ~So I've had to add nox --no-error-on-missing-interpreters flag~ Update: --python flag works fine

Winand avatar Feb 05 '23 11:02 Winand