From 23a1dff1ab0798e9b0b2a58b2f348d47a0255546 Mon Sep 17 00:00:00 2001 From: Sergey Arkhangelskiy Date: Thu, 13 Aug 2026 11:55:14 +0300 Subject: [PATCH 1/5] Make coverage opt-in instead of running on every `pytest` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing consumed the coverage numbers: no threshold, no `--cov-fail-under`, no codecov, and the CI HTML artifact and job summary went unread. It cost ~9% of suite wall time plus ~1.3 s of `pytest-cov` session startup on every invocation, which dominates a single-test run. `coverage` and `pytest-cov` stay in the dev group, so `pytest --cov=positronic --cov=pimm` still works on demand — `docs/CONTRIBUTING.md` documents that path. Also drop the `time.sleep(0.2)` in `test_background_process`: `start_in_subprocess` appends to `background_processes` before it returns, so the following assertion never needed the wait. --- .github/workflows/libero-e2e.yaml | 2 +- .github/workflows/unit-test.yaml | 19 ++++--------------- CLAUDE.md | 4 ++-- README.md | 2 +- pimm/tests/test_world.py | 2 -- positronic/simulator/libero/tests/test_e2e.py | 2 +- pyproject.toml | 4 ---- 7 files changed, 9 insertions(+), 26 deletions(-) diff --git a/.github/workflows/libero-e2e.yaml b/.github/workflows/libero-e2e.yaml index fe48cb385..97562add1 100644 --- a/.github/workflows/libero-e2e.yaml +++ b/.github/workflows/libero-e2e.yaml @@ -41,7 +41,7 @@ jobs: - name: Replay a LIBERO demo through the env server (software rendering) env: MUJOCO_GL: osmesa - run: uv run pytest positronic/simulator/libero/tests/test_e2e.py --no-cov -rs + run: uv run pytest positronic/simulator/libero/tests/test_e2e.py -rs # Reuses the LIBERO checkout the replay above cloned (``~/.cache/positronic/libero/src``). Validates the # command transform (FK/IK + each controller's set_goal inverse) and the gripper normalization against diff --git a/.github/workflows/unit-test.yaml b/.github/workflows/unit-test.yaml index 5ba68077b..c5389b8af 100644 --- a/.github/workflows/unit-test.yaml +++ b/.github/workflows/unit-test.yaml @@ -31,21 +31,10 @@ jobs: - name: Install dependencies run: uv sync --locked - - name: "Run tests (coverage: term, html)" + - name: Run tests env: PYTHONPATH: ${{ env.PYTHONPATH }}:$PWD - run: uv run pytest --cov-report=html - - - name: Coverage summary (job summary) - if: always() - run: uv run coverage report -m >> $GITHUB_STEP_SUMMARY - - - name: Upload coverage HTML artifact - if: always() - uses: actions/upload-artifact@v4 - with: - name: coverage-html-core-${{ matrix.os }}-${{ matrix.python }} - path: htmlcov + run: uv run pytest packaging: runs-on: ubuntu-latest @@ -81,7 +70,7 @@ jobs: - name: Run lerobot vendor tests env: PYTHONPATH: ${{ env.PYTHONPATH }}:$PWD - run: uv run pytest positronic/vendors/lerobot_0_3_3/tests --no-cov + run: uv run pytest positronic/vendors/lerobot_0_3_3/tests lerobot-latest: runs-on: ubuntu-latest @@ -101,7 +90,7 @@ jobs: - name: Run lerobot vendor tests env: PYTHONPATH: ${{ env.PYTHONPATH }}:$PWD - run: uv run pytest positronic/vendors/lerobot/tests --no-cov + run: uv run pytest positronic/vendors/lerobot/tests lockfile-portability: strategy: diff --git a/CLAUDE.md b/CLAUDE.md index 75c838f94..54c74288d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -15,8 +15,8 @@ # Commands - Every Python execution goes through `uv run --locked` — bare `python`/`pytest` bypasses the locked venv -- Run tests: `uv run --locked pytest --no-cov` -- Run single test file: `uv run --locked pytest path/to/test_file.py --no-cov` +- Run tests: `uv run --locked pytest` +- Run single test file: `uv run --locked pytest path/to/test_file.py` - Lint: `uv run --locked ruff check --fix .` - Format: `uv run --locked ruff format .` - Run any Python: `uv run --locked python script.py` diff --git a/README.md b/README.md index 5627200ef..41c3bd4a5 100644 --- a/README.md +++ b/README.md @@ -321,7 +321,7 @@ repoints it at the tool. Run tests and linters from the root directory: ```bash -uv run --locked pytest --no-cov +uv run --locked pytest uv run --locked ruff check . uv run --locked ruff format . ``` diff --git a/pimm/tests/test_world.py b/pimm/tests/test_world.py index 3b1b5c71a..7d8cc8ed3 100644 --- a/pimm/tests/test_world.py +++ b/pimm/tests/test_world.py @@ -1,7 +1,6 @@ import logging import multiprocessing as mp import struct -import time from queue import Empty, Full from unittest.mock import Mock, patch @@ -246,7 +245,6 @@ def test_background_process(self): with world: world.start_in_subprocess(dummy_process) - time.sleep(0.2) # Some time to let the process run assert len(world.background_processes) == 1 # We have to set the private event manually, because out of the scope of the context manager diff --git a/positronic/simulator/libero/tests/test_e2e.py b/positronic/simulator/libero/tests/test_e2e.py index e639d9d0a..62c8835ab 100644 --- a/positronic/simulator/libero/tests/test_e2e.py +++ b/positronic/simulator/libero/tests/test_e2e.py @@ -13,7 +13,7 @@ then run by explicit path (macOS renders via GLFW, no env var; Linux needs ``MUJOCO_GL=osmesa``):: - uv run --locked pytest positronic/simulator/libero/tests/test_e2e.py --no-cov + uv run --locked pytest positronic/simulator/libero/tests/test_e2e.py """ import os diff --git a/pyproject.toml b/pyproject.toml index f0ff38849..0deede18b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -203,10 +203,6 @@ addopts = [ # The suite waits on spawned processes and sockets about as much as it computes, so it scales with cores. # `-n0` puts it back in one process, which live logs and `--pdb` need. "-n", "auto", - "--cov=pimm", - "--cov=platform_client", - "--cov=positronic", - "--cov-report=term-missing", # Needs the `libero` extra and OSMesa; `libero-e2e.yaml` runs it by path, which `--ignore` allows. "--ignore=positronic/simulator/libero/tests/test_e2e.py", ] From 7b0146cc4765c8de15d9a34361d777c4abb49108 Mon Sep 17 00:00:00 2001 From: Sergey Arkhangelskiy Date: Thu, 13 Aug 2026 12:10:30 +0300 Subject: [PATCH 2/5] Wait for the background process to reach its loop before stopping it `dummy_process` yields nothing when it observes a set `_stop_event` on its first condition, so `_bg_wrapper` never runs a loop body and `test_background_process` passed without exercising the control loop it names. `HeartbeatProcess` emits from inside the body, and the test blocks on that message against a deadline before requesting shutdown. --- pimm/tests/test_world.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/pimm/tests/test_world.py b/pimm/tests/test_world.py index 7d8cc8ed3..5787c080a 100644 --- a/pimm/tests/test_world.py +++ b/pimm/tests/test_world.py @@ -1,6 +1,7 @@ import logging import multiprocessing as mp import struct +import time from queue import Empty, Full from unittest.mock import Mock, patch @@ -31,6 +32,18 @@ def dummy_process(stop_reader, clock): yield Sleep(0.01) +class HeartbeatProcess: + """Background process announcing every iteration of its control loop.""" + + def __init__(self, emitter: SignalEmitter): + self.emitter = emitter + + def run(self, stop_reader, clock): + while not stop_reader.read().data: + self.emitter.emit('beat') + yield Sleep(0.01) + + class DummyControlSystem(ControlSystem): """Minimal control system used for integration-style tests.""" @@ -243,16 +256,22 @@ def test_background_process(self): """Test that background processes will run simple control loop.""" world = World() with world: - world.start_in_subprocess(dummy_process) + emitter, receiver = world.mp_pipes() + assert isinstance(receiver, SignalReceiver) + world.start_in_subprocess(HeartbeatProcess(emitter).run) assert len(world.background_processes) == 1 + # Stopping before the child reaches its loop body would leave the loop untested: the generator + # sees a set event on its first condition and returns without ever running the body. + deadline = time.monotonic() + 30 + while receiver.read() is None: + assert time.monotonic() < deadline, 'background process never entered its control loop' + time.sleep(0.01) + # We have to set the private event manually, because out of the scope of the context manager # we can't access exit code of the process world._stop_event.set() - # The child is spawned, so it boots a fresh interpreter and imports this module before it can - # observe the stop event — on a slow runner that outlasts any tight deadline. `join` returns the - # moment it exits, so a generous ceiling costs a passing run nothing. world.background_processes[0].join(timeout=30) assert not world.background_processes[0].is_alive() assert world.background_processes[0].exitcode == 0 From d039ba130b772276d800f36a267a73283f848359 Mon Sep 17 00:00:00 2001 From: Sergey Arkhangelskiy Date: Thu, 13 Aug 2026 12:17:54 +0300 Subject: [PATCH 3/5] Rename `HeartbeatProcess` to `HeartbeatLoop` and move it beside its user `start_in_subprocess` takes a `ControlLoop`, which is what the class supplies through `run`; the class itself manages no process. Its only user is `TestWorld.test_background_process`, so it belongs directly above that class rather than in the helper block 200 lines up. --- pimm/tests/test_world.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/pimm/tests/test_world.py b/pimm/tests/test_world.py index 5787c080a..bdd240ac3 100644 --- a/pimm/tests/test_world.py +++ b/pimm/tests/test_world.py @@ -32,18 +32,6 @@ def dummy_process(stop_reader, clock): yield Sleep(0.01) -class HeartbeatProcess: - """Background process announcing every iteration of its control loop.""" - - def __init__(self, emitter: SignalEmitter): - self.emitter = emitter - - def run(self, stop_reader, clock): - while not stop_reader.read().data: - self.emitter.emit('beat') - yield Sleep(0.01) - - class DummyControlSystem(ControlSystem): """Minimal control system used for integration-style tests.""" @@ -240,6 +228,20 @@ def test_world_defaults_to_system_clock(self): assert isinstance(world.clock, SystemClock) +# Module scope: `start_in_subprocess` pickles the loop to reach a spawned child, and a class defined +# inside the test would not pickle. +class HeartbeatLoop: + """Control loop announcing every iteration of its body.""" + + def __init__(self, emitter: SignalEmitter): + self.emitter = emitter + + def run(self, stop_reader, clock): + while not stop_reader.read().data: + self.emitter.emit('beat') + yield Sleep(0.01) + + class TestWorld: """Test the World class.""" @@ -258,7 +260,7 @@ def test_background_process(self): with world: emitter, receiver = world.mp_pipes() assert isinstance(receiver, SignalReceiver) - world.start_in_subprocess(HeartbeatProcess(emitter).run) + world.start_in_subprocess(HeartbeatLoop(emitter).run) assert len(world.background_processes) == 1 From 0cba30cd44d0ddbef1402699a9aea60308cc09b8 Mon Sep 17 00:00:00 2001 From: Sergey Arkhangelskiy Date: Mon, 17 Aug 2026 15:34:21 +0200 Subject: [PATCH 4/5] Point the documented coverage run at the configured source list `[tool.coverage.run] source` names the packages, so spelling a subset of them again as `--cov=` flags gives the list a second owner that silently under-reports whatever it misses. --- docs/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 792e369f5..a39422062 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -151,7 +151,7 @@ uv run pytest With coverage report: ```bash -uv run pytest --cov=positronic --cov=pimm --cov-report=term-missing +uv run pytest --cov --cov-report=term-missing ``` ## Pull Request Guidelines From ee130a53f6fb0e15a00ec2971c692c0a5a3ac567 Mon Sep 17 00:00:00 2001 From: Sergey Arkhangelskiy Date: Mon, 17 Aug 2026 19:09:55 +0200 Subject: [PATCH 5/5] Bind the heartbeat emitter with `partial` instead of a one-use class The class carried a single field the caller already holds as a local. A module-level generator plus `functools.partial` pickles to a spawned child just as well, and mirrors `dummy_process` above it. --- pimm/tests/test_world.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/pimm/tests/test_world.py b/pimm/tests/test_world.py index bdd240ac3..7423a0e71 100644 --- a/pimm/tests/test_world.py +++ b/pimm/tests/test_world.py @@ -2,6 +2,7 @@ import multiprocessing as mp import struct import time +from functools import partial from queue import Empty, Full from unittest.mock import Mock, patch @@ -228,18 +229,13 @@ def test_world_defaults_to_system_clock(self): assert isinstance(world.clock, SystemClock) -# Module scope: `start_in_subprocess` pickles the loop to reach a spawned child, and a class defined -# inside the test would not pickle. -class HeartbeatLoop: +# Module scope: `start_in_subprocess` pickles the loop to reach a spawned child, and a definition inside +# the test would not pickle. +def heartbeat_loop(emitter, stop_reader, clock): """Control loop announcing every iteration of its body.""" - - def __init__(self, emitter: SignalEmitter): - self.emitter = emitter - - def run(self, stop_reader, clock): - while not stop_reader.read().data: - self.emitter.emit('beat') - yield Sleep(0.01) + while not stop_reader.read().data: + emitter.emit('beat') + yield Sleep(0.01) class TestWorld: @@ -260,7 +256,7 @@ def test_background_process(self): with world: emitter, receiver = world.mp_pipes() assert isinstance(receiver, SignalReceiver) - world.start_in_subprocess(HeartbeatLoop(emitter).run) + world.start_in_subprocess(partial(heartbeat_loop, emitter)) assert len(world.background_processes) == 1