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

Add Python 3.11 and PyPy 3.9 to the testing and drop 3.6 #567

Merged
merged 4 commits into from
Feb 28, 2023

Conversation

@cclauss
Copy link
Contributor Author

cclauss commented Dec 3, 2022

@RonnyPfannschmidt @ssbarnea Can you please approve the workflow run so we can see if it passes?

@ssbarnea
Copy link
Member

ssbarnea commented Dec 3, 2022

Is not a problem of approval, there is no option. Changes made to a workflow must come from another branch in the repository in order to be tested correctly by GHA. When they come from a pull-request, they are not applied as someone would expect.

@hugovk
Copy link
Member

hugovk commented Dec 5, 2022

@cclauss The CI is failing.

Copy link

@Mason-Lin Mason-Lin left a comment

Choose a reason for hiding this comment

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

Fixes some typos.

tox.ini Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
@hugovk
Copy link
Member

hugovk commented Feb 19, 2023

https://github.com/pytest-dev/pytest-cov/actions/runs/4215078717/jobs/7315964982 is erroring because GitHub Actions is dropping support for EOL Python 3.6.

pytest itself dropped 3.6 in 2021: pytest-dev/pytest#9437

@Mason-Lin
Copy link

Need to update pytest-xdist >=3.1.0
#842: Python 3.11 is now officially supported.

https://pytest-xdist.readthedocs.io/en/latest/changelog.html#pytest-xdist-3-1-0-2022-12-01

@cclauss cclauss force-pushed the patch-1 branch 2 times, most recently from e34d9e7 to 4f71782 Compare February 19, 2023 11:29
@Pierre-Sassoulas
Copy link
Member

@Mason-Lin There's no need to update pytest-xdist, pip will take the version compatible with the user interpreter (provided metadata are correct in pytest-xdist packaging). We might make the resolution impossible for lower interpreter version if we do that (i.e. no compatible pytest-xdist for python 3.7 or a false incompatibility with another dependencie that need an old pytest-xdist)

@cclauss
Copy link
Contributor Author

cclauss commented Feb 19, 2023

@hugovk @Mason-Lin @Pierre-Sassoulas Your reviews, please.

@Mason-Lin
Copy link

@Mason-Lin There's no need to update pytest-xdist, pip will take the version compatible with the user interpreter (provided metadata are correct in pytest-xdist packaging). We might make the resolution impossible for lower interpreter version if we do that (i.e. no compatible pytest-xdist for python 3.7 or a false incompatibility with another dependencie that need an old pytest-xdist)

I originally saw pytest-xdist==3.0.2 in tox.ini. After checking the changelog, it seems that only 3.2.0 supports python 3.11.
I thought it was just a typo, I am not very familiar with this project.😅
The following commit done much more, such as skip boxed option.
Sorry for giving a wrong direction with a positive tone, I was really just guessing, thanks for the detailed explanation.

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

LGTM but it also remove pypy3.7 and python 3.9, I'm not familiar with the maintenance guideline of pytest-cov so I can't approve.

hooks:
- id: pyupgrade
args: [--py36-plus]
args: [--py37-plus]
Copy link
Member

Choose a reason for hiding this comment

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

If dropping 3.6, there's also the classifier and python_requires in setup.py. Plus mention in the PR title.

Choose a reason for hiding this comment

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

I think also need to update few parts related to Python 3.6 if this PR is going to drop 3.6.

https://github.com/pytest-dev/pytest-cov/blob/master/setup.py#L114

Copy link
Contributor Author

@cclauss cclauss Feb 27, 2023

Choose a reason for hiding this comment

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

Done below.

Copy link
Member

Choose a reason for hiding this comment

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

Let's also mention the 3.6 drop in the PR title for visibility/transparency.

@cclauss
Copy link
Contributor Author

cclauss commented Feb 19, 2023

The pypy team dropped pypy3.7 https://www.pypy.org/download.html

@@ -1916,6 +1918,7 @@ def find_labels(text, pattern):


@pytest.mark.skipif("coverage.version_info < (5, 0)")
@pytest.mark.skipif("coverage.version_info > (6, 4)")
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for skipping coverage > 6.4?

Copy link
Contributor Author

@cclauss cclauss Feb 19, 2023

Choose a reason for hiding this comment

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

toxpython: 'python3.11'
python_arch: 'x64'
tox_env: 'py311-pytest72-xdist320-coverage65'
os: 'macos-latest'
- name: 'pypy37-pytest71-xdist250-coverage64 (ubuntu)'
Copy link
Member

Choose a reason for hiding this comment

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

