From c4bb7d4d2da85e25ceeed42367d1ed6b3040ec58 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Tue, 25 Aug 2026 22:38:49 -0500 Subject: [PATCH] Quote the rate the ticket settled on, and point the save gap at its ticket Two accuracy fixes to notes the earlier commits left behind. The test KDocs carried "roughly 1-in-130" and a 400-leg-attempt denominator. Both come from earlier comments on #49 that its own census later replaced -- that ticket has three recorded corrections to its rate claims, and a superseded figure in a permanent comment is the exact thing its author kept having to fix. What survives the corrections is the count and the spread: four occurrences, API 33, 35 and 36, every one on attempt 1 and green on re-run. The save exemption described a real defect with nowhere to look it up. It is #123 now, so the KDoc names a number instead of trailing off. Co-Authored-By: Claude Opus 5 (1M context) --- .../convert/ConversionViewModel.kt | 4 ++-- .../org/libremediaconverter/join/JoinViewModel.kt | 4 ++-- .../libremediaconverter/convert/PickOwnershipTest.kt | 8 ++++---- .../convert/ReattachmentOwnershipTest.kt | 11 ++++++----- .../join/JoinReattachmentOwnershipTest.kt | 6 +++--- 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt b/app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt index 212eaae..b8344fe 100644 --- a/app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt +++ b/app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt @@ -207,8 +207,8 @@ class ConversionViewModel @JvmOverloads constructor( * land on top of a [reset] taken while its copy was in flight, putting `Saved` on a screen the * user has just cleared. Guarding it would drop that write instead, reporting nothing for a * file that may genuinely have reached the user's destination. Which of those two is right is - * a question about what the screen should offer during a save, not about this race, and it is - * left open rather than answered in passing. + * a question about what the screen should offer during a save, not about this race, so it is + * filed as issue #123 rather than decided here in passing. */ private val ownership = ScreenOwnership() diff --git a/app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt b/app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt index a679b30..3d0cd2a 100644 --- a/app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt +++ b/app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt @@ -109,8 +109,8 @@ class JoinViewModel @JvmOverloads constructor( * lands after a suspension point is guarded: the one in [onInputsPicked] and the one in * [observe]. [save] is the one left out, deliberately and with the same limit its counterpart * in `ConversionViewModel` spells out: nothing can overwrite what it writes, but it can still - * land on top of a [reset] taken while its copy was in flight, and which way that should go is - * a question about the save screen rather than about this race. + * land on top of a [reset] taken while its copy was in flight. Which way that should go is a + * question about the save screen rather than about this race -- issue #123. */ private val ownership = ScreenOwnership() diff --git a/app/src/test/java/org/libremediaconverter/convert/PickOwnershipTest.kt b/app/src/test/java/org/libremediaconverter/convert/PickOwnershipTest.kt index 3d9a8fb..bd910c3 100644 --- a/app/src/test/java/org/libremediaconverter/convert/PickOwnershipTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/PickOwnershipTest.kt @@ -27,10 +27,10 @@ import java.io.File * second, which is the same defect the ticket reported against reattachment with a different * coroutine on the losing side. * - * Both cases below existed before the fix and neither was reported, because a pick that loses to - * another pick still shows *a* file the user chose. That is what made this worth closing in the - * same change: one rule covering every deferred write is checkable, where "the observer checks and - * the pick does not" is a rule nobody can hold in their head. + * Both cases below were measured rather than assumed: deleting either guard turns the matching + * test red, and deleting the probe one turns nine other tests red with it. Neither was ever + * reported, because a pick that loses to another pick still shows *a* file the user chose -- which + * is what made it worth closing alongside #49 rather than leaving as a second thing to find. */ @UnstableApi @RunWith(RobolectricTestRunner::class) diff --git a/app/src/test/java/org/libremediaconverter/convert/ReattachmentOwnershipTest.kt b/app/src/test/java/org/libremediaconverter/convert/ReattachmentOwnershipTest.kt index 1da0510..b118cff 100644 --- a/app/src/test/java/org/libremediaconverter/convert/ReattachmentOwnershipTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/ReattachmentOwnershipTest.kt @@ -21,10 +21,11 @@ import java.io.File * Issue #49, on the JVM and without the race. * * `ReattachOnLaunchTest.doesNotOverwriteAPickTheUserHasAlreadyMade` has been catching this on - * devices since 2026-08-24 — four times in 400 gating leg-attempts, on API 33, 35 and 36 — and - * every occurrence passed on re-run, which is why it was read as flaky infrastructure for two - * days. It is not. The assertion it fails on is `expected null, but was:`: a finished - * job from an earlier session taking a screen the user had already picked a file on. + * devices since 2026-08-24 — four occurrences, spread across API 33, 35 and 36, which is what + * ruled out an emulator-image quirk. Every one was on attempt 1 and every one passed on re-run, + * which is why it was read as flaky infrastructure for two days. It is not. The assertion it fails + * on is `expected null, but was:`: a finished job from an earlier session taking a + * screen the user had already picked a file on. * * The defect is a check-then-act whose act is deferred into another coroutine. `reattach()` reads * `_state.value` and then calls `observe()`, which *launches* a collector that has to suspend on @@ -92,7 +93,7 @@ class ReattachmentOwnershipTest { * as a message posted to the main looper — and that looper is paused, so it cannot run until * something pumps it. `onInputPicked` is an ordinary synchronous call from this thread. The * pick is therefore *always* issued before the guard runs; none of it is left to timing, which - * is the whole point of writing this here rather than relying on the 1-in-130 device sighting. + * is the whole point of writing it here rather than relying on the rare device sighting. */ @Test fun `a conversion found while the user was picking never reaches the screen`() { diff --git a/app/src/test/java/org/libremediaconverter/join/JoinReattachmentOwnershipTest.kt b/app/src/test/java/org/libremediaconverter/join/JoinReattachmentOwnershipTest.kt index ca05cb1..b47ec3e 100644 --- a/app/src/test/java/org/libremediaconverter/join/JoinReattachmentOwnershipTest.kt +++ b/app/src/test/java/org/libremediaconverter/join/JoinReattachmentOwnershipTest.kt @@ -41,9 +41,9 @@ import java.io.File * assignment below, so nothing can interleave". There is no assignment below, and the two lines * are in different coroutines. * - * The convert side has been failing this on CI for two days at roughly 1-in-130 and was read as - * flaky infrastructure. `JoinViewModel` has the identical shape and no test at all, which is why - * this one was written before the fix rather than after it. + * The convert side has been failing this on CI for two days — four occurrences across three API + * levels, each read as flaky infrastructure. `JoinViewModel` has the identical shape and no test + * at all, which is why this one was written before the fix rather than after it. */ @UnstableApi @RunWith(RobolectricTestRunner::class)