`the sweep tolerates a staging path that is not a directory` failed once on run
33069641674, against 468 tests that pass on this machine including under
`--rerun-tasks`:
java.io.FileNotFoundException at OutputPublisherStagingTest.kt:112
468 tests completed, 1 failed
Line 112 was `writeBytes` immediately after `deleteRecursively()`.
`FileOutputStream` answers `FileNotFoundException` for an existing directory, so
something had recreated the path inside that window. That something is
`LibreMediaConverterApp.onCreate`, which ends with
appScope.launch { OutputPublisher(...).sweepStaging() }
on `Dispatchers.IO`, and `sweepStaging` reads `stagingDir`, whose getter calls
`mkdirs()`. Robolectric builds the application for every test that asks for one,
so that background `mkdirs()` is in flight across the whole suite on a thread the
paused main looper does not control and no test awaits.
Retrying closes the window rather than narrowing it, because the race is not
symmetric: `mkdirs()` fails on an existing regular file, so the invariant only has
to survive being *established*. Once a write lands, nothing in the suite can turn
this path back into a directory -- which is also why the new assertion that the
sweep left a file behind is worth making.
The `check()` matters as much as the loop. The next failure here should say
"something recreated conversions/ as a directory", not `FileNotFoundException at
line 112` -- that is the difference between a flake someone reads and a flake
someone re-runs.
The wider problem is #159 and is deliberately not fixed here: `AppStartSweepTest`,
`JobSnapshotsTest` and `SpaceArithmeticTest` all name the same path, and the real
answer is an injectable scope rather than a retry loop in every staging test.
#159's done-when is that this loop can be deleted.
Mutation: `listFiles() ?: return` -> `listFiles()!!` reddens exactly this test.
Gate green: assembleDebug, testDebugUnitTest, compileDebugAndroidTestKotlin,
ktlintCheck, detekt, lintDebug.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
destinationIsKnownEmpty's three short-circuits -- no SIZE column, no row,
a null cell -- each had to answer false and none was tested. Its KDoc is
unambiguous about why: "this decides whether a delete is allowed and 'I
could not tell' must never authorise one." The existing tests only ever
drove a provider that answers properly, where the answer is zero and the
delete is correct. Getting the uncertain cases backwards costs the user a
file they already had, on a save that failed.
Also discardStaged's null parentFile, and sweepStaging's null listing --
which is not the case the existing `tolerates a staging directory that
does not exist yet` covers, because stagingDir's own mkdirs() recreates a
missing directory and it then lists as empty. Only a path that cannot be
a directory makes listFiles() answer null.
Five mutations, three bite:
drop !row.isNull(size) short-circuit test red
drop row.moveToFirst() five tests red
parentFile!! instead of ?: return false parentless test red
size >= 0 -> size >= -1 GREEN, does not bite
parentless treated as staged GREEN -- bad mutation, see below
The first green one is recorded in the test as a named exemption.
Measured: getColumnIndex returns -1 for an absent column and isNull(-1)
throws CursorIndexOutOfBoundsException, which the surrounding runCatching
already turns into `?: false`. Same answer, reached by the exception path,
so no behavioural test can pin that conjunct. It stays anyway -- control
flow through an exception is worse than a comparison, and another Cursor
implementation need not throw.
The second was my mistake rather than a finding: substituting stagingDir
for the null parent reaches `return false` by a different route, so it
proves nothing. parentFile!! is the honest mutation and it goes red.
:197, :235 and :258 are now covered. What is left in this file is exactly
what the ticket scoped out: :173-174 (#142) and :267 (#143).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JMR-devandClaude Opus 5