Skip to content

ci+test: Python quality baseline for the shared bridge (ruff + pytest), fix ActionEvent parser crash - #94

Open
EslaM-X wants to merge 1 commit into
fabricfoundation:mainfrom
EslaM-X:contrib/python-quality
Open

ci+test: Python quality baseline for the shared bridge (ruff + pytest), fix ActionEvent parser crash#94
EslaM-X wants to merge 1 commit into
fabricfoundation:mainfrom
EslaM-X:contrib/python-quality

Conversation

@EslaM-X

@EslaM-X EslaM-X commented Aug 14, 2026

Copy link
Copy Markdown

Summary

The Go side of the tunnel is now linted and unit-tested (#91, #92) — but the Python bridge had zero CI: no lint, no formatting, no unit tests. Any change to the shared zenoh_bridge could regress silently. This PR closes that gap with a minimal, green baseline — and fixes a real crash the new tests exposed.


What lands

pyproject.toml — ruff + pytest, scoped honestly

  • ruff (E/F/I/UP/B/RUF, line-length 100, target-version py39) scoped to the pure-logic modules that are importable without a ROS 2 runtime.
  • ROS 2 packages (isaac_sim_bridge, setup.py, launch/*.launch.py) and command_mapper.py (imports geometry_msgs) are deliberately excluded, so the baseline is green today and grows as new pure modules land.
  • pytest pythonpath points at the pure package dir so the suite never imports ROS 2.

bridge/common/zenoh_bridge/tests/ — 22 unit tests

  • test_action_event.py (15): valid envelopes, empty/missing payloads, params preservation, timestamp handling, malformed JSON, and non-object JSON ([], null, 42).
  • test_utils.py (7): clamp bounds, inclusivity, negative bounds.

.github/workflows/python-quality.yml

ruff check + ruff format --check + pytest on every change touching bridge/common/zenoh_bridge/**, scripts/**, or pyproject.toml — with concurrency, timeout-minutes, least-privilege permissions, consistent with #91/#93.

Bug fix — parse_action_event crashed on non-object JSON

Valid-but-non-object JSON ([], null, 42) made json.loads succeed and then .get("payload") raise AttributeError inside the subscriber's hot path — violating the module's own documented contract "Returns None on parse failure". The parser now rejects any non-dict envelope. (Regression-proven by the new parametrized tests.)


Validation

  • pytest -v: 22/22 pass (0.40s)
  • ruff check . clean
  • ruff format --check . clean
  • ✅ The parser-crash tests fail on the old code and pass after the fix

Notes

— EslaM-X 🇪🇬

…), fix ActionEvent parser crash

The Go side of the tunnel is now tested and linted (see fabricfoundation#91/fabricfoundation#92), but the Python bridge had zero CI: no lint, no formatting, no unit tests. Any change to the shared zenoh_bridge could regress silently. This PR closes that gap with a minimal, green baseline.

What lands:
- pyproject.toml: ruff (E/F/I/UP/B/RUF, line-length 100, target-version py39) scoped to the pure-logic modules that are testable without a ROS 2 runtime; ROS 2 packages (isaac_sim_bridge, setup.py, launch files) and command_mapper (imports geometry_msgs) are deliberately excluded so the baseline is green today and grows as bridges land.
- bridge/common/zenoh_bridge/tests/: 22 unit tests across test_action_event.py and test_utils.py. pytest imports the pure modules directly via pythonpath so the suite never pulls in ROS 2.
- .github/workflows/python-quality.yml: ruff check + ruff format --check + pytest on every change touching the bridge, scripts, or pyproject.toml, with concurrency/timeout/least-privilege consistent with fabricfoundation#91/fabricfoundation#93.
- Fix: parse_action_event crashed with AttributeError on valid-but-non-object JSON (lists, null, scalars) instead of honoring its documented 'Returns None on parse failure' contract. The parser now rejects any non-dict envelope.

Verified locally: 22/22 tests pass, ruff check clean, ruff format --check clean. The new tests cover the parser crash with []/null/42 before and after the fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant