test: cover the write rollback's success path - #128
Merged
Conversation
Closes the gap #127 found by mutation testing: deleting `handle.seek(pos)` or `handle.truncate()` from `_write_buf` failed no test. The rollback's *failure* was covered by test_write_buf_deletes_partial_on_non_retryable_rollback_failure; its success -- that a failed attempt is discarded and the retry writes over clean state -- was not, so the two lines that guarantee a consistent output file were free to disappear. The new test makes the first attempt put more bytes into the stream than the retry will, then fail with a retryable errno, and asserts on the resulting file rather than on the call sequence. Both halves of the rollback are load-bearing under that setup: - Without `seek(pos)`, `truncate()` cuts at the position left after the debris, so the retried line lands after it. - Without `truncate()`, `seek(pos)` rewinds but the retried line is shorter than the debris, so the debris tail survives past the end of the line. Either way the writer reports success over a malformed file, which is why the assertion is on content. Confirmed to bite: with `handle.truncate()` stubbed out, 1 failed; with `handle.seek(pos)` stubbed out, 1 failed; with both, 1 failed. Each fails on this test's own assertion ("Rollback must discard the failed attempt's bytes"), not incidentally elsewhere. Before this test all three mutants survived. Test-only. `pytest tests/` -- 2243 passed, 10 skipped, 3 xfailed. Baseline unchanged at 77 entries; this adds none.
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.
Closes the gap #127 surfaced by mutation testing: deleting
handle.seek(pos)orhandle.truncate()from_write_buffailed no test.The rollback's failure was covered
(
test_write_buf_deletes_partial_on_non_retryable_rollback_failure). Its success — that afailed attempt is discarded and the retry writes over clean state — was not. So the two lines
that guarantee a consistent output file after a transient write error were free to disappear
unnoticed.
The test
The first attempt puts more bytes into the stream than the retry will, then fails with a
retryable errno. That setup makes both halves of the rollback load-bearing:
seek(pos),truncate()cuts at the position left after the debris, so the retriedline lands after it.
truncate(),seek(pos)rewinds but the retried line is shorter than the debris, sothe debris tail survives past the end of the line.
In both cases the writer reports success over a malformed file — which is why the assertion is
on file contents, not on the call sequence.
It bites
handle.truncate()→passhandle.seek(pos)→passpassEach fails on this test's own assertion (
Rollback must discard the failed attempt's bytes),not incidentally somewhere else — checked, because a mutant caught by an unrelated test would
not actually mean this path is covered.
Checks
pytest tests/test_incremental_writer.pypytest tests/ruff check/ruff format --checkTest-only; no production change.