Skip to content

Commit 38be2c4

Browse files
petercorkeclaude
andauthored
fix(tests): fix Python 3.10-only mock.patch AttributeError in fknm fallback tests (#539)
* fix(tests): fix Python 3.10-only mock.patch AttributeError in fknm fallback tests roboticstoolbox/robot/ETS.py defines a class also called ETS, and robot/__init__.py's `from ...ETS import ETS` rebinds the "ETS" attribute on the roboticstoolbox.robot package to the class, shadowing the submodule of the same name. Python 3.10's unittest.mock resolves dotted-string patch targets via plain getattr (falling back to import only on AttributeError), so patch("...ETS.ETS_fkine", ...) resolved "ETS" to the shadowing class and failed with AttributeError; 3.11+ uses pkgutil.resolve_name and isn't fooled by the same shadowing. This is why it only ever showed up on Python 3.10 in CI. Not a real code bug - the actual fknm/facade fallback machinery was always fine, only the test's patch target resolution was broken on 3.10. Fixed by looking up the real module via sys.modules directly (the only lookup with no getattr involved) and patching against that with patch.object() instead of a dotted string. Rehearsed: all 41 tests in test_fknm_fallback.py pass under both Python 3.10 and 3.13. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(tech-debt): note the Python 3.10 mock workaround for future removal Cross-references the fknm fallback test fix in this same branch. Python 3.10 reaches EOL October 2026 — flag this specific workaround for removal (and a general 3.10-specific-code sweep) at that point. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent fc51bc2 commit 38be2c4

2 files changed

Lines changed: 60 additions & 5 deletions

File tree

tech-debt.md

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,3 +647,39 @@ practice, re-add caching with a corrected key that includes
647647
`matrix.python-version` (or better, restructure so only one job per OS
648648
does the network fetch and others restore from it) — don't just restore
649649
this exact removed code.
650+
651+
---
652+
653+
## Python-3.10-specific workaround: `sys.modules` lookup in `test_fknm_fallback.py`
654+
655+
### Background
656+
657+
`tests/test_fknm_fallback.py` (2026-07-05) has a `_ETS_module =
658+
sys.modules["roboticstoolbox.robot.ETS"]` workaround, with a long comment
659+
explaining why: `roboticstoolbox/robot/ETS.py` defines a class also called
660+
`ETS`, `robot/__init__.py`'s `from ...ETS import ETS` shadows the
661+
submodule of the same name on the `roboticstoolbox.robot` package, and
662+
Python 3.10's `unittest.mock` resolves dotted-string `patch()` targets via
663+
plain `getattr` (falling back to import only on `AttributeError`) — so it
664+
gets fooled by the shadowing and raises `AttributeError`. Python 3.11+
665+
rewrote this resolution to use `pkgutil.resolve_name`, which isn't fooled.
666+
Not a real code bug (the actual fknm/facade fallback machinery was always
667+
fine) — purely a Python-3.10 `unittest.mock` limitation the test had to
668+
work around.
669+
670+
Python 3.10 reaches end-of-life in **October 2026** (per the official
671+
CPython release schedule). `pyproject.toml`'s `requires-python = ">=3.10"`
672+
and the module/class name collision in `ETS.py` aren't going anywhere on
673+
their own, but this specific workaround exists *only* because of 3.10's
674+
mock behavior.
675+
676+
### Proposed fix
677+
678+
When `requires-python` drops support for 3.10 (naturally, around/after its
679+
EOL), search for this specific workaround and simplify
680+
`test_fknm_fallback.py` back to plain `patch("roboticstoolbox.robot.ETS.ETS_fkine", ...)`
681+
-style dotted strings, since 3.11+'s `pkgutil.resolve_name`-based resolution
682+
handles the shadowing correctly on its own. Also worth a quick sweep for
683+
any other `sys.version_info`/Python-3.10-specific conditionals elsewhere in
684+
the codebase at that point, so 3.10 cleanup happens in one pass rather than
685+
piecemeal.

tests/test_fknm_fallback.py

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
"""
1414

1515
import os
16+
import sys
1617
import timeit
1718
import unittest
1819
from contextlib import contextmanager
@@ -23,6 +24,24 @@
2324
import sympy
2425

2526
import roboticstoolbox as rtb
27+
# roboticstoolbox/robot/ETS.py defines a class also called ETS, and
28+
# roboticstoolbox/robot/__init__.py does `from ...ETS import ETS`, which
29+
# rebinds the "ETS" attribute on the roboticstoolbox.robot package to the
30+
# class, shadowing the submodule of the same name. `import a.b.c as x` is
31+
# defined as `import a.b.c; x = a.b.c` — that second step is still
32+
# attribute access, so it hits the same shadowing and also gives the
33+
# class, not the module. sys.modules[...] is a plain dict keyed by the
34+
# literal dotted string, with no getattr involved, so it's the only
35+
# reliably-correct way to get the real module object here.
36+
#
37+
# This only matters for patch() at all because Python 3.10's
38+
# unittest.mock resolves dotted string patch targets via plain getattr
39+
# (falling back to import only on AttributeError), so
40+
# patch("...ETS.ETS_fkine", ...) resolves "ETS" to the shadowing class
41+
# and fails with AttributeError; 3.11+ uses pkgutil.resolve_name and
42+
# isn't fooled. patch.object() against the real module (via sys.modules)
43+
# works on every version.
44+
_ETS_module = sys.modules["roboticstoolbox.robot.ETS"]
2645
from spatialmath import SE3
2746

2847

@@ -57,7 +76,7 @@ def _no_c_fkine():
5776
def _py(fknm, q, base, tool, include_base, _data=None):
5877
return _python_fkine(_data, q, base, tool, include_base)
5978

60-
with patch("roboticstoolbox.robot.ETS.ETS_fkine", new=_py):
79+
with patch.object(_ETS_module, "ETS_fkine", new=_py):
6180
yield
6281

6382

@@ -69,7 +88,7 @@ def _no_c_jacob0():
6988
def _py(fknm, q, tool, _data=None, _n=None):
7089
return _python_jacob0(_data, _n, q, tool)
7190

72-
with patch("roboticstoolbox.robot.ETS.ETS_jacob0", new=_py):
91+
with patch.object(_ETS_module, "ETS_jacob0", new=_py):
7392
yield
7493

7594

@@ -81,7 +100,7 @@ def _no_c_jacobe():
81100
def _py(fknm, q, tool, _data=None, _n=None):
82101
return _python_jacobe(_data, _n, q, tool)
83102

84-
with patch("roboticstoolbox.robot.ETS.ETS_jacobe", new=_py):
103+
with patch.object(_ETS_module, "ETS_jacobe", new=_py):
85104
yield
86105

87106

@@ -101,7 +120,7 @@ def _py(fknm, q, J0, tool, _data=None, _n=None):
101120
verifymatrix(J0, (6, _n))
102121
return _python_hessian(J0)
103122

104-
with patch("roboticstoolbox.robot.ETS.ETS_hessian0", new=_py):
123+
with patch.object(_ETS_module, "ETS_hessian0", new=_py):
105124
yield
106125

107126

@@ -121,7 +140,7 @@ def _py(fknm, q, Je, tool, _data=None, _n=None):
121140
verifymatrix(Je, (6, _n))
122141
return _python_hessian(Je)
123142

124-
with patch("roboticstoolbox.robot.ETS.ETS_hessiane", new=_py):
143+
with patch.object(_ETS_module, "ETS_hessiane", new=_py):
125144
yield
126145

127146

0 commit comments

Comments
 (0)