bpo-35357: Add _mock_ prefix to mangle _Call/_MagicProxy attributes #10873

and3rson · 2018-12-03T19:17:20Z

As suggested by Chris Withers (cjw296) in https://bugs.python.org/issue35357

Renaming them in MagicProxy didn't seem necessary, but I found it better & less error-prone to make them conform to the _mock_* naming style.

EDIT: Reverted renames in MagicProxy as they don't need to conform to _Call & Mock namings because they are different things.

https://bugs.python.org/issue35357

cjw296 · 2018-12-03T21:35:14Z

Lib/unittest/mock.py

-        self.name = name
-        self.parent = parent
+        self._mock_name = name
+        self._mock_parent = parent


Not sure this is related. Please can you add a unit test to show why it's needed?

I've just did a deeper check and it looks like leaving it as it was before doesn't break anything - it's just a matter of conforming to the naming. So I believe we can leave it as it is.

cjw296 · 2018-12-03T21:35:28Z

Lib/unittest/mock.py

-        entry = self.name
-        parent = self.parent
+        entry = self._mock_name
+        parent = self._mock_parent


Please can you add a unit test to show why this is needed?

(Removed, see above)

cjw296 · 2018-12-03T21:36:19Z

Lib/unittest/mock.py

-        self.from_kall = from_kall
+        self._mock_name = name
+        self._mock_parent = parent
+        self._mock_from_kall = from_kall


This is the fix I had in mind, but it needs unit tests to show why it's needed.

cjw296 · 2018-12-03T21:37:07Z

Misc/NEWS.d/next/Core and Builtins/2018-12-03-21-20-24.bpo-35357.rhhoiC.rst

@@ -0,0 +1,3 @@
+Internal attributes' names of unittest.mock._Call and
+unittest.mock.MagicProxy (name, parent & from_kall) are now prefixed by
+_mock_


This should describe the why of the change not just the what (in this case, because of the potential name clash)

bedevere-bot · 2018-12-03T21:37:14Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

and3rson · 2018-12-03T22:52:33Z

I have made the requested changes; please review again.

bedevere-bot · 2018-12-03T22:52:36Z

Thanks for making the requested changes!

@cjw296: please review the changes made to this pull request.

and3rson · 2018-12-03T23:00:51Z

@bedevere-bot I knew I heard your name somewhere.

cjw296

I'm afraid it looks like you've ended up with some infinite recursion.
I'd suggest rebasing on master and trying the whole test suite for mock.
The test you've added would be better split into a couple of simpler cases; for loops in tests aren't usually a good sign!

cjw296 · 2018-12-03T22:53:48Z

Lib/unittest/test/testmock/testcallable.py

@@ -128,7 +128,7 @@ class Multi(SomeClass, Sub):
                    result.foo.assert_called_once_with(3, 2, 1)


-    def test_create_autopsec(self):
+    def test_create_autospec(self):


bedevere-bot · 2018-12-03T23:03:59Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

…Call/_MagicProxy. Fix minor typo in test function name.

and3rson · 2018-12-03T23:21:37Z

@cjw296 My bad - seems like the upstream has just introduced some code that breaks my fix. I've updated my branch and fixed the naming.

Edit: and, yes, I forgot: I have made the requested changes; please review again.

and3rson · 2018-12-04T09:04:14Z

I have made the requested changes; please review again.

bedevere-bot · 2018-12-04T09:04:16Z

Thanks for making the requested changes!

@cjw296: please review the changes made to this pull request.

cjw296

Nice, thanks!

bedevere-bot · 2018-12-04T09:08:48Z

@cjw296: Please replace # with GH- in the commit message next time. Thanks!

miss-islington · 2018-12-04T09:08:58Z

Thanks @and3rson for the PR, and @cjw296 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington · 2018-12-04T09:08:58Z

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

bedevere-bot · 2018-12-04T09:09:11Z

GH-10887 is a backport of this pull request to the 3.7 branch.

…Call/_MagicProxy. (pythonGH-10873) Fix minor typo in test function name. (cherry picked from commit e63e617) Co-authored-by: Andrew Dunai <andunai@gmail.com>

bedevere-bot · 2018-12-04T09:09:21Z

GH-10888 is a backport of this pull request to the 3.6 branch.

