Repository navigation
test(egosuite): cover projection validation guards - #268
kstonekuan merged 1 commit into
Conversation
|
👋 Hi @lKolabrodl — thank you so much for your first contribution to HFlow! A maintainer will review your pull request as soon as possible. In the meantime:
💡 Tip: one open pull request per contributor at a time. Issues with an assignee are taken; everything else is fair game. We are excited to have you here and appreciate your help making the project better! 🙌 |
kstonekuan
left a comment
There was a problem hiding this comment.
LGTM, merging. Welcome.
Reran the mutation experiment independently. Every guard has exactly the coverage it should, and no test fires for a guard it does not own:
guard removed tests that went red
quaternion norm zero-quaternion-norm, nonfinite-quaternion-norm
width/height positive zero-width, zero-height
focal lengths positive zero-horizontal-focal-length, zero-vertical-focal-length
calibration finite all four nonfinite-focal / nonfinite-principal-point cases
nonempty joints empty-joint-sequence
joint coords finite nonfinite-joint-x, nonfinite-joint-y, nonfinite-joint-z
On DoD 4, the boundary answer is stronger than the PR body claims. Zero is the only value that pins these three guards. Weakening them instead of deleting them:
width/height: <= 0 becomes < 0 -> zero-width, zero-height fail
focal lengths: <= 0.0 becomes < 0.0 -> both zero-focal cases fail
quaternion norm: <= 0.0 becomes < 0.0 -> zero-quaternion-norm fails
A width=-1 fixture would have caught none of those. An off-by-one in the comparison is the realistic regression here, so zero is the correct side, not just the convenient one.
Two more choices that are easy to get wrong. math.nan for the finiteness cases rather than -math.inf: the positivity check runs first and -inf <= 0.0 is True, so an infinite focal length lands on "must be positive" and never reaches the guard you meant to cover. And matching the message rather than the type. With the quaternion guard removed:
Quaternion(inf, 0, 0, 1) -> ValueError: hand joint coordinates must be finite
Quaternion(0, 0, 0, 0) -> ZeroDivisionError: division by zero
The first one still raises ValueError, just for a different reason two guards later, so a bare pytest.raises(ValueError) would have passed on the wrong refusal.
Gate clean on the example project: ruff check, ruff format --check, ty check, 21 passed.
The fcntl shim you mentioned is correctly not in the diff.
Closes #267.
Summary
Add table-driven rejection coverage for every validation guard in the EgoSuite projection geometry:
This is test-only and does not change production behavior.
Why
The existing tests covered valid projections but did not prove that malformed camera or joint data is rejected. A future regression could therefore remove any of the guards without failing the suite.
Verification
uv run --project examples/egosuite_evaluation ruff check examples/egosuite_evaluationuv run --project examples/egosuite_evaluation ruff format --check examples/egosuite_evaluationuv run --project examples/egosuite_evaluation ty check examples/egosuite_evaluationuv run --project examples/egosuite_evaluation pytest -q examples/egosuite_evaluation/tests(21 passed)The native Windows test run used a local no-op
fcntlimport shim solely for test collection, consistent with the project's documented Windows limitation. These geometry tests do not exercise file locking or storage.AI assistance
AI assistance was used to implement and validate this change. The final diff and test results were reviewed before submission.