From ac815cac8a1b39a3e6862b9035132639d9786147 Mon Sep 17 00:00:00 2001 From: Elena Oat Date: Sat, 2 Nov 2019 20:41:50 -0700 Subject: [PATCH 1/8] bpo-38669: Add check for the type of target --- Lib/unittest/mock.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index a48132c5b1cb5bd..17cd5689880a56b 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -1378,6 +1378,10 @@ async def patched(*args, **keywargs): def get_original(self): target = self.getter() + if type(target) != type: + raise TypeError( + "Target %s must be a class" % (target,) + ) name = self.attribute original = DEFAULT From 2dbf0a5ea21d508a646376c1f997f05f616c4cba Mon Sep 17 00:00:00 2001 From: Elena Oat Date: Sun, 3 Nov 2019 18:45:08 -0800 Subject: [PATCH 2/8] Move type check into _patch_object Also add a test. --- Lib/unittest/mock.py | 10 +++++----- Lib/unittest/test/testmock/testpatch.py | 9 +++++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 17cd5689880a56b..7b8e58f5f1e8915 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -1378,10 +1378,6 @@ async def patched(*args, **keywargs): def get_original(self): target = self.getter() - if type(target) != type: - raise TypeError( - "Target %s must be a class" % (target,) - ) name = self.attribute original = DEFAULT @@ -1605,7 +1601,11 @@ def _patch_object( When used as a class decorator `patch.object` honours `patch.TEST_PREFIX` for choosing which methods to wrap. """ - getter = lambda: target + if type(target) is str: + raise TypeError(f"Target {target} must be a class") + else: + getter = lambda: target + return _patch( getter, attribute, new, spec, create, spec_set, autospec, new_callable, kwargs diff --git a/Lib/unittest/test/testmock/testpatch.py b/Lib/unittest/test/testmock/testpatch.py index 0632d95e58fec05..5ac0b8553ebec81 100644 --- a/Lib/unittest/test/testmock/testpatch.py +++ b/Lib/unittest/test/testmock/testpatch.py @@ -105,6 +105,15 @@ def test(): self.assertEqual(Something.attribute, sentinel.Original, "patch not restored") + def test_patchobject_with_string_as_target(self): + class Something(object): + def do_something(self): + pass + + def test(): + with patch.object('Something', 'do_something'): + pass + self.assertRaises(TypeError, test) def test_patchobject_with_none(self): class Something(object): From 92c73517aaf8ff8b352bd6c0826772a8f16e69eb Mon Sep 17 00:00:00 2001 From: Elena Oat Date: Sun, 3 Nov 2019 19:56:51 -0800 Subject: [PATCH 3/8] Simplify test --- Lib/unittest/test/testmock/testpatch.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/Lib/unittest/test/testmock/testpatch.py b/Lib/unittest/test/testmock/testpatch.py index 5ac0b8553ebec81..7ebda0be11f3abf 100644 --- a/Lib/unittest/test/testmock/testpatch.py +++ b/Lib/unittest/test/testmock/testpatch.py @@ -106,14 +106,7 @@ def test(): "patch not restored") def test_patchobject_with_string_as_target(self): - class Something(object): - def do_something(self): - pass - - def test(): - with patch.object('Something', 'do_something'): - pass - self.assertRaises(TypeError, test) + self.assertRaises(TypeError, patch.object, 'Something', 'do_something') def test_patchobject_with_none(self): class Something(object): From 8d163131e37a522fbeb827cc08ee951a07f83cf2 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Mon, 4 Nov 2019 02:54:17 +0000 Subject: [PATCH 4/8] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Misc/NEWS.d/next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst diff --git a/Misc/NEWS.d/next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst b/Misc/NEWS.d/next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst new file mode 100644 index 000000000000000..dfde466e96a101d --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst @@ -0,0 +1 @@ +Raise TypeError if the target is a string when patching an object. \ No newline at end of file From 726c5dfd7d90ca3e6f2987ab22e4c164bf8653a4 Mon Sep 17 00:00:00 2001 From: Elena Oat Date: Mon, 4 Nov 2019 20:09:32 -0800 Subject: [PATCH 5/8] Improve the error message Remove the unnecessary else too. --- Lib/unittest/mock.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 7b8e58f5f1e8915..dff2fd057b8fcb4 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -1602,9 +1602,9 @@ def _patch_object( for choosing which methods to wrap. """ if type(target) is str: - raise TypeError(f"Target {target} must be a class") - else: - getter = lambda: target + raise TypeError(f"{target!r} must be the actual object to be patched, " + f"not a str") + getter = lambda: target return _patch( getter, attribute, new, spec, create, From 16c178b2da29cd70e1903f7d3abd3f7cfd52493a Mon Sep 17 00:00:00 2001 From: Elena Oat Date: Tue, 5 Nov 2019 20:04:02 -0800 Subject: [PATCH 6/8] Improve test to test for exception message Also improve formatting and enrich news. --- Lib/unittest/mock.py | 6 +++--- Lib/unittest/test/testmock/testpatch.py | 3 ++- .../next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index dff2fd057b8fcb4..b3cf6d855687f17 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -1602,10 +1602,10 @@ def _patch_object( for choosing which methods to wrap. """ if type(target) is str: - raise TypeError(f"{target!r} must be the actual object to be patched, " - f"not a str") + raise TypeError( + f"{target!r} must be the actual object to be patched, not a str" + ) getter = lambda: target - return _patch( getter, attribute, new, spec, create, spec_set, autospec, new_callable, kwargs diff --git a/Lib/unittest/test/testmock/testpatch.py b/Lib/unittest/test/testmock/testpatch.py index 7ebda0be11f3abf..cedf549725a0414 100644 --- a/Lib/unittest/test/testmock/testpatch.py +++ b/Lib/unittest/test/testmock/testpatch.py @@ -106,7 +106,8 @@ def test(): "patch not restored") def test_patchobject_with_string_as_target(self): - self.assertRaises(TypeError, patch.object, 'Something', 'do_something') + with self.assertRaisesRegex(TypeError, f"'Something' must be the actual object to be patched, not a str"): + patch.object('Something', 'do_something') def test_patchobject_with_none(self): class Something(object): diff --git a/Misc/NEWS.d/next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst b/Misc/NEWS.d/next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst index dfde466e96a101d..3279d3d9ce566b5 100644 --- a/Misc/NEWS.d/next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst +++ b/Misc/NEWS.d/next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst @@ -1 +1 @@ -Raise TypeError if the target is a string when patching an object. \ No newline at end of file +Raise :exc:`TypeError` when patching a string with :meth:`unittest.mock.patch.object`. \ No newline at end of file From f03482646282c4422c5fe4315f7761c3783be6cc Mon Sep 17 00:00:00 2001 From: Elena Oat Date: Wed, 6 Nov 2019 20:26:08 -0800 Subject: [PATCH 7/8] Improve the News --- Misc/NEWS.d/next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst b/Misc/NEWS.d/next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst index 3279d3d9ce566b5..5060ecf2dc5a468 100644 --- a/Misc/NEWS.d/next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst +++ b/Misc/NEWS.d/next/Tests/2019-11-04-02-54-16.bpo-38669.pazXZ8.rst @@ -1 +1 @@ -Raise :exc:`TypeError` when patching a string with :meth:`unittest.mock.patch.object`. \ No newline at end of file +Raise :exc:`TypeError` when passing target as a string with :meth:`unittest.mock.patch.object`. \ No newline at end of file From d0abbe01c5ad2eded4b029c7b40ed11860ffbbc3 Mon Sep 17 00:00:00 2001 From: Elena Oat Date: Fri, 8 Nov 2019 19:02:55 -0800 Subject: [PATCH 8/8] PEP8 improvements No need to use f-string. --- Lib/unittest/test/testmock/testpatch.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/unittest/test/testmock/testpatch.py b/Lib/unittest/test/testmock/testpatch.py index cedf549725a0414..e065a2c35fbeec5 100644 --- a/Lib/unittest/test/testmock/testpatch.py +++ b/Lib/unittest/test/testmock/testpatch.py @@ -106,7 +106,8 @@ def test(): "patch not restored") def test_patchobject_with_string_as_target(self): - with self.assertRaisesRegex(TypeError, f"'Something' must be the actual object to be patched, not a str"): + msg = "'Something' must be the actual object to be patched, not a str" + with self.assertRaisesRegex(TypeError, msg): patch.object('Something', 'do_something') def test_patchobject_with_none(self):