Skip to content

Commit 0c5c4f6

Browse files
committed
fix: handle functools.partialmethod in autospec eat_self logic
mock._must_skip returns False for partialmethod attributes (not plain functions), causing eat_self=False and incorrect signature enforcement. Introduce a _must_skip wrapper that corrects this, and document the two upstream issues: - no enforcement without autospec - NonCallableMagicMock with explicit autospec=True.
1 parent a9a9229 commit 0c5c4f6

4 files changed

Lines changed: 66 additions & 8 deletions

File tree

mockey/fixture.py

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,19 @@
2727
_T = TypeVar("_T")
2828

2929

30+
def _must_skip(spec: Any, entry: str, is_type: bool) -> bool:
31+
"""Return whether the first argument must be consumed when autospeccing entry on spec.
32+
33+
Extends mock._must_skip to handle functools.partialmethod, which the
34+
upstream function misidentifies as a non-function and returns False for it.
35+
"""
36+
eat_self: bool = mock._must_skip(spec, entry, is_type) # type: ignore[attr-defined]
37+
if not eat_self and is_type and isinstance(spec.__dict__.get(entry), functools.partialmethod):
38+
eat_self = True
39+
40+
return eat_self
41+
42+
3043
def _lazy_autospec_method(
3144
mocked_method: Any,
3245
original_method: Any,
@@ -104,10 +117,7 @@ def __getattr__(self, name: str) -> Any:
104117
# lazily autospec callable attributes.
105118
original_attr = getattr(original_spec, name)
106119
if callable(original_attr):
107-
# NOTE: _must_skip is a private function in the mock module
108-
eat_self = mock._must_skip( # type: ignore[attr-defined]
109-
original_spec, name, isinstance(original_spec, type)
110-
)
120+
eat_self = _must_skip(original_spec, name, isinstance(original_spec, type))
111121

112122
_lazy_autospec_method(attr, original_attr, eat_self)
113123

@@ -203,10 +213,7 @@ def __enter__(self) -> _T:
203213
if autospec:
204214
target = self.getter()
205215
original_attr = getattr(target, self.attribute)
206-
# NOTE: _must_skip is a private function in the mock module
207-
eat_self = mock._must_skip( # type: ignore[attr-defined]
208-
target, self.attribute, isinstance(target, type)
209-
)
216+
eat_self = _must_skip(target, self.attribute, isinstance(target, type))
210217

211218
new = super().__enter__()
212219

tests/test_issues_exist.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,31 @@ def test_patch_already_mocked_target(self):
173173
patcher.start()
174174
patcher.stop()
175175

176+
def test_patch_partialmethod_not_enforced(self):
177+
# Without the fixture, patching a partialmethod attribute does not
178+
# enforce the effective signature, any call succeeds.
179+
with (
180+
self._restored_patch(),
181+
mock.patch.object(utils._ClientWithPartialMethod, "_send_post_request"),
182+
):
183+
client = utils._ClientWithPartialMethod()
184+
client._send_post_request() # missing url
185+
# too many args
186+
client._send_post_request("https://speedwagon.foundation", "ripple_energy", "za_warudo")
187+
188+
def test_patch_partialmethod_autospec_true(self):
189+
# Upstream, with explicit autospec=True, create_autospec resolves the
190+
# partialmethod descriptor to a NonCallableMagicMock, rendering the
191+
# mock completely uncallable. Any call, even with the correct effective
192+
# signature, raises TypeError.
193+
with (
194+
self._restored_patch(),
195+
mock.patch.object(utils._ClientWithPartialMethod, "_send_post_request", autospec=True) as mocked,
196+
):
197+
self.assertIsInstance(mocked, mock.NonCallableMagicMock)
198+
client = utils._ClientWithPartialMethod()
199+
self.assertRaises(TypeError, client._send_post_request, "https://globex.com")
200+
176201
def test_constructor_autospec_not_enforced(self):
177202
# Without the fixture, constructor signature is NOT enforced.
178203
# mock.Mock / mock.MagicMock do not have an autospec argument

tests/test_issues_fixed.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,23 @@ def test_patch_already_mocked_target(self):
158158
patcher.start,
159159
)
160160

161+
def test_patch_partialmethod(self):
162+
# The effective signature of _send_post_request is (url, payload=None)
163+
# "POST" is pre-filled by partialmethod.
164+
with mock.patch.object(utils._ClientWithPartialMethod, "_send_post_request") as mocked:
165+
client = utils._ClientWithPartialMethod()
166+
167+
# Correct effective args work.
168+
client._send_post_request("https://speedwagon.foundation")
169+
client._send_post_request("https://umbrella.corp", payload="t_virus")
170+
mocked.assert_called_with("https://umbrella.corp", payload="t_virus")
171+
172+
# Missing required arg raises TypeError.
173+
self.assertRaises(TypeError, client._send_post_request)
174+
175+
# Unknown kwarg raises TypeError.
176+
self.assertRaises(TypeError, client._send_post_request, "https://initech.com", foo="lish")
177+
161178
def test_constructor_autospec(self):
162179
# Correct usage.
163180
m = mock.Mock(autospec=utils._ClassWithInit)

tests/utils.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
# License for the specific language governing permissions and limitations
1414
# under the License.
1515

16+
import functools
17+
1618

1719
class Foo:
1820
def bar(self, a, b, c, d=None):
@@ -59,3 +61,10 @@ def get_the_thing(self) -> Foo:
5961

6062
def get_none(self) -> None:
6163
pass
64+
65+
66+
class _ClientWithPartialMethod:
67+
def _send_request(self, method: str, url: str, payload: str | None = None) -> None:
68+
pass
69+
70+
_send_post_request = functools.partialmethod(_send_request, "POST")

0 commit comments

Comments
 (0)