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

Move mypy test module exclusions to per-module ignores #13265

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AA-Turner
Copy link
Member

Purpose

I think this is the right thing to do -- we switch from blanket exclusions of the test modules to finer-grained per-module ignores. This won't catch mutch at the moment as most functions are still untyped, but it has found e.g. node.getattr(...) -> getattr(node, ...). I'd welcome feedback though, as mypy config for the tests has changed a few times. I also don't know if we should attemt a similar thing for Pyright yet.

cc @danieleades who originally introduced suggested type-checking for tests/ in #12097.

A

References

@AA-Turner
Copy link
Member Author

Mypy passes locally for me, as typing_test_data.py is excluded, but fails on CI, not sure what's going on here:

tests/test_util/test_util_inspect.py:251: note: In module imported here:
tests/test_util/typing_test_data.py: note: In function "f1":
tests/test_util/typing_test_data.py:10:1: error: Missing return statement  [empty-body]
tests/test_util/typing_test_data.py: note: In function "f2":
tests/test_util/typing_test_data.py:19:1: error: Missing return statement  [empty-body]
tests/test_util/typing_test_data.py: note: In function "f6":
tests/test_util/typing_test_data.py:38:1: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
tests/test_util/typing_test_data.py: note: In function "f7":
tests/test_util/typing_test_data.py:42:17: error: Incompatible default for argument "x" (default has type "None", argument has type "int")  [assignment]
tests/test_util/typing_test_data.py:42:17: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
tests/test_util/typing_test_data.py:42:17: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase
tests/test_util/typing_test_data.py:42:26: error: Missing type parameters for generic type "dict"  [type-arg]
tests/test_util/typing_test_data.py: note: In function "f9":
tests/test_util/typing_test_data.py:51:11: error: Missing type parameters for generic type "Callable"  [type-arg]
tests/test_util/typing_test_data.py: note: In member "__repr__" of class "CustomAnnotation":
tests/test_util/typing_test_data.py:60:5: error: Function is missing a type annotation  [no-untyped-def]
tests/test_util/typing_test_data.py: note: In function "f11":
tests/test_util/typing_test_data.py:64:12: error: Invalid type comment or annotation  [valid-type]
tests/test_util/typing_test_data.py:64:12: note: Suggestion: use CustomAnnotation[...] instead of CustomAnnotation(...)
tests/test_util/typing_test_data.py:64:35: error: Invalid type: try using Literal[123] instead?  [valid-type]
tests/test_util/typing_test_data.py: note: In function "f12":
tests/test_util/typing_test_data.py:68:1: error: Missing return statement  [empty-body]
tests/test_util/typing_test_data.py: note: In function "f15":
tests/test_util/typing_test_data.py:80:12: error: Name "Unknown" is not defined  [name-defined]
tests/test_util/typing_test_data.py: note: In function "f16":
tests/test_util/typing_test_data.py:84:1: error: Function is missing a type annotation  [no-untyped-def]
tests/test_util/typing_test_data.py: note: In function "f17":
tests/test_util/typing_test_data.py:88:1: error: Function is missing a type annotation  [no-untyped-def]
tests/test_util/typing_test_data.py: note: In function "f18":
tests/test_util/typing_test_data.py:92:1: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
tests/test_util/typing_test_data.py:92:1: error: Missing return statement  [empty-body]
tests/test_util/typing_test_data.py:92:32: error: Missing type parameters for generic type "Tuple"  [type-arg]
tests/test_util/typing_test_data.py:92:53: error: Missing type parameters for generic type "Dict"  [type-arg]
tests/test_util/typing_test_data.py: note: In function "f19":
tests/test_util/typing_test_data.py:96:1: error: Function is missing a return type annotation  [no-untyped-def]
tests/test_util/typing_test_data.py: note: In function "f21":
tests/test_util/typing_test_data.py:104:1: error: Function is missing a type annotation  [no-untyped-def]
tests/test_util/typing_test_data.py: note: In function "f22":
tests/test_util/typing_test_data.py:108:1: error: Function is missing a type annotation  [no-untyped-def]
tests/test_util/typing_test_data.py: note: In function "f23":
tests/test_util/typing_test_data.py:112:1: error: Function is missing a type annotation  [no-untyped-def]
tests/test_util/typing_test_data.py: note: In function "f24":
tests/test_util/typing_test_data.py:116:1: error: Function is missing a type annotation  [no-untyped-def]
tests/test_util/typing_test_data.py: note: In function "f25":
tests/test_util/typing_test_data.py:120:1: error: Function is missing a type annotation  [no-untyped-def]
tests/test_util/typing_test_data.py: note: In member "children" of class "Node":
tests/test_util/typing_test_data.py:132:5: error: Missing return statement  [empty-body]
Found 24 errors in 1 file (checked 379 source files)

@picnixz
Copy link
Member

picnixz commented Jan 23, 2025

I don't mind a finer-grained exclusion but sometimes it's a pain to update when you add a new test file. I think I encountered the current failures because typing_test_data is imported weirdly (it's really for typing annotations so it's even more weirder how it's being used).

Concerning the failure it might be because of a local cache. Now, the reason why I used that ignore-list is because of that:

The rationale is that, with ignore-errors, if you mypy the file without removing the module from the overrides list, you will not do anything (namely, mypy just skips the file). However, if you mypy the file and that file is excluded by default, mypy runs normally (so you can force mypyification).

@AA-Turner
Copy link
Member Author

If you have a moment, could you try running mypy with this PR? If it is a CI caching issue then we can look at that, but it might be a Linux vs Windows issue.

@picnixz
Copy link
Member

picnixz commented Jan 23, 2025

Mmh. That's weird. It passes locally as well. I've run this:

$ mypy --config-file=pyproject.toml --no-incremental tests
Warning: unused section(s) in pyproject.toml: module = ['pypi_attestations', 'sigstore.models', 'sigstore.verify', 'sigstore.verify.policy', 'tests.test_abc']
Success: no issues found in 165 source files

And running only mypy does not output any error. I have the same version as the CI's but my Python is Pyhton 3.12.6. However, I don't think we've changed something? I know I encountered a similar issue but I really don't remember how I fixed it or whether I even fixed it...

@adamtheturtle
Copy link
Contributor

If you get rid of exclude you can use e.g.:

[[tool.mypy.overrides]]

module = ["tests.test_util.typing_test_data"]
ignore_errors = true

and this will work and IMO be more consistent - having one way to do excludes.

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.

3 participants