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 ruff on Lib/test/test_typing.py #110179

Merged
merged 2 commits into from Oct 2, 2023

Conversation

AlexWaygood
Copy link
Member

This removes Lib/test/test_typing.py from the ignorelist we have for ruff. If merged, it means we'll get a helpful CI error if we accidentally have two test methods of the same name in the same class, which leads to one of the two tests being silently skipped.

@AlexWaygood
Copy link
Member Author

Locally I was getting this error when running ruff manually (with this PR branch):

>ruff Lib/test
error: Found non-Expr::Tuple argument to PEP 593 Annotation.

I don't know what that means or what's causing it, since there's no line number. But it doesn't seem to be causing any issues in CI 🤷

@hugovk
Copy link
Member

hugovk commented Oct 1, 2023

Minimal reproducer:

from typing import Annotated

Annotated[int]

@hugovk
Copy link
Member

hugovk commented Oct 1, 2023

Which is from:

    def test_too_few_type_args(self):
        with self.assertRaisesRegex(TypeError, 'at least two arguments'):
            Annotated[int]

@AlexWaygood
Copy link
Member Author

Locally I was getting this error when running ruff manually (with this PR branch):

>ruff Lib/test
error: Found non-Expr::Tuple argument to PEP 593 Annotation.

I don't know what that means or what's causing it, since there's no line number. But it doesn't seem to be causing any issues in CI 🤷

I reported this issue on the ruff Discord server (with the help of @hugovk's minimal repro -- thanks!). The issue will go away in the next ruff release (see astral-sh/ruff#7745, which is the fix). So we shouldn't need to worry about this now -- it should be fine to merge this!

@hugovk
Copy link
Member

hugovk commented Oct 2, 2023

I wouldn't be surprised if there's a Ruff release today to support Python 3.12, they just merged a massive PR to support the new f-strings (astral-sh/ruff#7376) and one to declare support (astral-sh/ruff#7713).

Let's wait a little bit and include that in here too.

@hugovk hugovk merged commit 014aacd into python:main Oct 2, 2023
22 of 23 checks passed
@AlexWaygood AlexWaygood deleted the test-typing-enable-ruff branch October 2, 2023 20:15
@AlexWaygood AlexWaygood added the needs backport to 3.12 bug and security fixes label Oct 2, 2023
@miss-islington
Copy link
Contributor

Thanks @AlexWaygood for the PR, and @hugovk for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @AlexWaygood and @hugovk, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 014aacda6239f0e33b3ad5ece343df66701804b2 3.12

@bedevere-app
Copy link

bedevere-app bot commented Oct 3, 2023

GH-110288 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Oct 3, 2023
AlexWaygood added a commit to AlexWaygood/cpython that referenced this pull request Oct 3, 2023
AlexWaygood added a commit to AlexWaygood/cpython that referenced this pull request Oct 3, 2023
@bedevere-app
Copy link

bedevere-app bot commented Oct 3, 2023

GH-110290 is a backport of this pull request to the 3.11 branch.

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.

None yet

3 participants