bpo-45756: do not execute `@property` descrs while creating mock autospecs by sobolevn · Pull Request #29901

bpo-45756: do not execute `@property` descrs while creating mock autospecs by sobolevn · Pull Request #29901 · python/cpython

This comment has been minimized.

CC @corona10

Maybe you will be interested to review this? 🙂

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks!

		@@ -0,0 +1,2 @@
		We no longer execute ``@property`` descriptors while creating autospecs in

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

	We no longer execute ``@property`` descriptors while creating autospecs in
	Fixed :class:`property` decorators executing descriptors while creating autospecs in

		@@ -0,0 +1,2 @@
		We no longer execute ``@property`` descriptors while creating autospecs in
		``mock.py``. This was not safe and could affect user's code in unknown way.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

	``mock.py``. This was not safe and could affect user's code in unknown way.
	:mod:`unittest.mock`. This was not safe and could affect user's code in unknown ways.

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

Is this potentially a breaking change (can some mocks be relying on running a property)?

In general, it is a good idea to avoid accidentally running code; however, mocks are advanced case where users are already doing something invasive, and properties are also a special case where programmers are intentionally running behavior upon access.

Long ago, In a slightly related issue, it was decided that hasattr() would be left as-is rather than making perhap futile efforts to avoid running a descriptor.

f0k mentioned this pull request

Feb 22, 2023

@rhettinger

Is this potentially a breaking change (can some mocks be relying on running a property)?

It is possible. However, the execution of properties by specced mocks was introduced relatively recently (3.8) along with AsyncMock, not intentionally but as an accidental side effect of looking for async methods. This was an unintentional breaking change at the time, and it was reported reasonably promptly in #85934. Although we've failed to fix it promptly, it seems to me that it should still be fixed and restored to the long-term established behavior.

The current PR to fix it is #22209.