testsuite: fix flaky byte-corruption in check_cmd tests#9771
Merged
ThomasWaldmann merged 1 commit intoJun 13, 2026
Merged
Conversation
Several check_cmd tests corrupted a repo object by overwriting a byte at
a fixed position with a fixed value, e.g.:
manifest[:250] + b"x" + manifest[251:]
Manifests/chunks are stored as AEAD-encrypted repo objects, so their
bytes are ~random. When the target byte already happened to hold the
overwrite value (~1/256), the "corruption" was a no-op: the object
stayed valid, "check" returned 0 instead of 1, and the test failed
intermittently (observed in test_spoofed_archive).
Introduce a corrupt(data, position) helper that flips the byte (XOR
0xFF), so the result is guaranteed to differ, and use it in all the
byte-overwrite corruption sites: test_corrupted_manifest,
test_spoofed_manifest, test_manifest_rebuild_corrupted_chunk,
test_spoofed_archive, test_verify_data and test_corrupted_file_chunk.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
8ccdddf to
27401aa
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9771 +/- ##
==========================================
+ Coverage 84.86% 84.90% +0.04%
==========================================
Files 92 92
Lines 15172 15107 -65
Branches 2273 2260 -13
==========================================
- Hits 12875 12826 -49
+ Misses 1593 1581 -12
+ Partials 704 700 -4 ☔ View full report in Codecov by Harness. |
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
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.
What
Several tests in
archiver/check_cmd_test.pyintermittently failed (e.g.test_spoofed_archivewithassert 0 == 1on the firstcheck).Why
They corrupted a repo object by overwriting a byte at a fixed position with a fixed value:
Manifests and chunks are stored as AEAD-encrypted repo objects (
repokey-aes-ocb), so their bytes are essentially random and differ every run. Whenever the targeted byte already happened to hold the overwrite value (~1 in 256), the "corruption" was a no-op: the object stayed valid,checkfound nothing wrong and returned0, and theexit_code=1assertion failed — hence the flakiness.Fix
Introduce a small helper that flips the byte (XOR
0xFF), so the result is guaranteed to differ:and use it at all the byte-overwrite corruption sites:
test_corrupted_manifest,test_spoofed_manifest,test_manifest_rebuild_corrupted_chunk,test_spoofed_archive,test_verify_data,test_corrupted_file_chunk.The crypto unit tests (
crypto_test.py) do a similar overwrite but with fixed keys/IVs and deterministic output (they assert exact MAC hex), so they are not affected and are left unchanged.Test
Ran the affected tests locally:
14 passed, 10 skipped(skips are the remote/binary variants).🤖 Generated with Claude Code