From 154012f9f17409e429ca36f4f9d7665bac61d4d8 Mon Sep 17 00:00:00 2001 From: Christian Klein <167265+cklein@users.noreply.github.com> Date: Tue, 3 Jan 2023 12:35:06 +0100 Subject: [PATCH 1/4] Prevent calling assert functions without the prefix "assert_" in safe mode Raise an AttributeError when calling e.g. `mock_obj.called_once` instead of `mock_obj.assert_called_once`. --- Lib/test/test_unittest/testmock/testmock.py | 6 ++++++ Lib/unittest/mock.py | 8 ++++++-- .../2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst | 3 +++ 3 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst diff --git a/Lib/test/test_unittest/testmock/testmock.py b/Lib/test/test_unittest/testmock/testmock.py index 8a92490137f9a7..5c85e7f2a2a72c 100644 --- a/Lib/test/test_unittest/testmock/testmock.py +++ b/Lib/test/test_unittest/testmock/testmock.py @@ -1645,12 +1645,18 @@ def test_mock_unsafe(self): m.aseert_foo_call() with self.assertRaisesRegex(AttributeError, msg): m.assrt_foo_call() + with self.assertRaisesRegex(AttributeError, msg): + m.called_once_with() + with self.assertRaisesRegex(AttributeError, msg): + m.has_calls() m = Mock(unsafe=True) m.assert_foo_call() m.assret_foo_call() m.asert_foo_call() m.aseert_foo_call() m.assrt_foo_call() + m.called_once_with() + m.has_calls() #Issue21262 def test_assert_not_called(self): diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index 994947cad518f9..23c4ac814d1470 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -653,7 +653,7 @@ def __getattr__(self, name): elif _is_magic(name): raise AttributeError(name) if not self._mock_unsafe: - if name.startswith(('assert', 'assret', 'asert', 'aseert', 'assrt')): + if name.startswith(('assert', 'assret', 'asert', 'aseert', 'assrt')) or name in ATTRIB_DENY_LIST: raise AttributeError( f"{name!r} is not a valid assertion. Use a spec " f"for the mock if {name!r} is meant to be an attribute.") @@ -1062,6 +1062,10 @@ def _calls_repr(self, prefix="Calls"): return f"\n{prefix}: {safe_repr(self.mock_calls)}." +# gh-100690 Denylist for forbidden method names in safe mode +ATTRIB_DENY_LIST = {name.removeprefix("assert_") for name in dir(NonCallableMock) if name.startswith("assert_")} + + class _AnyComparer(list): """A list which checks if it contains a call which may have an argument of ANY, flipping the components of item and self from @@ -1231,7 +1235,7 @@ class or instance) that acts as the specification for the mock object. If `return_value` attribute. * `unsafe`: By default, accessing any attribute whose name starts with - *assert*, *assret*, *asert*, *aseert* or *assrt* will raise an + *assert*, *assret*, *asert*, *aseert*, *assrt*, or *called_* will raise an AttributeError. Passing `unsafe=True` will allow access to these attributes. diff --git a/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst b/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst new file mode 100644 index 00000000000000..cacbeffe99dd1f --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst @@ -0,0 +1,3 @@ +Mock objects which are not unsafe will now raise an AttributeError if an +attribute with the prefix ``called_`` is accessed, in addition to this already +happening for the prefixes assert, assret, asert, aseert, assrt. From 2bd97a17b535de645c873c0f2553aad91a28767f Mon Sep 17 00:00:00 2001 From: Christian Klein <167265+cklein@users.noreply.github.com> Date: Thu, 5 Jan 2023 07:45:29 +0100 Subject: [PATCH 2/4] Added tests to show that names from denylist still can be used with a spec --- Lib/test/test_unittest/testmock/testmock.py | 18 ++++++++++++++++++ Lib/unittest/mock.py | 6 +++--- ...3-01-02-16-59-49.gh-issue-100690.2EgWPS.rst | 4 ++-- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_unittest/testmock/testmock.py b/Lib/test/test_unittest/testmock/testmock.py index cadda352c2f21a..97fe66fa4ca2a5 100644 --- a/Lib/test/test_unittest/testmock/testmock.py +++ b/Lib/test/test_unittest/testmock/testmock.py @@ -1647,14 +1647,32 @@ def test_mock_unsafe(self): m.assrt_foo_call() with self.assertRaisesRegex(AttributeError, msg): m.called_once_with() + with self.assertRaisesRegex(AttributeError, msg): + m.called_once() with self.assertRaisesRegex(AttributeError, msg): m.has_calls() + + class Foo(object): + def called_once(self): + pass + + def has_calls(self): + pass + + m = Mock(spec=Foo) + m.called_once() + m.has_calls() + + m.called_once.assert_called_once() + m.has_calls.assert_called_once() + m = Mock(unsafe=True) m.assert_foo_call() m.assret_foo_call() m.asert_foo_call() m.aseert_foo_call() m.assrt_foo_call() + m.called_once() m.called_once_with() m.has_calls() diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index b1dd84e1422e79..ce462f7e4053d8 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -1235,9 +1235,9 @@ class or instance) that acts as the specification for the mock object. If `return_value` attribute. * `unsafe`: By default, accessing any attribute whose name starts with - *assert*, *assret*, *asert*, *aseert*, *assrt*, or *called_* will raise an - AttributeError. Passing `unsafe=True` will allow access to - these attributes. + *assert*, *assret*, *asert*, *aseert*, *assrt*, or an attribute name that matches + an assertion method without the prefix *assert_* will raise an + AttributeError. Passing `unsafe=True` will allow access to these attributes. * `wraps`: Item for the mock object to wrap. If `wraps` is not None then calling the Mock will pass the call through to the wrapped object diff --git a/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst b/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst index cacbeffe99dd1f..6901dce22d90b7 100644 --- a/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst +++ b/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst @@ -1,3 +1,3 @@ Mock objects which are not unsafe will now raise an AttributeError if an -attribute with the prefix ``called_`` is accessed, in addition to this already -happening for the prefixes assert, assret, asert, aseert, assrt. +attribute that matches the name of an existing assertion is accessed, e.g. ``called_once`` instead of ``assert_called_once``. +This is in addition to this already happening for accessing attributes with prefixes assert, assret, asert, aseert, and assrt. From 01946cb02964f45985c5ec83f52663bfd115aa9b Mon Sep 17 00:00:00 2001 From: Christian Klein <167265+cklein@users.noreply.github.com> Date: Fri, 6 Jan 2023 17:11:51 +0100 Subject: [PATCH 3/4] Changed documentation --- Lib/unittest/mock.py | 10 ++++++---- .../2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py index ce462f7e4053d8..78827d61b69dc3 100644 --- a/Lib/unittest/mock.py +++ b/Lib/unittest/mock.py @@ -1062,7 +1062,7 @@ def _calls_repr(self, prefix="Calls"): return f"\n{prefix}: {safe_repr(self.mock_calls)}." -# gh-100690 Denylist for forbidden attribute names in safe mode +# Denylist for forbidden attribute names in safe mode ATTRIB_DENY_LIST = {name.removeprefix("assert_") for name in dir(NonCallableMock) if name.startswith("assert_")} @@ -1235,9 +1235,11 @@ class or instance) that acts as the specification for the mock object. If `return_value` attribute. * `unsafe`: By default, accessing any attribute whose name starts with - *assert*, *assret*, *asert*, *aseert*, *assrt*, or an attribute name that matches - an assertion method without the prefix *assert_* will raise an - AttributeError. Passing `unsafe=True` will allow access to these attributes. + *assert*, *assret*, *asert*, *aseert*, or *assrt* raises an AttributeError. + Additionally, an AttributeError is raised when accessing + attributes that match the name of an assertion method without the prefix + `assert_`, e.g. accessing `called_once` instead of `assert_called_once`. + Passing `unsafe=True` will allow access to these attributes. * `wraps`: Item for the mock object to wrap. If `wraps` is not None then calling the Mock will pass the call through to the wrapped object diff --git a/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst b/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst index 6901dce22d90b7..65ab2a733404f1 100644 --- a/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst +++ b/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst @@ -1,3 +1,3 @@ -Mock objects which are not unsafe will now raise an AttributeError if an -attribute that matches the name of an existing assertion is accessed, e.g. ``called_once`` instead of ``assert_called_once``. +Mock objects which are not unsafe will now raise an AttributeError when accessing an +attribute that matches the name of an assertion but without the prefix ``assert_``, e.g. accessing ``called_once`` instead of ``assert_called_once``. This is in addition to this already happening for accessing attributes with prefixes assert, assret, asert, aseert, and assrt. From e9b8f739a5790a8c4a7b9110a23ae70c1d76ffe8 Mon Sep 17 00:00:00 2001 From: Christian Klein <167265+cklein@users.noreply.github.com> Date: Fri, 6 Jan 2023 18:41:00 +0100 Subject: [PATCH 4/4] Reformatted NEWS --- .../2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst b/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst index 65ab2a733404f1..3796772aebdf6d 100644 --- a/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst +++ b/Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst @@ -1,3 +1,7 @@ -Mock objects which are not unsafe will now raise an AttributeError when accessing an -attribute that matches the name of an assertion but without the prefix ``assert_``, e.g. accessing ``called_once`` instead of ``assert_called_once``. -This is in addition to this already happening for accessing attributes with prefixes assert, assret, asert, aseert, and assrt. +``Mock`` objects which are not unsafe will now raise an +``AttributeError`` when accessing an attribute that matches the name +of an assertion but without the prefix ``assert_``, e.g. accessing +``called_once`` instead of ``assert_called_once``. +This is in addition to this already happening for accessing attributes +with prefixes ``assert``, ``assret``, ``asert``, ``aseert``, +and ``assrt``.