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 type validation to mock constructor #146

Merged
merged 4 commits into from Apr 9, 2020

Conversation

fornellas
Copy link
Contributor

@fornellas fornellas commented Apr 8, 2020

Ditto.

This is currently failing due to #145, tests will likely pass once that's fixed.

Closes #146.
Closes #142.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Apr 8, 2020
@fornellas fornellas added this to the Type Checking milestone Apr 8, 2020
@fornellas fornellas marked this pull request as draft April 8, 2020 21:16
@fornellas fornellas force-pushed the add_type_validation_to_mock_constructor branch from 5c905e3 to 7bb043d Compare April 9, 2020 09:57
@fornellas
Copy link
Contributor Author

@david-caro your #149 fix worked, thanks!

@fornellas fornellas marked this pull request as ready for review April 9, 2020 10:00
@fornellas fornellas force-pushed the add_type_validation_to_mock_constructor branch from 7bb043d to 9f1bf7c Compare April 9, 2020 12:03
Copy link
Contributor

@david-caro david-caro left a comment

Choose a reason for hiding this comment

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

👍

messenger_mock = testslide.StrictMock(template=Messenger)
self.mock_constructor(sys.modules[__name__], "Messenger").to_return_value(messenger_mock)
with self.assertRaises(TypeError):
# TypeError: Call with incompatible argument types:
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: indents are weird (2 spaces here, 4 in the block before)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed this on master after the merge, tkz.

if target_class_id in _target_class_id_by_original_class_id:
raise RuntimeError("Can not mock the same class at two different modules!")
else:
_target_class_id_by_original_class_id[id(original_class)] = target_class_id

original_class_new = original_class.__new__
Copy link
Contributor

Choose a reason for hiding this comment

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

I would have forgotten about new xd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out, it is there only for being future proof, as mock_constructor() currently can't handle classes that define their own __new__: #155.

@fornellas fornellas merged commit 58de71e into master Apr 9, 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.

Fix argument type validation for *args Update documentation Add type validation for mock_constructor
3 participants