test(lerobot): cover metadata refusal paths - #415
Conversation
…in messages exactly The repeated-URL stub fed the loop its own URL forever, so removing the visited-URL guard hung the test instead of failing it: CI reports nothing and a human waits. Bounded at four passes so a guard-less loop terminates and fails, plus an assertion that only one request went out, which is what proves the guard stopped it rather than the bound. match= patterns now go through re.escape. Several messages contain a dot (meta/info.json), so the unanchored originals would also have matched meta/infoXjson, which is not the byte-identical pinning Hebbian-Robotics#405 asked for.
kstonekuan
left a comment
There was a problem hiding this comment.
Merging with a fixup in 454f65f. All eight refusals are reached through the public entry point, and each asserts no landing directory and no manifest, which is what makes them about the fail-loud contract rather than about exception types.
The pagination test hung instead of failing. This is the one that mattered. Your stub fed the loop its own URL forever, so with the visited_urls guard removed:
REPEATED PAGINATION URL guard removed HUNG (no result in 90s)
An anti-hang guard whose test hangs when the guard goes is worse than no test at all: CI reports nothing and burns its job timeout while a human waits for a red mark that never arrives. The stub now drops the next link after four passes, so a guard-less loop terminates and fails on the missing refusal. It goes red in 0.27 seconds. I also added assert fetched_urls == [initial_url], because without it the bound could be doing the stopping and the test would still pass; one request going out is what proves the guard fired on the second pass, before a second fetch.
On the lint fix you pushed while I was reviewing. Raw strings silence RUF043 without doing what it is warning about. r"^meta/info.json not found...$" still has . as a metacharacter, so it matches meta/infoXjson too, and #405 asked for these to be byte-identical. The rule's hint offers "a raw string or re.escape()" as if they were equivalent; only the second one pins. There is now an _exactly() helper that wraps re.escape with the anchors, used at all five sites including the parametrized one that was building f"^{message}$".
All five guards now fail on their own mutation, none hang:
meta/info.json missing 0.44s
no meta/episodes parquet 0.27s
tree not a list of objects 0.47s
repeated pagination URL 0.27s
invalid pagination URL 0.24s
Worth flagging for next time rather than as a complaint: the PR listed GitHub Actions as the authoritative validation, and the submitted commit had five ruff errors. Running uv run ruff check locally takes about a second and would have saved you the round trip. The full suite is the slow part; the linters are not.
Gate on the merged result: ruff check, ruff format --check, ty check clean, 1565 passed / 6 skipped.
Closes #405.
One thing I got wrong on the coordination, and it is worth saying out loud. Sagar-024 handed this to @msparsh13 an hour before you opened this, in a comment on the issue. I never assigned anyone, and in this repo only an assignee reserves an issue, so your PR is the first one and it stands. But the handoff was visible and I should have assigned msparsh13 when I saw it. That is my miss, not yours, and I have said so on the issue.
Summary
Add regression coverage for the eight LeRobot importer metadata refusals called out in #405.
The tests exercise the supported
import_lerobot_datasetentry point and pin failures for:meta/info.json;meta/info.jsonvalues that are not JSON objects;data_pathandvideo_pathtemplates;meta/episodesparquet shards;The pagination cases use a small
urlopenresponse stub so they exercise the real_hf_treeloop while remaining network-free. Each refusal also asserts that no landing directory or prepared manifest is published.No production behavior or refusal text is changed.
Closes #405.
Validation
AI assistance disclosure: AI assistance was used to inspect the open issue, trace the importer paths, prepare the focused regression tests, and review the resulting change.