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

add typechecking for patched functions args #124

Merged
merged 4 commits into from Mar 3, 2020

Conversation

deathowl
Copy link
Member

@deathowl deathowl commented Mar 3, 2020

No description provided.

@deathowl deathowl requested a review from fornellas March 3, 2020 16:28
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Mar 3, 2020
def calling_patched_functions_with_bad_types_raises_TypeError(self):
t = TargetStr()
self.mock_callable(t, "typedfun").to_return_value("This is patched")
with self.assertRaises(TypeError):
Copy link
Contributor

Choose a reason for hiding this comment

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

Better use https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertRaisesRegex here, and assert that the error str matches what we expect. Without it, if there's a TypeError for other reasons (eg: giving more arguments than expected), test will pass, but it won't be actually testing type checking.

@@ -10,6 +10,7 @@
import os.path

from unittest.mock import _must_skip
from .lib import _validate_function_signature


def _add_signature_validation(value, template, attr_name):
Copy link
Contributor

Choose a reason for hiding this comment

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

s/_add_signature_validation/_wrap_signature_and_type_validation/g

Fair?

@fornellas fornellas self-requested a review March 3, 2020 17:04
@fornellas
Copy link
Contributor

This is an amazing feature, I feel like it deserves being added to the documentation here
https://testslide.readthedocs.io/en/2.2.1/patching/mock_callable/index.html#signature-validation

I'd add another section "Type Checking Validation" and give an example, similar to the one for signatures.

It is amazing how small this PR is, and how powerful it'll be :-D

Copy link
Contributor

@fornellas fornellas left a comment

Choose a reason for hiding this comment

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

Neat!

@fornellas fornellas merged commit 654ddb7 into facebook:master Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants