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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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