Skip to content

Test the pure-Python fallback path (and, optionally, real Pyodide) in CI #633

Description

@petercorke

Background

ci.yml's test-core/test jobs currently assert _C_AVAILABLE == True for both _fknm_c and _frne_c -- meaning the real test suite only ever exercises the compiled C path. tests/test_fknm_fallback.py does narrow, targeted C-vs-Python cross-validation, but nothing runs the whole suite against the pure-Python fallback path.

This gap is exactly how a real bug shipped undetected: a stale branch, feat/pyodide-pure-wheel-v2 (pushed to origin 2026-07-23, 4 commits, never merged -- 109 commits behind current main as of 2026-08-25, too stale to merge as-is), ran the full test suite inside a real Pyodide environment (pyodide venv, Node.js-backed) with zero compiled extensions present, and found 4 real issues:

  1. test_ETS.py::test_insert -- a genuine off-by-one (7-joint model, 6-element q) that the C extension's lack of bounds checking silently tolerated (q[6] read past the array end, undefined behaviour, happened not to crash). The pure-Python path correctly raised IndexError.
  2. URDFRobot.py's _load_rd_module -- the sys.platform == "emscripten" guard only checked inside except Exception, not except ImportError; GitPython's subprocess failure under Pyodide surfaces as a plain ImportError, so the guard never fired and users got a misleading "model renamed" error instead of the real, actionable one.
  3. test_ET.py's .fknm-identity tests -- not bugs, just needed skipUnless(_C_AVAILABLE) gating (meaningless assertions with no compiled extension present at all).
  4. test_trajectory.py::test_quintic -- a velocity-tolerance too tight for cross-platform float noise; Pyodide's WASM-compiled numpy gives ~-3.7e-15 at a point desktop numpy gives ~-1e-16 for.

Also logged there (in that branch's now-defunct tech-debt.md addition, not carried forward as its own issue): IK.py's _calc_qnull divides by a near-zero quantity, causing an overflow that recovers gracefully on desktop but hard-crashes (math.cos(-inf)) under Pyodide's WASM numpy for the same seeded case. Not yet independently verified against current main -- worth checking if this is picked up.

What was cherry-picked from that branch (2026-08-25, this session)

Findings 1 and 2 are real, live bugs, confirmed still present on main at the time of filing -- fixed directly (with fail/fix/pass verification against a real editable install of this fix, not just eyeballing the diff), independent of everything else on that branch. Findings 3 and 4 were not re-verified or ported in this pass -- 3 is cosmetic (gating, not a bug); 4 (the WASM-numpy tolerance) can't be reproduced without a real Pyodide/WASM numpy build, which wasn't set up this session. Worth reverifying both if/when the discussion below is acted on.

The actual open question: is real-Pyodide CI testing still worth it?

Splitting what that branch's test-pyodide CI job was actually catching:

  • Findings 1, 2, and 3 are pure Python-vs-C-path coverage gaps -- nothing WASM-specific about them. They'd be caught by running the whole suite against the pure-Python fallback path on an ordinary desktop Linux runner. No Pyodide, no Emscripten, no Node.js needed.
  • Finding 4 is the only genuinely Pyodide/WASM-specific one -- a real difference between WASM-compiled numpy's floating-point behaviour and desktop numpy's. No desktop test run would ever surface it; either real Pyodide execution or a defensively-generous tolerance (not derived from observing the exact WASM behaviour) are the only ways to handle it.

Since this session's separate change (roboticstoolbox-python#632) made the Pyodide/JupyterLite wheel a genuine py3-none-any build, the cheap fix for the first bullet is now nearly free: a CI job that installs that wheel into a normal ubuntu-latest runner and runs the full pytest tests/ suite (no Pyodide, no Node.js, no pyodide venv) would catch findings-1/2/3-class issues directly. The old branch's real-Pyodide job (pyodide venv + Node.js) would still be needed to catch finding-4-class issues, but that's a narrower, optional value-add on top, not a prerequisite.

Next steps, if picked up

  1. Add the cheap desktop job first (install the RTB_PURE_WHEEL=1 wheel, run the full suite on a normal runner) -- low cost, catches the bulk of what was found.
  2. Re-verify findings 3 and 4 against current main before deciding whether the heavier real-Pyodide job is worth adding on top.
  3. Re-check whether the IK.py _calc_qnull numerical issue (noted above, never independently filed) is still live; file its own issue if so.

Metadata

Metadata

Assignees

No one assigned

    Labels

    tech-debtKnown technical debt / deferred cleanup, not a live bug

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions