Stacked on #144 — based on test/fake-provider-scaffolding because the cursor third needs RowShape. Merge #144 first, then this retargets to main cleanly. Items 2 and 3 below are independent of it.
1. destinationIsKnownEmpty's three short-circuits (:197)
No SIZE column, no row, a null cell. Each must answer false, and none was tested. The KDoc is unambiguous:
Every other answer … is false, because 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 size 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. The contrast test is right above them in the file.
2. discardStaged's null parentFile (:235)
A relative single-segment name has no parent. The handle reaches the ViewModel as a path string out of WorkInfo.outputData and is turned straight into a File, so it is not a shape the caller can rule out.
3. sweepStaging's null listing (:258)
Not the case the sweep tolerates a staging directory that does not exist yet covers — stagingDir's own mkdirs() recreates a missing directory, which then lists as empty. Only a path that cannot be a directory makes listFiles() answer null.
Mutations — five run, three bite
mutation
result
drop !row.isNull(size)
red — short-circuit test
drop row.moveToFirst()
red — five tests
parentFile!! instead of ?: return false
red — parentless test
size >= 0 → size >= -1
green — does not bite
parentless file treated as staged
green — my bad mutation, not a finding
The first green one is a named exemption, recorded in the test. 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, not a finding. Substituting stagingDir for the null parent reaches return false by a different route, so it proves nothing either way. parentFile!! is the honest mutation and it goes red.
Coverage
:197, :235 and :258 now covered. What remains in this file is exactly what the ticket scoped out: :173-174 (#142) and :267 (#143), both seam work.
Local gate green: ktlintCheck, detekt, testDebugUnitTest, compileDebugAndroidTestKotlin.
Closes #140.
> **Stacked on #144** — based on `test/fake-provider-scaffolding` because the cursor third needs `RowShape`. Merge #144 first, then this retargets to `main` cleanly. Items 2 and 3 below are independent of it.
### 1. `destinationIsKnownEmpty`'s three short-circuits (`:197`)
No `SIZE` column, no row, a null cell. Each must answer `false`, and none was tested. The KDoc is unambiguous:
> Every other answer … is false, because 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 size 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.** The contrast test is right above them in the file.
### 2. `discardStaged`'s null `parentFile` (`:235`)
A relative single-segment name has no parent. The handle reaches the ViewModel as a path string out of `WorkInfo.outputData` and is turned straight into a `File`, so it is not a shape the caller can rule out.
### 3. `sweepStaging`'s null listing (`:258`)
**Not** the case `the sweep tolerates a staging directory that does not exist yet` covers — `stagingDir`'s own `mkdirs()` recreates a missing directory, which then lists as empty. Only a path that *cannot* be a directory makes `listFiles()` answer null.
### Mutations — five run, three bite
| mutation | result |
|---|---|
| drop `!row.isNull(size)` | **red** — short-circuit test |
| drop `row.moveToFirst()` | **red** — five tests |
| `parentFile!!` instead of `?: return false` | **red** — parentless test |
| `size >= 0` → `size >= -1` | **green — does not bite** |
| parentless file treated as staged | **green — my bad mutation, not a finding** |
**The first green one is a named exemption, recorded in the test.** 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, not a finding.** Substituting `stagingDir` for the null parent reaches `return false` by a different route, so it proves nothing either way. `parentFile!!` is the honest mutation and it goes red.
### Coverage
`:197`, `:235` and `:258` now covered. What remains in this file is exactly what the ticket scoped out: `:173-174` (#142) and `:267` (#143), both seam work.
Local gate green: `ktlintCheck`, `detekt`, `testDebugUnitTest`, `compileDebugAndroidTestKotlin`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Pushed a fix for a real flake this PR surfaced — 9f6f8ab.
the sweep tolerates a staging path that is not a directory failed once on run 33069641674 with java.io.FileNotFoundException at OutputPublisherStagingTest.kt:112, against 468 tests that pass on this machine including under --rerun-tasks. Line 112 was writeBytes immediately after deleteRecursively(), and FileOutputStream answers FileNotFoundException for an existing directory — so something recreated the path inside that window.
That something is LibreMediaConverterApp.onCreate:
sweepStaging reads stagingDir, whose getter is File(cacheDir, "conversions").apply { 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 nothing 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 the 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 should name the cause rather than read as FileNotFoundException at line 112.
Mutation re-run after the change: listFiles() ?: return → listFiles()!! reddens exactly this test, so it still bites.
The wider problem is #159 and is deliberately not fixed here.AppStartSweepTest, JobSnapshotsTest and SpaceArithmeticTest all name the same path; the real answer is an injectable scope, and #159's done-when is that this retry loop can be deleted.
#151's branch has been updated with the same commit so the stack stays linear.
**Pushed a fix for a real flake this PR surfaced** — `9f6f8ab`.
`the sweep tolerates a staging path that is not a directory` failed once on run `33069641674` with `java.io.FileNotFoundException at OutputPublisherStagingTest.kt:112`, against 468 tests that pass on this machine including under `--rerun-tasks`. Line 112 was `writeBytes` immediately after `deleteRecursively()`, and `FileOutputStream` answers `FileNotFoundException` for an existing **directory** — so something recreated the path inside that window.
That something is `LibreMediaConverterApp.onCreate`:
```kotlin
appScope.launch { OutputPublisher(this@LibreMediaConverterApp).sweepStaging() } // Dispatchers.IO
```
`sweepStaging` reads `stagingDir`, whose getter is `File(cacheDir, "conversions").apply { 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 nothing 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 the 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 should name the cause rather than read as `FileNotFoundException at line 112`.
Mutation re-run after the change: `listFiles() ?: return` → `listFiles()!!` reddens exactly this test, so it still bites.
**The wider problem is #159 and is deliberately not fixed here.** `AppStartSweepTest`, `JobSnapshotsTest` and `SpaceArithmeticTest` all name the same path; the real answer is an injectable scope, and #159's done-when is that this retry loop can be deleted.
#151's branch has been updated with the same commit so the stack stays linear.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #140.
1.
destinationIsKnownEmpty's three short-circuits (:197)No
SIZEcolumn, no row, a null cell. Each must answerfalse, and none was tested. The KDoc is unambiguous:The existing tests only ever drove a provider that answers properly, where the size 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. The contrast test is right above them in the file.
2.
discardStaged's nullparentFile(:235)A relative single-segment name has no parent. The handle reaches the ViewModel as a path string out of
WorkInfo.outputDataand is turned straight into aFile, so it is not a shape the caller can rule out.3.
sweepStaging's null listing (:258)Not the case
the sweep tolerates a staging directory that does not exist yetcovers —stagingDir's ownmkdirs()recreates a missing directory, which then lists as empty. Only a path that cannot be a directory makeslistFiles()answer null.Mutations — five run, three bite
!row.isNull(size)row.moveToFirst()parentFile!!instead of?: return falsesize >= 0→size >= -1The first green one is a named exemption, recorded in the test. Measured:
getColumnIndexreturns-1for an absent column, andisNull(-1)throwsCursorIndexOutOfBoundsException— which the surroundingrunCatchingalready 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 anotherCursorimplementation need not throw.The second was my mistake, not a finding. Substituting
stagingDirfor the null parent reachesreturn falseby a different route, so it proves nothing either way.parentFile!!is the honest mutation and it goes red.Coverage
:197,:235and:258now covered. What remains in this file is exactly what the ticket scoped out::173-174(#142) and:267(#143), both seam work.Local gate green:
ktlintCheck,detekt,testDebugUnitTest,compileDebugAndroidTestKotlin.🤖 Generated with Claude Code
Pushed a fix for a real flake this PR surfaced —
9f6f8ab.the sweep tolerates a staging path that is not a directoryfailed once on run33069641674withjava.io.FileNotFoundException at OutputPublisherStagingTest.kt:112, against 468 tests that pass on this machine including under--rerun-tasks. Line 112 waswriteBytesimmediately afterdeleteRecursively(), andFileOutputStreamanswersFileNotFoundExceptionfor an existing directory — so something recreated the path inside that window.That something is
LibreMediaConverterApp.onCreate:sweepStagingreadsstagingDir, whose getter isFile(cacheDir, "conversions").apply { mkdirs() }. Robolectric builds the application for every test that asks for one, so that backgroundmkdirs()is in flight across the whole suite, on a thread the paused main looper does not control and nothing 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 the path back into a directory — which is also why the new assertion that the sweep left a file behind is worth making. Thecheck()matters as much as the loop: the next failure should name the cause rather than read asFileNotFoundException at line 112.Mutation re-run after the change:
listFiles() ?: return→listFiles()!!reddens exactly this test, so it still bites.The wider problem is #159 and is deliberately not fixed here.
AppStartSweepTest,JobSnapshotsTestandSpaceArithmeticTestall name the same path; the real answer is an injectable scope, and #159's done-when is that this retry loop can be deleted.#151's branch has been updated with the same commit so the stack stays linear.