Skip to content
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

Enable testing with py311 #51

Merged
merged 3 commits into from
Jul 16, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 23 additions & 19 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ on:

jobs:
test:
runs-on: ${{ matrix.os }}
runs-on: ${{ matrix.os || 'ubuntu-latest' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment explaining why this is necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

In theory it should not be needed to have a fallback value as we do already have all values defined for "os" dimension, but to my surprise, without it GHA failed to load the workflow complaining about the empty value inside run-on. Adding a default, sorts this issue. it is not really used by any job on the matrix, but if you forget to add os on one "include", it will make use of it.

strategy:
fail-fast: false
matrix:
python: ["3.7", "3.8", "3.9", "3.10", "pypy-3.8"]
os: [ubuntu-latest, windows-latest]
python: ["3.7", "3.8", "3.9", "3.10","~3.11.0-0", "pypy-3.8"]
ssbarnea marked this conversation as resolved.
Show resolved Hide resolved
os:
- ubuntu-latest
- windows-latest
Copy link
Contributor

Choose a reason for hiding this comment

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

I think reverting this change is better than having to add:

runs-on: ${{ matrix.os || 'ubuntu-latest' }}

Copy link
Member Author

Choose a reason for hiding this comment

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

That is the second place where I raised my brows while testing the pipeline, GHA complained about os at this line, and making it an explicit list fixed the issue. It is bit perplexing because I know well enough that part of the YAML specification and both representations are valid and should load the same. Still, one failed another one did not.

Probably you know the Github is not using a standard YAML loading library and they have their own implementation, one that does not even supports anchors. I would not be surprised to be another issue related to that.

PS. || works even for undefined values, if the value on the left side is not defined, it will pick the second value.

You are free to try other variants including modifying this pr.

include:
- python: "3.7"
tox_env: "py37"
Expand All @@ -26,22 +28,24 @@ jobs:
tox_env: "py39"
- python: "3.10"
tox_env: "py310"
- python: "~3.11.0-0" # see https://github.com/actions/setup-python/issues/213#issuecomment-1146676713
tox_env: py311
- python: "pypy-3.8"
tox_env: "pypy3"
steps:
- uses: actions/checkout@v3
- name: Set up Python
uses: actions/setup-python@v3
with:
python-version: ${{ matrix.python }}
- name: Install tox
run: |
python -m pip install --upgrade pip
pip install tox
- name: Cache tox environments
uses: actions/cache@v3
with:
path: .tox
key: tox-${{ matrix.os }}-${{ matrix.python }}-${{ hashFiles('pyproject.toml') }}
- name: Test
run: tox -e ${{ matrix.tox_env }}
- uses: actions/checkout@v3
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python }}
- name: Install tox
run: |
python -m pip install --upgrade pip
pip install tox
- name: Cache tox environments
uses: actions/cache@v3
with:
path: .tox
key: tox-${{ matrix.os }}-${{ matrix.python }}-${{ hashFiles('pyproject.toml') }}
- name: Test
run: tox -e ${{ matrix.tox_env }}