classification
Title: unittest.mock._Call ignores `name` parameter
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.7, Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Jiajun Huang, abarry, berker.peksag, michael.foord, python-dev, vstinner
Priority: normal Keywords: patch

Created on 2016-12-13 13:47 by Jiajun Huang, last changed 2017-03-31 16:36 by dstufft. This issue is now closed.

Files
File name Uploaded Description Edit
mock_class_call.patch Jiajun Huang, 2016-12-14 02:15 review
mock.patch Jiajun Huang, 2016-12-14 02:41 review
mock.patch Jiajun Huang, 2016-12-14 05:46 review
mock.patch Jiajun Huang, 2016-12-16 07:12 review
mock.patch Jiajun Huang, 2017-01-03 14:34 review
Pull Requests
URL Status Linked Edit
PR 305 merged berker.peksag, 2017-02-26 11:39
PR 306 merged berker.peksag, 2017-02-26 11:42
PR 552 closed dstufft, 2017-03-31 16:36
Messages (19)
msg283104 - (view) Author: Jiajun Huang (Jiajun Huang) * Date: 2016-12-13 13:47
code in `_Call.__new__`:

      def __new__(cls, value=(), name=None, parent=None, two=False,                                                                    
      ¦   ¦   ¦   from_kall=True):                                                                                                     
      ¦   name = ''                                                                                                                    
      ¦   args = ()                                                                                                                    
      ¦   kwargs = {}                                                                                                                  
      ¦   _len = len(value)                                                                                                            
      ¦   if _len == 3:                                                                                                                
              ...

the parameter `name` had been override since the function starts. so whatever name is, it's been ignored. Is it a bug? or something else?
msg283120 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-12-13 16:38
That indeed looks like a bug. Well spotted :)

That code has been there since unittest.mock was added back in March 2012. If I were to guess, I'd say that it should be `if name is None: name = ''`. Care to submit a patch?
msg283157 - (view) Author: Jiajun Huang (Jiajun Huang) * Date: 2016-12-14 02:15
Thanks for reply :) the patch has been uploaded.
msg283159 - (view) Author: Jiajun Huang (Jiajun Huang) * Date: 2016-12-14 02:41
update the patch file follow the doc(https://docs.python.org/devguide/gitdevs.html)
msg283164 - (view) Author: Jiajun Huang (Jiajun Huang) * Date: 2016-12-14 05:46
The new patch has been updated. :)
msg283231 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-12-15 02:46
Thanks for the patch, Jiajun. The _Call class is tested in CallTest (located in Lib/unittest/test/testmock/testhelpers.py) Would it be possible to add a test case to make sure that we actually fixed the bug?
msg283235 - (view) Author: Jiajun Huang (Jiajun Huang) * Date: 2016-12-15 03:40
I think we can write `_Call.__new__` as:

def __new__(cls, value=(), name='',...)

it's much simpler and readable.
msg283369 - (view) Author: Jiajun Huang (Jiajun Huang) * Date: 2016-12-16 07:12
code and test case has been updated.
msg284246 - (view) Author: Jiajun Huang (Jiajun Huang) * Date: 2016-12-29 10:44
hi, do this need more test case or something else to be merged? please let me know :)
msg284564 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-01-03 14:09
Please remove "import ipdb; ipdb.set_trace() # TODO remove it" before posting patches ;-)
msg284572 - (view) Author: Jiajun Huang (Jiajun Huang) * Date: 2017-01-03 14:34
sorry about that, fixed.
msg284589 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-01-03 19:25
The patch looks good to me. There are some styling issues in the test code (e.g. no need to add trailing spaces after commas in some cases), but I can take care of them before committing the patch :) Thanks!
msg284595 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-01-03 22:12
The latest patch (Jiajun Huang, 2017-01-03 14:34) LGTM.
msg284823 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2017-01-06 14:59
Yep, LGTM as well. Nicely spotted!
msg284839 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2017-01-06 17:16
New changeset 50424a903593 by Victor Stinner in branch '3.6':
Fix unittest.mock._Call: don't ignore name
https://hg.python.org/cpython/rev/50424a903593
msg284840 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-01-06 17:18
I applied the latest mock.patch to Python 3.6 and default (future 3.7). I prefer to wait for the 3.5.3 release before backporting the fix to 3.5, the fix is minor, I don't want to annoy the release manager yet.
msg284885 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-01-07 05:04
IIRC 3.5.3rc1 is already tagged so the 3.5 branch is open for 3.5.4. Anything that needs to be in 3.5.3 should be cherry-picked by the RM at this point if I'm not mistaken (and note that I don't have time check, but 3.5.3 may be the last bugfix release of 3.5 series so you may just skip 3.5 :))