…Call/_MagicProxy. (pythonGH-10873) Fix minor typo in test function name. (cherry picked from commit e63e617) Co-authored-by: Andrew Dunai <andunai@gmail.com>

and3rson · 2018-12-04T09:15:30Z

@cjw296 Thank you for taking your time!

…Call/_MagicProxy. (GH-10873) (#10887) Fix minor typo in test function name. (cherry picked from commit e63e617) Co-authored-by: Andrew Dunai <andunai@gmail.com>

…Call/_MagicProxy. (GH-10873) Fix minor typo in test function name. (cherry picked from commit e63e617) Co-authored-by: Andrew Dunai <andunai@gmail.com>

cjw296 · 2018-12-04T09:35:05Z

No problem, thanks for the contribution :-)

In python 3.6.8, 3.7.2, and in python-next, some attributes on the unittest.mock.call object were renamed from things like `name` and `parent` to `_mock_name` and `_mock_parent`. resolves simplistix#102 Python bug: * bpo-35357 (https://bugs.python.org/issue35357) pull requests which added this change: * python/cpython#10873 * python/cpython#10887 * python/cpython#10888

the-knights-who-say-ni added the CLA signed label Dec 3, 2018

bedevere-bot added the awaiting review label Dec 3, 2018

and3rson force-pushed the 35357-mock-name-collision branch from 13152f5 to 76a1da6 Compare December 3, 2018 19:21

cjw296 requested changes Dec 3, 2018

View reviewed changes

bedevere-bot removed the awaiting review label Dec 3, 2018

bedevere-bot added the awaiting changes label Dec 3, 2018

and3rson force-pushed the 35357-mock-name-collision branch from 76a1da6 to 8d4390a Compare December 3, 2018 22:48

bedevere-bot added awaiting change review and removed awaiting changes labels Dec 3, 2018

cjw296 requested changes Dec 3, 2018

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting change review labels Dec 3, 2018

bpo-35357: Add _mock_ prefix to name/parent/from_kall attributes of _…

e4cd309

…Call/_MagicProxy. Fix minor typo in test function name.

and3rson force-pushed the 35357-mock-name-collision branch from 8d4390a to e4cd309 Compare December 3, 2018 23:20

bedevere-bot removed the awaiting changes label Dec 4, 2018

bedevere-bot added the awaiting change review label Dec 4, 2018

cjw296 approved these changes Dec 4, 2018

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Dec 4, 2018

cjw296 merged commit e63e617 into python:master Dec 4, 2018

bedevere-bot removed the awaiting merge label Dec 4, 2018

cjw296 added needs backport to 3.6 labels Dec 4, 2018

bedevere-bot removed the needs backport to 3.7 label Dec 4, 2018

bedevere-bot removed the needs backport to 3.6 label Dec 4, 2018

dsalisbury mentioned this pull request Jan 2, 2019

Address recent changes to unittest.mock.call (3.6.8, 3.7.2, python-next) simplistix/testfixtures#103

Closed

Uh oh!

bpo-35357: Add _mock_ prefix to mangle _Call/_MagicProxy attributes #10873

bpo-35357: Add _mock_ prefix to mangle _Call/_MagicProxy attributes #10873

Uh oh!

Conversation

and3rson commented Dec 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Dec 3, 2018

Uh oh!

and3rson commented Dec 3, 2018

Uh oh!

bedevere-bot commented Dec 3, 2018

Uh oh!

and3rson commented Dec 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cjw296 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Dec 3, 2018

Uh oh!

and3rson commented Dec 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

and3rson commented Dec 4, 2018

Uh oh!

bedevere-bot commented Dec 4, 2018

Uh oh!

cjw296 left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Dec 4, 2018

Uh oh!

miss-islington commented Dec 4, 2018

Uh oh!

miss-islington commented Dec 4, 2018

Uh oh!

bedevere-bot commented Dec 4, 2018

Uh oh!

bedevere-bot commented Dec 4, 2018

Uh oh!

and3rson commented Dec 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cjw296 commented Dec 4, 2018

Uh oh!

Uh oh!

and3rson commented Dec 3, 2018 •

edited

Loading

and3rson commented Dec 3, 2018 •

edited

Loading

and3rson commented Dec 3, 2018 •

edited

Loading

and3rson commented Dec 4, 2018 •

edited

Loading