Re: PyPy dropping PyPy3.7 (https://www.pypy.org/posts/2022/12/pypy-v7311-release.html), should we remove these three pypy-3.7 jobs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am willing to go either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

tox.ini Outdated
check-manifest
colorama
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to add colorama ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

For future reference, when the GHA log is deleted:

check: commands[3]> isort --check-only --diff src tests setup.py
Traceback (most recent call last):
  File "/home/runner/work/pytest-cov/pytest-cov/.tox/check/lib/python3.11/site-packages/isort/main.py", line 87, in sort_imports
    incorrectly_sorted = not api.check_file(file_name, config=config, **kwargs)
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/pytest-cov/pytest-cov/.tox/check/lib/python3.11/site-packages/isort/api.py", line [33](https://github.com/pytest-dev/pytest-cov/actions/runs/4284044499/jobs/7460448953#step:5:34)8, in check_file
    return check_stream(
           ^^^^^^^^^^^^^
  File "/home/runner/work/pytest-cov/pytest-cov/.tox/check/lib/python3.11/site-packages/isort/api.py", line 272, in check_stream
    printer = create_terminal_printer(
              ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/pytest-cov/pytest-cov/.tox/check/lib/python3.11/site-packages/isort/format.py", line 153, in create_terminal_printer
    colorama.init(strip=False)
    ^^^^^^^^
NameError: name 'colorama' is not defined

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/runner/work/pytest-cov/pytest-cov/.tox/check/bin/isort", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/runner/work/pytest-cov/pytest-cov/.tox/check/lib/python3.11/site-packages/isort/main.py", line 1225, in main
    for sort_attempt in attempt_iterator:
  File "/home/runner/work/pytest-cov/pytest-cov/.tox/check/lib/python3.11/site-packages/isort/main.py", line 1209, in <genexpr>
    sort_imports(  # type: ignore
  File "/home/runner/work/pytest-cov/pytest-cov/.tox/check/lib/python3.11/site-packages/isort/main.py", line 114, in sort_imports
    _print_hard_fail(config, offending_file=file_name)
  File "/home/runner/work/pytest-cov/pytest-cov/.tox/check/lib/python3.11/site-packages/isort/main.py", line 127, in _print_hard_fail
    printer = create_terminal_printer(
              ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/pytest-cov/pytest-cov/.tox/check/lib/python3.11/site-packages/isort/format.py", line 1[53](https://github.com/pytest-dev/pytest-cov/actions/runs/4284044499/jobs/7460448953#step:5:54), in create_terminal_printer
    colorama.init(strip=False)
    ^^^^^^^^
NameError: name 'colorama' is not defined
check: exit 1 (0.09 seconds) /home/runner/work/pytest-cov/pytest-cov> isort --check-only --diff src tests setup.py pid=1722
  check: FAIL code 1 (7.13=setup[4.74]+cmd[0.[58](https://github.com/pytest-dev/pytest-cov/actions/runs/4284044499/jobs/7460448953#step:5:59),0.79,0.94,0.09] seconds)
  evaluation failed :( (7.19 seconds)
Error: Process completed with exit code 1.

Copy link
Member

Choose a reason for hiding this comment

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

This looks like something that should be fixed upstream in isort rather than adding extra dependencies here.

We're installing isort 6.0.0b2 (released Dec 12, 2022) but looks like it was fixed in PyCQA/isort#2032, merged Dec 12, 2022, which looks like it was only released in 5.11.1 (also Dec 12, 2022) - but there's no tags for the 6.x beta releases so it's hard to check.

Anyway, I asked in that PR if they can release a 6.x beta with the fix, then we can remove this extra dependency.

Alternatively, we could test on the non-prerelease isort, but I don't know the original reasons for that, so maybe better to keep it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment added.

@@ -6,25 +6,25 @@ jobs:
strategy:
fail-fast: false
matrix:
python-version: ["pypy-3.7", "3.9"]
python-version: ["pypy-3.9", "3.11"]
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be pypy 3.8 (3.7 was dropped but the lower bound seems to be 3.8 not 3.9) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are UPPER bounds, not lower bounds.

Copy link
Member

Choose a reason for hiding this comment

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

I thought it was lower bound (3.7) and upper bound (3.9). Wouldn't we use 3.9 only if it was upper bounds ?

@cclauss cclauss requested review from hugovk and Pierre-Sassoulas and removed request for hugovk and Pierre-Sassoulas February 28, 2023 06:03
tox.ini Outdated Show resolved Hide resolved
Co-authored-by: Hugo van Kemenade <[email protected]>
@hugovk hugovk changed the title Add Python 3.11 and PyPy 3.9 to the testing Add Python 3.11 and PyPy 3.9 to the testing and drop 3.6 Feb 28, 2023
@Pierre-Sassoulas Pierre-Sassoulas merged commit d9789af into pytest-dev:master Feb 28, 2023
@cclauss cclauss deleted the patch-1 branch February 28, 2023 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants