test(warm): probe warm/offline GitHub Action contract#51
Conversation
Add targeted tests verifying the composite action's warm→offline contract and the uvx cache key: - action.yml: assert cache key hashes both tools.py and tools.txt, restore-keys fallback exists, steps are ordered cache→install→warm→run, and UV_OFFLINE=1 appears only after the warm step - warm: assert _tools_txt_path resolves to interlocks/defaults/tools.txt, and per-tool fallback carries the pinned version from DEFAULTS for every tool
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request adds comprehensive tests for the warm task and the GitHub Action configuration. Specifically, it verifies that tools.txt is correctly located, uvx invocations use pinned versions, and the action.yml file maintains the correct step order and cache configuration. Review feedback recommends strengthening the GitHub Action tests by checking for full relative paths in the file hashing expression and validating that the restore keys are properly derived from the primary cache key.
| def test_action_cache_key_covers_pin_material() -> None: | ||
| """Cache key must hash both tools.py (pin table) and tools.txt (compiled hashes).""" | ||
| hash_match = re.search(r"hashFiles\([^)]+\)", _ACTION) | ||
| assert hash_match is not None, "no hashFiles() expression in action.yml cache key" | ||
| hash_expr = hash_match.group() | ||
| assert "tools.py" in hash_expr, "tools.py not in hashFiles expression" | ||
| assert "tools.txt" in hash_expr, "tools.txt not in hashFiles expression" |
There was a problem hiding this comment.
The current test only checks for the presence of the filenames tools.py and tools.txt within the hashFiles expression. It would be more robust to verify the full relative paths (e.g., 'interlocks/defaults/tools.py') to ensure the cache key is correctly tracking the intended files and to guard against accidental path changes in action.yml.
| def test_action_cache_key_covers_pin_material() -> None: | |
| """Cache key must hash both tools.py (pin table) and tools.txt (compiled hashes).""" | |
| hash_match = re.search(r"hashFiles\([^)]+\)", _ACTION) | |
| assert hash_match is not None, "no hashFiles() expression in action.yml cache key" | |
| hash_expr = hash_match.group() | |
| assert "tools.py" in hash_expr, "tools.py not in hashFiles expression" | |
| assert "tools.txt" in hash_expr, "tools.txt not in hashFiles expression" | |
| def test_action_cache_key_covers_pin_material() -> None: | |
| """Cache key must hash both tools.py (pin table) and tools.txt (compiled hashes).""" | |
| hash_match = re.search(r"hashFiles\(([^)]+)\)", _ACTION) | |
| assert hash_match is not None, "no hashFiles() expression in action.yml cache key" | |
| hash_args = hash_match.group(1) | |
| assert "'interlocks/defaults/tools.py'" in hash_args | |
| assert "'interlocks/defaults/tools.txt'" in hash_args |
| def test_action_restore_keys_provides_fallback() -> None: | ||
| """restore-keys must allow a partial cache hit when the exact pin set changes.""" | ||
| assert "restore-keys:" in _ACTION |
There was a problem hiding this comment.
The test test_action_restore_keys_provides_fallback is currently very weak as it only checks for the existence of the string "restore-keys:". It should be improved to verify that the fallback key is actually a prefix of the primary cache key, ensuring the fallback mechanism is correctly configured.
| def test_action_restore_keys_provides_fallback() -> None: | |
| """restore-keys must allow a partial cache hit when the exact pin set changes.""" | |
| assert "restore-keys:" in _ACTION | |
| def test_action_restore_keys_provides_fallback() -> None: | |
| """restore-keys must allow a partial cache hit when the exact pin set changes.""" | |
| key_match = re.search(r"key: (uvx-tools-[^\n]+)", _ACTION) | |
| restore_match = re.search(r"restore-keys: \|\s+([^\n]+)", _ACTION) | |
| assert key_match and restore_match, "Could not find cache key or restore-keys in action.yml" | |
| assert key_match.group(1).startswith(restore_match.group(1).strip()) |
Address gemini-code-assist review feedback on PR #51: - test_action_cache_key_covers_pin_material: capture hashFiles() args group and assert full relative paths ('interlocks/defaults/tools.py' and 'interlocks/defaults/tools.txt') instead of bare filenames, so the test guards against accidental path changes in action.yml. - test_action_restore_keys_provides_fallback: replace the trivial "restore-keys:" substring check with a regex that extracts both the primary cache key and the restore prefix, then asserts the restore key is a true prefix of the primary key, validating the fallback wiring.
1 participant
Summary
Targeted probes for the warm → offline GitHub Action contract. No production code changed.
tests/test_github_action.py— 4 new tests:tools.py(pin table) andtools.txt(compiled hashes)restore-keys:fallback is present for partial cache hitsUV_OFFLINE=1appears only after the warm step (warm must run online)tests/tasks/test_warm.py— 2 new tests:_tools_txt_path()resolves tointerlocks/defaults/tools.txtinside the packageDEFAULTSfor every toolAlso refactored
test_action_metadata_delegates_to_interlock_cito use a module-level_ACTIONconstant (was re-reading the file on each test call).Test plan
uv run pytest -q tests/tasks/test_warm.py tests/test_github_action.py→ 19 passed