Like I already said in msg284589, the test code doesn't follow the current style in Lib/unittest/test/testmock/testhelpers.py and it can be simplified like the following patch:

     def test_call_with_name(self):
-        self.assertEqual(
-            'foo',
-            _Call((), 'foo')[0],
-        )
-        self.assertEqual(
-            '',
-            _Call((('bar', 'barz'), ), )[0]
-        )
-        self.assertEqual(
-            '',
-            _Call((('bar', 'barz'), {'hello': 'world'}), )[0]
-        )
+        self.assertEqual(_Call((), 'foo')[0], 'foo')
+        self.assertEqual(_Call((('bar', 'barz'),),)[0], '')
+        self.assertEqual(_Call((('bar', 'barz'), {'hello': 'world'}),)[0], '')
msg290393 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-03-24 23:42
New changeset fae59e1aa87ee9598d032e0bd697969a5784025f by Berker Peksag in branch '3.6':
bpo-28961: Address my comments from earlier code review (#306)
https://github.com/python/cpython/commit/fae59e1aa87ee9598d032e0bd697969a5784025f
msg290395 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-03-24 23:43
New changeset 5aa3856b4f325457e8ec1ccf669369f543e1f6b5 by Berker Peksag in branch 'master':
bpo-28961: Address my comments from earlier code review (#305)
https://github.com/python/cpython/commit/5aa3856b4f325457e8ec1ccf669369f543e1f6b5
History
Date User Action Args
2017-03-31 16:36:27dstufftsetpull_requests: + pull_request1011
2017-03-24 23:43:06berker.peksagsetmessages: + msg290395
2017-03-24 23:42:40berker.peksagsetmessages: + msg290393
2017-02-26 13:06:44berker.peksagsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-02-26 11:42:13berker.peksagsetpull_requests: + pull_request270
2017-02-26 11:39:21berker.peksagsetpull_requests: + pull_request269
2017-01-07 05:04:12berker.peksagsetmessages: + msg284885
2017-01-06 17:18:32vstinnersetmessages: + msg284840
2017-01-06 17:16:32python-devsetnosy: + python-dev
messages: + msg284839
2017-01-06 14:59:50michael.foordsetmessages: + msg284823
2017-01-03 22:12:24vstinnersetmessages: + msg284595
2017-01-03 19:25:05berker.peksagsetmessages: + msg284589
2017-01-03 14:34:54Jiajun Huangsetfiles: + mock.patch

messages: + msg284572
2017-01-03 14:09:41vstinnersetnosy: + vstinner
messages: + msg284564
2016-12-29 10:44:56Jiajun Huangsetmessages: + msg284246
2016-12-16 07:12:06Jiajun Huangsetfiles: + mock.patch

messages: + msg283369
2016-12-15 03:40:52Jiajun Huangsetmessages: + msg283235
2016-12-15 02:46:46berker.peksagsetnosy: + berker.peksag

messages: + msg283231
stage: commit review -> patch review
2016-12-14 13:06:07abarrysetstage: needs patch -> commit review
2016-12-14 05:46:47Jiajun Huangsetfiles: + mock.patch

messages: + msg283164
2016-12-14 02:41:35Jiajun Huangsetfiles: + mock.patch

messages: + msg283159
2016-12-14 02:15:27Jiajun Huangsetfiles: + mock_class_call.patch
keywords: + patch
messages: + msg283157
2016-12-13 16:38:15abarrysetversions: + Python 3.5, Python 3.6
type: enhancement -> behavior

nosy: + abarry, michael.foord
title: Is it a bug(method `_Call.__new__` in unittest.mock)? -> unittest.mock._Call ignores `name` parameter
messages: + msg283120
stage: needs patch
2016-12-13 13:47:48Jiajun Huangcreate