From 267bfc39d1a5ff711ecb3327dc9b9b753ad8bfdf Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Sun, 5 Jul 2026 17:43:11 +1000 Subject: [PATCH 1/2] test(models): add smoke test that every DH/URDF/ETS model constructs Existing tests are hand-written per model, so a model exported in __all__ can go completely untested -- Valkyrie, Fetch, KinovaGen3, FetchCamera, and LBR all had zero coverage this way, three of them silently broken with nothing to catch it (see tech-debt.md). Iterates each category's own __all__ directly (rather than scanning module __dict__/inspect.getmembers, which would also pick up re-exported base classes like Robot/URDFRobot) and asserts every listed class constructs with no arguments. Known-currently-broken models are tracked in an explicit EXPECTED_FAILURES set, which fails loudly in *both* directions: a newly-broken model fails the test, and a since-fixed "expected failure" that starts passing also fails the test, forcing its entry to be cleaned up rather than silently going stale. Co-Authored-By: Claude Sonnet 5 --- tests/test_models.py | 68 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tests/test_models.py b/tests/test_models.py index f303b768a..9ef7921e0 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -142,6 +142,74 @@ def test_pr2(self): rp.models.PR2() +class TestModelSmoke(unittest.TestCase): + """Generic smoke test: every model exported from DH/URDF/ETS must + construct with no arguments. + + The tests above are hand-written per model, so a model can be added to + __all__ and never get a dedicated test -- Valkyrie, Fetch, KinovaGen3, + FetchCamera and LBR all sat with zero test coverage this way, three of + them silently broken (see tech-debt.md for details/history). Iterating + __all__ directly means newly-added models are covered automatically. + """ + + # (category, class name) pairs known to currently fail to construct. + # Remove an entry once its underlying issue is actually fixed -- if you + # don't, this test starts failing for the *opposite* reason (a listed + # failure unexpectedly started passing). + EXPECTED_FAILURES = { + # Bundled rtb-data xacro tree names this directory "kuka_lbr_iiwa", + # but the xacro file's own $(find kuka_lbr_iiwa_support) expects + # the "_support" suffix -- needs an rtb-data rename + republish. + # See tech-debt.md, "rtb-data" section. + ("URDF", "LBR"), + # The next four are already fixed on unmerged branches as of this + # writing (2026-07-05) -- remove once merged to main: + # Valkyrie, Fetch: patched broken upstream robot_descriptions + # files, PR #543. + # KinovaGen3: migrated to robot_descriptions + XACRO_ARGS + # support, PR #546. + # FetchCamera: removed outright (unshippable, unused in RVC3), + # PR #545 -- once merged it disappears from __all__ entirely + # and this entry becomes a harmless no-op; delete it then. + ("URDF", "Valkyrie"), + ("URDF", "Fetch"), + ("URDF", "KinovaGen3"), + ("URDF", "FetchCamera"), + } + + def test_all_models_construct(self): + unexpected_failures = [] + unexpected_passes = [] + + for category_name in ("DH", "URDF", "ETS"): + category = getattr(rp.models, category_name) + for name in category.__all__: + cls = getattr(category, name) + key = (category_name, name) + try: + cls() + except Exception as e: + if key not in self.EXPECTED_FAILURES: + unexpected_failures.append( + f"{category_name}.{name}: {type(e).__name__}: {e}" + ) + else: + if key in self.EXPECTED_FAILURES: + unexpected_passes.append(f"{category_name}.{name}") + + if unexpected_failures: + self.fail( + "Model(s) failed to construct:\n" + "\n".join(unexpected_failures) + ) + if unexpected_passes: + self.fail( + "Model(s) in EXPECTED_FAILURES now construct successfully -- " + "remove from EXPECTED_FAILURES (and close out the matching " + "tech-debt.md entry):\n" + "\n".join(unexpected_passes) + ) + + if __name__ == "__main__": # pragma nocover unittest.main() # pytest.main(['tests/test_SerialLink.py']) From 553c93fd09482bea613dcbb76fcfbc889203192c Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Sun, 5 Jul 2026 21:19:09 +1000 Subject: [PATCH 2/2] test(models): trim EXPECTED_FAILURES now that #543/#545/#546 are merged Valkyrie, Fetch, and KinovaGen3 are now fixed on main; FetchCamera is removed entirely (so its entry would be a harmless no-op, but delete it for hygiene). Only LBR remains genuinely broken. --- tests/test_models.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/tests/test_models.py b/tests/test_models.py index f2e0e79ff..bcdc84f6f 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -163,19 +163,6 @@ class TestModelSmoke(unittest.TestCase): # the "_support" suffix -- needs an rtb-data rename + republish. # See tech-debt.md, "rtb-data" section. ("URDF", "LBR"), - # The next four are already fixed on unmerged branches as of this - # writing (2026-07-05) -- remove once merged to main: - # Valkyrie, Fetch: patched broken upstream robot_descriptions - # files, PR #543. - # KinovaGen3: migrated to robot_descriptions + XACRO_ARGS - # support, PR #546. - # FetchCamera: removed outright (unshippable, unused in RVC3), - # PR #545 -- once merged it disappears from __all__ entirely - # and this entry becomes a harmless no-op; delete it then. - ("URDF", "Valkyrie"), - ("URDF", "Fetch"), - ("URDF", "KinovaGen3"), - ("URDF", "FetchCamera"), } def test_all_models_construct(self):