Repository: python/cpython
Pull request: gh-103329: Add regression test for PropertyMock with side effect
#102213 introduced an optimisation to `__getattr__` that had some unexpected side effects, one of which was a change in behavior of PropertyMock when the property has a side effect.

This change was reverted in #103332; This PR adds a regression test to ensure that PropertyMock behavior is preserved. It also enhances the test of the return value for PropertyMock.

Fixes #103329.

<!-- gh-issue-number: gh-103329 -->
* Issue: gh-103329
<!-- /gh-issue-number -->


Problem statement:
3.12.0a7 changes behavior of PropertyMock
# Bug report

3.12.0a7 changed the behavior of `unittest.mock.PropertyMock`

Setup:
```
from unittest.mock import MagicMock, PropertyMock
m = MagicMock()
p1 = PropertyMock(return_value=3)
p2 = PropertyMock(side_effect=ValueError)
type(m).foo = p1
type(m).bar = p2
```

In Python 3.12.0a6 and earlier, `m.foo` evaluates as 3, and `m.bar` raises a ValueError.

In Python 3.12.0a7, `m.foo` evaluates as 3, and `m.bar` returns `<MagicMock name='mock.bar' id=...>`

# Your environment

- CPython versions tested on: 3.12.0a7, 3.12.0a6, 3.11.2, 3.10.10, 3.9.16, 3.8.16
- Operating system and architecture: 
  - macOS Ventura 13.2.1, on an M1 MacBook Pro.
  - GitHub Actions macOS 12 x86_64 
  - GitHub Actions Ubuntu 22.04
  - GitHub Actions Windows Server 2022


<!-- gh-linked-prs -->
### Linked PRs
* gh-103358
* gh-103364
<!-- /gh-linked-prs -->

Hints from issue discussion:
A known on-going failure was introduced in the related components: https://github.com/python/cpython/commit/aa0a73d1bc53dcb6348a869df1e775138991e561

See https://github.com/python/cpython/issues/103272

Can anyone test if it starts here?

@sunmy2019 I'm not sure aa0a73d is the cause here (or, at least, it isn't the *only* thing in play); in my testing, that commit still raises a ValueError - although the presentation is slightly different, as it also includes
```
>>> m.bar
Traceback (most recent call last):
  File "/Users/rkm/projects/python/cpython/github/Lib/unittest/mock.py", line 2957, in __get__
    return self()
           ^^^^^^
  File "/Users/rkm/projects/python/cpython/github/Lib/unittest/mock.py", line 1128, in __call__
    return self._mock_call(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/rkm/projects/python/cpython/github/Lib/unittest/mock.py", line 1132, in _mock_call
    return self._execute_mock_call(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/rkm/projects/python/cpython/github/Lib/unittest/mock.py", line 1187, in _execute_mock_call
    raise effect
ValueError

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/rkm/projects/python/cpython/github/Lib/unittest/mock.py", line 656, in __getattr__
    if name.startswith(('assert', 'assret', 'asert', 'aseert', 'assrt')) or name in _ATTRIB_DENY_LIST:
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
SystemError: <method 'startswith' of 'str' objects> returned a result with an exception set

```
whereas 3.11 et al raise:
```
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/unittest/mock.py", line 2932, in __get__
    return self()
           ^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/unittest/mock.py", line 1118, in __call__
    return self._mock_call(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/unittest/mock.py", line 1122, in _mock_call
    return self._execute_mock_call(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/unittest/mock.py", line 1177, in _execute_mock_call
    raise effect
ValueError
```

Looks like it was fixed by https://github.com/python/cpython/pull/103332

With current main, I got the same output as in your python3.11. @freakboy3742
Before closing this issue, we should add a test for this case that would have failed on the original breaking PR.

Commit message: 📜🤖 Added by blurb_it.

Patch to review:
diff --git a/Lib/test/test_unittest/testmock/testhelpers.py b/Lib/test/test_unittest/testmock/testhelpers.py
index 9e7ec5d62d5da2..760011ac625141 100644
--- a/Lib/test/test_unittest/testmock/testhelpers.py
+++ b/Lib/test/test_unittest/testmock/testhelpers.py
@@ -1077,7 +1077,7 @@ def test_propertymock(self):
             p.stop()
 
 
-    def test_propertymock_returnvalue(self):
+    def test_propertymock_bare(self):
         m = MagicMock()
         p = PropertyMock()
         type(m).foo = p
@@ -1088,6 +1088,27 @@ def test_propertymock_returnvalue(self):
         self.assertNotIsInstance(returned, PropertyMock)
 
 
+    def test_propertymock_returnvalue(self):
+        m = MagicMock()
+        p = PropertyMock(return_value=42)
+        type(m).foo = p
+
+        returned = m.foo
+        p.assert_called_once_with()
+        self.assertEqual(returned, 42)
+        self.assertNotIsInstance(returned, PropertyMock)
+
+
+    def test_propertymock_sideffect(self):
+        m = MagicMock()
+        p = PropertyMock(side_effect=ValueError)
+        type(m).foo = p
+
+        with self.assertRaises(ValueError):
+            m.foo
+        p.assert_called_once_with()
+
+
 class TestCallablePredicate(unittest.TestCase):
 
     def test_type(self):
diff --git a/Misc/NEWS.d/next/Tests/2023-04-08-00-50-23.gh-issue-103329.M38tqF.rst b/Misc/NEWS.d/next/Tests/2023-04-08-00-50-23.gh-issue-103329.M38tqF.rst
new file mode 100644
index 00000000000000..79448ed728040d
--- /dev/null
+++ b/Misc/NEWS.d/next/Tests/2023-04-08-00-50-23.gh-issue-103329.M38tqF.rst
@@ -0,0 +1 @@
+Regression tests for the behaviour of ``unittest.mock.PropertyMock`` were added.


Code review comments:
diff_hunk: @@ -1088,6 +1088,27 @@ def test_propertymock_returnvalue(self):
         self.assertNotIsInstance(returned, PropertyMock)
 
 
+    def test_propertymock_returnvalue(self):
+        m = MagicMock()
+        p = PropertyMock(return_value=42)
+        type(m).foo = p
+
+        returned = m.foo
+        p.assert_called_once_with()
+        self.assertEqual(returned, 42)
+        self.assertNotIsInstance(returned, PropertyMock)
+
+
+    def test_propertymock_sideffect(self):
path: Lib/test/test_unittest/testmock/testhelpers.py
text: @user1:
Nit:
```suggestion
    def test_propertymock_side_effect(self):
```