Skip to content

Add missing typings to unittest.mock #7431

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

Conversation

itaisteinherz
Copy link
Contributor

(I found these by adding the --disallow-incomplete-defs flag to the mypy tests.)

@@ -428,14 +428,14 @@ if sys.version_info >= (3, 8):
class AsyncMock(AsyncMockMixin, AsyncMagicMixin, Mock): ... # type: ignore # argument disparities between base classes

class MagicProxy:
name: Any
name: str
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'm not sure if I should change this, however it doesn't really make much sense to me that name is typed as Any in the module's stubs, when it's clear that it's supposed to be a string (at least from what I could tell from the CPython source).

@itaisteinherz
Copy link
Contributor Author

itaisteinherz commented Mar 3, 2022

Note that I doubt that mypy_primer's output will be of use here, as tests in Python projects are usually not type-checked, and the unittest module is usually not used in regular (non-test) code.

@AlexWaygood
Copy link
Member

If these are the only missing annotations in this file, you could also delete this line as part of this PR, increasing pyright's strictness when checking this file as part of CI:

"stdlib/unittest/mock.pyi",

@itaisteinherz
Copy link
Contributor Author

@AlexWaygood will do!

@github-actions

This comment has been minimized.

@itaisteinherz
Copy link
Contributor Author

Seems like there are some missing typings which mypy didn't report. I'll look into pyright's output.

@github-actions

This comment has been minimized.

@@ -74,21 +74,35 @@ class _Sentinel:

sentinel: Any
DEFAULT: Any
_ArgsKwargs = tuple[tuple[Any, ...], Mapping[str, Any]]
_NameArgsKwargs = tuple[str, tuple[Any, ...], Mapping[str, Any]]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that kwargs is typed as Any in the rest of the stubs, but I wanted to be more strict here.

@github-actions

This comment has been minimized.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Skimmed, the simple annotation additions look good to me. If we do want to try making _Call generic over something, I'd move that into its own PR. We've had at least a couple bad regressions involving unittest.mock, so it's something I'm wary of :-)

@itaisteinherz
Copy link
Contributor Author

@hauntsaninja I agree :) I'll revert some of my newer additions.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2022

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@JelleZijlstra JelleZijlstra merged commit b9909b1 into python:master Mar 6, 2022
@itaisteinherz itaisteinherz deleted the feature/unittest-mock-strict branch March 7, 2022 06:34
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.

4 participants