test: drop the r1_lite fake-e2e harness tests, whose module is not in the repo - #18
Merged
Noietch merged 1 commit intoSep 8, 2026
Conversation
… the repo `tests/hardware/test_r1lite_fake_e2e_harness.py` loads `tools/run_r1lite_fake_e2e.py` via `exec_module`, but that file is not tracked. It was added in 7e630f5 and removed again in da08620, which rewrote .gitignore in the same commit and left the test file behind. All six tests in it have failed on a clean checkout since then with FileNotFoundError. CI runs Ruff only, so nothing surfaced it. Restoring the module does not revive them: it imports a sibling `browser_cdp` (Chrome DevTools driving) that was removed in the same commit and is not tracked either, so the tests fail at import with ModuleNotFoundError. Reviving this would mean re-shipping two local dev modules, which .gitignore's `tools/*` rule suggests was not the intent. Removing the file makes `pytest` reflect what the repo can actually run. Happy to restore both modules behind a .gitignore allow-list instead, as Noietch#14 does for tools/mcp, if the harness is meant to ship.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thanks for the offline test suite — being able to run all of it with no ROS and no hardware is genuinely unusual, and it made this easy to track down.
The question behind the PR: should
tests/hardware/test_r1lite_fake_e2e_harness.pybe dropped, or istools/run_r1lite_fake_e2e.pymeant to ship again? I went with dropping it for the reason below, but this is your call and I'm happy to redo it the other way.What I ran into
On a fresh clone of
mainat0e0e5f8(Python 3.11,uv pip install -e ".[dev]"), all six tests in that file fail with the same cause:The module
exec_modules that path attests/hardware/test_r1lite_fake_e2e_harness.py:17and reads it again at:59. The file is not in the tree, and.gitignore:32-37ignorestools/*with an allow-list for__init__.py,conversion/andskills/.History, if it helps:
58a848b(2026-07-08) added the tool and the tests together9e98a3e(2026-07-09, Revert "Update R1 Lite hardware launch docs and profiles" #4) reverted both7e630f5(2026-07-20) re-added bothda08620(2026-07-20) rewrote.gitignoreand deletedtools/run_r1lite_fake_e2e.py, leaving the test file behind — which is presumably why these six now fail on a clean checkout.github/workflows/has onlyformat-check.ymlandformat-fix.yml, both Ruff-only, which is likely why this hasn't surfaced in CI. Mentioning that only as context, not as a suggestion to add more CI.Why removal rather than restoring the module
I tried restoring it first.
git show 7e630f5:tools/run_r1lite_fake_e2e.pyputs the file back, but the tests still fail — one step later:The harness imports a sibling
browser_cdp(Chrome DevTools driving) at module scope, and that module was removed in the same commit and is not tracked either. So reviving these tests would mean re-shipping two local dev modules, which thetools/*rule suggests wasn't the intent.Removing the file makes
pytestreflect what the repo can actually run. If the harness is meant to ship, I'd be glad to restore both modules behind a.gitignoreallow-list instead, the way #14 does fortools/mcp— just say which you'd prefer.After this change
Measured in the same fully set-up checkout (submodules initialised, adapter envs built),
pytestgoes from8 failed, 1172 passed, 1 skippedonmainto2 failed, 1172 passed, 1 skippedwith this branch. The passed count is deliberately unchanged — this removes six failing tests rather than making them pass. The two that remain are theresets == [True]assertions intests/config/test_control_state.py, which #14 already fixes, so nothing is needed here for them.ruff check .andruff format --check .are clean.For contrast, a bare clone with only the root venv reports
10 failed, 1166 passed, 4 skippedbefore this change: the two extra failures and three extra skips come fromgit submodule update --initandexamples/hardware/r1_lite/setup_env.shnot having been run, not from anything this PR touches. That looks like it's working as intended — I mention it only becausedocs/development.md:16says "No ROS install or hardware required — the robot side is faked." I had read that as meaning a bare checkout should be green, and it took me a little while to work out which failures were mine and which weren't. That may well be my misreading rather than anything wrong with the docs.