Skip to content

Fix testAsync, test_stdlibsamples and testSrcPEP420Packages with Python 3.10 #11017

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

Merged
merged 3 commits into from
Oct 8, 2021

Conversation

sbraz
Copy link
Contributor

@sbraz sbraz commented Aug 24, 2021

Description

Hi,
This fixes 3 failing tests with Python 3.10:

mypyc/test/test_run.py::TestRun::testAsync
mypy/test/testsamples.py::SamplesSuite::test_stdlibsamples
mypy/test/testcmdline.py::PythonCmdlineSuite::testSrcPEP420Packages

I have included details about the problems I encountered with Python 3.10 in each commit message.

  • The fix for test_stdlibsamples is trivial.
  • However I have never really used asyncio so I am not 100% sure about the fix for testAsync.
  • As for testSrcPEP420Packages, I don't really understand why there were two closing brackets in the first place.

Please let me know if I should squash the commits but since they tacke different issues I thought it best to leave them this way.

Test Plan

I ran each of the 3 tests with Python 3.8 to 3.10 to make sure I didn't break anything, I guess the CI will double-check that.

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Thank you for these fixes!

Separate commits are fine; we'll squash them on merge.

@@ -791,7 +791,7 @@ c.py:2: error: Argument 1 to "bar" has incompatible type "str"; expected "int"
[case testSrcPEP420Packages]
# cmd: mypy -p anamespace --namespace-packages
[file mypy.ini]
\[mypy]]
\[mypy]
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing this was just a typo.

loop = asyncio.get_event_loop()
result = loop.run_until_complete(f())

result = asyncio.run(f())
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately asyncio.run doesn't exist yet in 3.6 which we still support. You may have to put an if sys.version_info >= (3, 7) check here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I have updated the first commit, however there is a typing problem which I don't really understand and which I have not been able to reproduce by running mypyc on a test file:

mypy.errors.CompileError: native.py:14: error: Unsupported left operand type for >= ("Tuple[int, int, int, str, int]")

I tried to slice sys.version_info[:2] but it did not change anything. Is it because mypyc cannot compare tuples? Should I use sys.hexversion?

Copy link
Member

Choose a reason for hiding this comment

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

That sounds like a problem with the fixtures being used in the test. I'm not sure there is an easy solution. :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you elaborate? What fixtures?
The following is not pretty but works:

if sys.version_info[0] >= 3 and sys.version_info[1] >= 7:

Copy link
Member

Choose a reason for hiding this comment

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

I mean the fixtures in https://github.com/python/mypy/tree/master/test-data/unit/fixtures, which are used instead of typeshed's builtins.pyi in mypy unit tests.

If what you wrote works I'm fine with it, but I suspect it won't typecheck in 3.6. I'm not really familiar with mypyc tests though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weirdly the second comparison doesn't cause a problem, do you know why?
I ran tests with 3.6 with this Dockerfile:

FROM python:3.6
RUN git clone --branch py310 --single-branch https://github.com/sbraz/mypy
WORKDIR mypy
RUN pip install -r test-requirements.txt
RUN pytest -vv mypyc/test/test_run.py

@github-actions

This comment has been minimized.

sbraz added 3 commits August 26, 2021 01:11
The test would previously yield "DeprecationWarning: There is no current
event loop".
by accounting for the move from collections to collections.abc.
Before this change, the test would yield "mypy.ini: No [mypy] section in
config file" because of the double closing bracket.

This is caused by a fix for https://bugs.python.org/issue38741 that is
included in Python 3.10:
python/cpython@1cc6769
@github-actions

This comment has been minimized.

Copy link
Collaborator

@97littleleaf11 97littleleaf11 left a comment

Choose a reason for hiding this comment

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

lgtm!

@JelleZijlstra
Copy link
Member

The mypy-primer output here is suspicious, since this diff should only touch tests. I'm going to close and re-enable to see what happens.

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

No mypy-primer output now, so this is fine to merge.

@JelleZijlstra JelleZijlstra merged commit f42c862 into python:master Oct 8, 2021
@JelleZijlstra
Copy link
Member

Thank you for your contributions!

ilevkivskyi pushed a commit that referenced this pull request Nov 2, 2021
…on 3.10 (#11017)

* tests: make testAsync work with Python 3.10

The test would previously yield "DeprecationWarning: There is no current
event loop".

* tests: fix test_stdlibsamples with Python 3.10

by accounting for the move from collections to collections.abc.

* tests: fix testSrcPEP420Packages with Python 3.10

Before this change, the test would yield "mypy.ini: No [mypy] section in
config file" because of the double closing bracket.

This is caused by a fix for https://bugs.python.org/issue38741 that is
included in Python 3.10:
python/cpython@1cc6769
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants