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

Click tests fails on Python 3.12.0-beta.1 #2524

Closed
kdeldycke opened this issue May 30, 2023 · 5 comments
Closed

Click tests fails on Python 3.12.0-beta.1 #2524

kdeldycke opened this issue May 30, 2023 · 5 comments

Comments

@kdeldycke
Copy link
Contributor

The last test job on main branch dates back to a month ago. At that time Python 3.12.0-alpha.7 was used and everything went fine: https://github.com/pallets/click/actions/runs/4874262829/jobs/8694965847#step:3:33

With the release of Python 3.12.0-beta.1, Click tests are failing on the 3.12-dev job, with the following issue:

==================================== ERRORS ====================================
___________________ ERROR collecting tests/test_arguments.py ___________________
/opt/hostedtoolcache/Python/3.12.0-beta.1/x64/lib/python3.12/importlib/__init__.py:90: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1293: in _gcd_import
    ???
<frozen importlib._bootstrap>:1266: in _find_and_load
    ???
<frozen importlib._bootstrap>:1237: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:841: in _load_unlocked
    ???
.tox/py312/lib/python3.12/site-packages/_pytest/assertion/rewrite.py:163: in exec_module
    source_stat, co = _rewrite_test(fn, self.config)
.tox/py312/lib/python3.12/site-packages/_pytest/assertion/rewrite.py:346: in _rewrite_test
    rewrite_asserts(tree, source, strfn, config)
.tox/py312/lib/python3.12/site-packages/_pytest/assertion/rewrite.py:407: in rewrite_asserts
    AssertionRewriter(module_path, config, source).run(mod)
.tox/py312/lib/python3.12/site-packages/_pytest/assertion/rewrite.py:965: in visit_Name
    inlocs = ast.Compare(ast.Str(name.id), [ast.In()], [locs])
/opt/hostedtoolcache/Python/3.12.0-beta.1/x64/lib/python3.12/ast.py:584: in _new
    warnings._deprecated(
/opt/hostedtoolcache/Python/3.12.0-beta.1/x64/lib/python3.12/warnings.py:529: in _deprecated
    warn(msg, DeprecationWarning, stacklevel=3)
E   DeprecationWarning: ast.Str is deprecated and will be removed in Python 3.14; use ast.Constant instead
...

See for instance my latest #2523 PR: https://github.com/pallets/click/actions/runs/5121540227/jobs/9209435195?pr=2523#step:7:73

@darless
Copy link
Contributor

darless commented Jun 26, 2023

Running with python 3.12.0b3 there are several issues:

tox - testenv - pytest

This causes deprecation warnings about ast.Str, they will be removed in python 3.14, but in 3.12, it should continue to work.
To fix we can change the commands to ignore the deprecation warning:

[testenv]
...
commands = pytest -v -W ignore::DeprecationWarning --tb=short --basetemp={envtmpdir} {posargs}

tox - testenv - docs

Running this fails with the error about not being able to find pkg_resources due to it being deprecated.
This can be fixed by adding setuptools to the docs.txt requirements.

tox - testenv - style

The errors that are seen about python 3.12 issues with files in the /root/.cache directory for other files outside of the click repo.

One of the errors looks like this:

  File "/usr/local/lib/python3.12/ast.py", line 417, in generic_visit
    self.visit(value)
  File "/usr/local/lib/python3.12/site-packages/bugbear.py", line 347, in visit
    super().visit(node)
  File "/usr/local/lib/python3.12/ast.py", line 407, in visit
    return visitor(node)
           ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/bugbear.py", line 519, in visit_JoinedStr
    self.check_for_b907(node)
  File "/usr/local/lib/python3.12/site-packages/bugbear.py", line 1267, in check_for_b907
    and value.value[-1] in quote_marks
        ~~~~~~~~~~~^^^^
IndexError: string index out of range

The value.value[-1] is an issue with flake8 it looks like with bugbear. Issue goes away if bugbear is fixed to check that value string is filled in with at least one char before trying to get the last character. If the value is empty then it will throw the IndexError.

From what I can see the style portion of tox is not run in github actions, so we won't see that when running it.

Tox runs the pre-commit which has --all-files set.
Why are we running pre-commit on this? This can be fixed by removing the flake8 in pre-commit and instead running flake8 manually within the repo since we do not care about running on other python libraries.

I'll create a PR that fixes the items mentioned above, specifically flake8 running within the testing repo and not on every library in the system, which does not make sense.

@davidism
Copy link
Member

All of the suggested changes are incorrect, this just requires updating some libraries, which I'll take care of when I have a chance.

@darless
Copy link
Contributor

darless commented Jun 26, 2023

The linked PR actually shows that all the tests passed.
https://github.com/pallets/click/pull/2540/checks

I addressed more issues than the bug is for, the bug is only for the environment py312, the docs and style which are not run in the matrix have issues in 3.12 as I outlined, but are outside of scope of this bug.

That being said, in the current docker python:3.12.0b3, the py312 does not fail.

$ docker run -v $(pwd):/app -w /app -it python:3.12.0b3 bash
root@0042936aa7c4:/app# pip3 install tox -q
root@0042936aa7c4:/app# tox run -e py312
.pkg: _optional_hooks> python /usr/local/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: get_requires_for_build_wheel> python /usr/local/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: build_wheel> python /usr/local/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta
py312: install_package> python -I -m pip install --force-reinstall --no-deps /app/.tox/.tmp/package/10/click-8.2.0.dev0-py3-none-any.whl
py312: commands[0]> pytest -v --tb=short --basetemp=/app/.tox/py312/tmp
...
============================== 579 passed, 21 skipped, 1 xfailed in 1.73s ===============================
.pkg: _exit> python /usr/local/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta
  py312: OK (3.44=setup[1.36]+cmd[2.08] seconds)
  congratulations :) (3.53 seconds)
root@0042936aa7c4:/app# python -V
Python 3.12.0b3

@davidism
Copy link
Member

Yes, you can get them to pass that way, but that's not the correct fix.

@davidism
Copy link
Member

Addressed in b5280c6

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants