| 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) *  |
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) *  |
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) * |
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) * |
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) * |
Date: 2017-01-03 22:12 |
| The latest patch (Jiajun Huang, 2017-01-03 14:34) LGTM. |
|
|
| msg284823 - (view) |
Author: Michael Foord (michael.foord) * |
Date: 2017-01-06 14:59 |
| Yep, LGTM as well. Nicely spotted! |
|
|
| msg284839 - (view) |
Author: Roundup Robot (python-dev) |
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) * |
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) * |
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 , 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) * |
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) * |
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 |
|
|