Fix golden fixture hashes on Windows and add CI - #104
Merged
Conversation
The three assertions in tests/test_golden_run.py have failed on every Windows checkout since the fixtures were added. The fixtures were never corrupted: they are stored LF and hash to exactly the pinned values, but core.autocrlf=true with no .gitattributes checked them out as CRLF, changing their bytes and therefore their SHA-256. Normalizing CRLF to LF makes all three byte-identical to the committed blobs. Fixed by adding .gitattributes, so the working tree is LF on every platform. The pinned hashes and the fixture contents are both unchanged. Verified against a fresh clone configured with core.autocrlf=true. This also explains the earlier churn in that file: 4fbab87 made the tests green by repinning to the Windows CRLF hashes, which would then have failed on Linux, and a71d477 correctly reverted it. Neither commit found the cause. Adds the repository's first CI. The ubuntu x windows matrix is load-bearing rather than redundancy: a Linux-only matrix cannot catch a CRLF checkout altering hashed bytes, and a Windows-only matrix cannot catch someone pinning Windows-specific hashes. pip-audit runs weekly on a schedule rather than as a PR gate, since it queries a remote advisory database and should not block unrelated reviews. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
utils/id_generator.py and runners/run_demo.py imported datetime.UTC, which was added in Python 3.11, so `import hardstop` raised ImportError on 3.10 while pyproject.toml advertised requires-python = ">=3.10". The package could not be installed and used on its own declared floor. Replaced with timezone.utc, which UTC is an alias for, so behavior is unchanged on every version. Found by the CI matrix added in the previous commit, on its first run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test_cmd_incidents_replay_emits_run_record picked the replay record with
sorted(glob("*.json"))[-1]. RunRecords are named
"{started_at}_{run_id}.json", so two records written within the same
clock tick share a timestamp prefix and sort by their random UUID
suffix. On Windows, where clock granularity is coarser, that made the
selection a coin flip: the test passed on the first CI run and failed on
the second with the baseline correlation.window record.
Select by operator_id instead, which is what the assertion actually
means. Only the test was wrong; record filenames remain unique.
Verified with 30 consecutive runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
tests/test_golden_run.pyhas been failing on Windows checkouts since the fixtures were added. The fixtures were never corrupted. They are stored LF and hash to exactly the pinned values, butcore.autocrlf=truewith no.gitattributeschecked them out as CRLF — 9, 22 and 101 translated line endings — changing their bytes and therefore their SHA-256.Normalize CRLF→LF and all three are byte-identical to the committed blobs:
event_spill.json72e81b37…(pinned)c6e92df5…normalized_event_spill.json4c8538e8…(pinned)e44452d1…incident_evidence_spill.json723ce988…(pinned)4857a37c…Fixed with
.gitattributesso the working tree is LF on every platform. The pinned hashes and the fixture contents are both unchanged — the only content change in this PR is.gitattributes, the workflow, a CHANGELOG entry, and atestpathssetting.This also explains the earlier churn in that file:
4fbab87made the tests green by repinning to the Windows CRLF hashes (which would then have failed on Linux), anda71d477correctly reverted it. The revert was right; neither commit found the cause.CI
The repository had no CI (
.github/contained onlyFUNDING.yml).The ubuntu × windows matrix is load-bearing rather than redundancy: a Linux-only matrix cannot catch a CRLF checkout altering hashed bytes, and a Windows-only matrix cannot catch someone pinning Windows-specific hashes. Running both is what keeps
.gitattributeshonest. There is also an explicit line-ending check before install, so a regression reports as itself rather than as an opaque hash mismatch.pip-auditruns weekly on a schedule and on manual dispatch, deliberately not as a PR gate — it queries a remote advisory database and a network blip should not block an unrelated review.Verification
Verified in a fresh clone of this branch configured with
core.autocrlf=true(the setting that caused the bug), with its own venv so no editable install could leak source from elsewhere:🤖 Generated with Claude Code