Skip to content

Commit 86d6f7a

Browse files
committed
fix(harness): address Copilot/CodeRabbit review nits
- local_harness: fail fast (exit 2) on empty --legs and on --ci without UNITY_IMAGE, instead of silently exiting green / crashing with an unhandled CalledProcessError - local_harness: correct the nearest-patch `best` type annotation (key is (patch:int, suffix:str), not a 3-tuple) - test_tool_test_symmetry: fix the module docstring to match behavior (only test_*.py files are scanned; non-test_*.py scripts like tests/e2e/bridge_smoke.py do not count toward coverage) Deferred CodeRabbit's SHA-pin suggestion for e2e-bridge.yml: the repo pins actions by tag (@v4/@v6), not SHA, so pinning one workflow would be inconsistent — left as a repo-wide policy decision.
1 parent 60f43e0 commit 86d6f7a

2 files changed

Lines changed: 12 additions & 4 deletions

File tree

Server/tests/test_tool_test_symmetry.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22
33
Enforces the CLAUDE.md rule that "Every new feature needs tests". A tool module
44
(any file under ``services/tools/`` that exposes an ``@mcp_for_unity_tool``) is
5-
considered covered when any file under ``Server/tests/`` references it by module
6-
name -- a dedicated ``test_<module>.py``, a characterization test, or an
7-
integration test all count.
5+
considered covered when a ``test_*.py`` file under ``Server/tests/`` references it
6+
by module name -- a dedicated ``test_<module>.py`` or a characterization test
7+
counts. Non-``test_*.py`` scripts (e.g. ``tests/e2e/bridge_smoke.py``) are not
8+
scanned, so a tool exercised only there still needs a unit/characterization test.
89
910
Tools that are genuinely untested today live in ``KNOWN_UNTESTED`` so this guard
1011
fails for *new* untested tools without forcing a full backfill first. SHRINK the

tools/local_harness.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ def discover_editor(version: str,
382382
if sec:
383383
roots.append(sec)
384384

385-
best: tuple[tuple[int, int, str], str, str] | None = None # ((patch, suffix-as-key), binary, dir_version)
385+
best: tuple[tuple[int, str], str, str] | None = None # ((patch, suffix), binary, dir_version)
386386
for root in roots:
387387
try:
388388
entries = _listdir(root)
@@ -1349,8 +1349,15 @@ def main(argv: list[str] | None = None) -> int:
13491349
args = build_arg_parser().parse_args(argv)
13501350

13511351
legs = parse_legs(args.legs)
1352+
if not legs:
1353+
print("::error:: --legs did not include any valid values (allowed: smoke,editmode,playmode)")
1354+
return 2
13521355
if args.ci:
13531356
args.no_warmup = True
1357+
if not os.environ.get("UNITY_IMAGE"):
1358+
print("::error:: --ci requires the UNITY_IMAGE environment variable "
1359+
"(the unityci/editor image to run the headless Editor in)")
1360+
return 2
13541361

13551362
# Resolve project path (repo-relative or absolute).
13561363
project_path = Path(args.project_path)

0 commit comments

Comments
 (0)