This is not cold code, and that is the interesting part.ConversionViewModel$observe$1$1 already reported 28 covered lines and 24 covered branches before this PR — every test that drives a real worker runs the mapping. What no test did was choose which arm it took.
A real worker reaches a terminal state with well-formed output. So these were the only two arms any test had ever produced:
arm
reached before?
SUCCEEDED with an output path
yes
FAILED with a non-blank message
yes
RUNNING, reading KEY_PROGRESS
no
ENQUEUED, runAttemptCount > 0 → Waiting
no
ENQUEUED, runAttemptCount == 0 → Converting
no
SUCCEEDED with a null output path
no
FAILED with a blank or missing message
no
CANCELLED
no
BLOCKED
no
A grep for WorkInfo.State. across the JVM suite makes that look untrue — all six constants are there. They are in ReattachmentTest, driven into Reattachment.choose, a different function that encodes the same enqueued-means-retry rule. So that rule had a test in one of its two homes, and the copy the user's screen reads had none.
The seam
workManager comes from WorkManager.getInstance in the constructor and observe is private, so nothing could hand this a chosen WorkInfo. The when is now conversionStateFrom, a pure function over a ConversionUpdate carrying only the fields it reads — the same shape as JobSnapshot beside Reattachment.choose, whose KDoc gives the same reason ("the rule stays testable on the JVM").
outputData stays a Data rather than being unpacked into five nullable strings: workDataOf is already the idiom throughout this suite, so unpacking would move the same reads without making anything easier to drive.
Two things stay outside it, deliberately:
The ownership check stays at the call site. Its comment is explicit that it guards the file ownership the SUCCEEDED arm takes, not merely the assignment — moving it inside would change what it protects.
The mapping takes no responsibility for the staged file. It returns the state; the caller reads the file off the result. That is strictly better than the original, where pendingStaged = staged happened inside one arm: "the state and pendingStaged refer to the same file or to no file" is now the shape of the code rather than a rule two branches have to keep.
Mutations
Each kills exactly the test it should:
mutation
red test
progress read ignored
reports the progress it published
runAttemptCount ignored → always Waiting
never run is simply starting
runAttemptCount ignored → never Waiting
already run is waiting to retry
success with no path → empty Converted
named no file is a failure
blank name/type no longer falls back
blank falls back like missing
blank error no longer falls back
blank message falls back
cancellation ignores the caller's landing state
lands wherever the caller said
BLOCKED remapped
blocked looks like one that is starting
ENQUEUED needed two tests and two mutations: either one alone passes against a mapping that ignores runAttemptCount entirely.
Two things the gate caught
The extracted functions needed @UnstableApi — lint's UnsafeOptInUsageError flagged 16 usages. Carried rather than swallowed with @OptIn, per CLAUDE.md.
An early @Suppress("ReturnCount") turned out to be unnecessary. Removed rather than left in place: detekt is clean without it, and CLAUDE.md is explicit that a bare @Suppress is not the answer. The file now carries none.
Next in the wave: #155, the join side, which should come out the same shape as this one.
Closes #154. Second of wave 2 (#153), after #161.
## The claim, stated precisely
**This is not cold code, and that is the interesting part.** `ConversionViewModel$observe$1$1` already reported 28 covered lines and 24 covered branches before this PR — every test that drives a real worker runs the mapping. What no test did was **choose which arm it took**.
A real worker reaches a terminal state with well-formed output. So these were the only two arms any test had ever produced:
| arm | reached before? |
|---|---|
| `SUCCEEDED` with an output path | yes |
| `FAILED` with a non-blank message | yes |
| `RUNNING`, reading `KEY_PROGRESS` | **no** |
| `ENQUEUED`, `runAttemptCount > 0` → `Waiting` | **no** |
| `ENQUEUED`, `runAttemptCount == 0` → `Converting` | **no** |
| `SUCCEEDED` with a null output path | **no** |
| `FAILED` with a blank or missing message | **no** |
| `CANCELLED` | **no** |
| `BLOCKED` | **no** |
A `grep` for `WorkInfo.State.` across the JVM suite makes that look untrue — all six constants are there. **They are in `ReattachmentTest`, driven into `Reattachment.choose`**, a different function that encodes the same enqueued-means-retry rule. So that rule had a test in one of its two homes, and the copy the user's screen reads had none.
## The seam
`workManager` comes from `WorkManager.getInstance` in the constructor and `observe` is private, so nothing could hand this a chosen `WorkInfo`. The `when` is now `conversionStateFrom`, a pure function over a `ConversionUpdate` carrying only the fields it reads — the same shape as `JobSnapshot` beside `Reattachment.choose`, whose KDoc gives the same reason ("the rule stays testable on the JVM").
`outputData` stays a `Data` rather than being unpacked into five nullable strings: `workDataOf` is already the idiom throughout this suite, so unpacking would move the same reads without making anything easier to drive.
**Two things stay outside it, deliberately:**
- **The ownership check stays at the call site.** Its comment is explicit that it guards the file ownership the `SUCCEEDED` arm takes, not merely the assignment — moving it inside would change what it protects.
- **The mapping takes no responsibility for the staged file.** It returns the state; the caller reads the file off the result. That is strictly better than the original, where `pendingStaged = staged` happened *inside* one arm: "the state and `pendingStaged` refer to the same file or to no file" is now the shape of the code rather than a rule two branches have to keep.
## Mutations
Each kills exactly the test it should:
| mutation | red test |
|---|---|
| progress read ignored | `reports the progress it published` |
| `runAttemptCount` ignored → always `Waiting` | `never run is simply starting` |
| `runAttemptCount` ignored → never `Waiting` | `already run is waiting to retry` |
| success with no path → empty `Converted` | `named no file is a failure` |
| blank name/type no longer falls back | `blank falls back like missing` |
| blank error no longer falls back | `blank message falls back` |
| cancellation ignores the caller's landing state | `lands wherever the caller said` |
| `BLOCKED` remapped | `blocked looks like one that is starting` |
`ENQUEUED` needed **two** tests and two mutations: either one alone passes against a mapping that ignores `runAttemptCount` entirely.
## Two things the gate caught
- The extracted functions needed `@UnstableApi` — lint's `UnsafeOptInUsageError` flagged 16 usages. Carried rather than swallowed with `@OptIn`, per `CLAUDE.md`.
- An early `@Suppress("ReturnCount")` turned out to be unnecessary. Removed rather than left in place: detekt is clean without it, and `CLAUDE.md` is explicit that a bare `@Suppress` is not the answer. The file now carries none.
**502 → 516 tests, 87.1% → 87.7% line, 69.1% → 70.4% branch.** Gate green: `assembleDebug`, `testDebugUnitTest`, `compileDebugAndroidTestKotlin`, `ktlintCheck`, `detekt`, `lintDebug`.
Next in the wave: **#155**, the join side, which should come out the same shape as this one.
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 #154. Second of wave 2 (#153), after #161.
The claim, stated precisely
This is not cold code, and that is the interesting part.
ConversionViewModel$observe$1$1already reported 28 covered lines and 24 covered branches before this PR — every test that drives a real worker runs the mapping. What no test did was choose which arm it took.A real worker reaches a terminal state with well-formed output. So these were the only two arms any test had ever produced:
SUCCEEDEDwith an output pathFAILEDwith a non-blank messageRUNNING, readingKEY_PROGRESSENQUEUED,runAttemptCount > 0→WaitingENQUEUED,runAttemptCount == 0→ConvertingSUCCEEDEDwith a null output pathFAILEDwith a blank or missing messageCANCELLEDBLOCKEDA
grepforWorkInfo.State.across the JVM suite makes that look untrue — all six constants are there. They are inReattachmentTest, driven intoReattachment.choose, a different function that encodes the same enqueued-means-retry rule. So that rule had a test in one of its two homes, and the copy the user's screen reads had none.The seam
workManagercomes fromWorkManager.getInstancein the constructor andobserveis private, so nothing could hand this a chosenWorkInfo. Thewhenis nowconversionStateFrom, a pure function over aConversionUpdatecarrying only the fields it reads — the same shape asJobSnapshotbesideReattachment.choose, whose KDoc gives the same reason ("the rule stays testable on the JVM").outputDatastays aDatarather than being unpacked into five nullable strings:workDataOfis already the idiom throughout this suite, so unpacking would move the same reads without making anything easier to drive.Two things stay outside it, deliberately:
SUCCEEDEDarm takes, not merely the assignment — moving it inside would change what it protects.pendingStaged = stagedhappened inside one arm: "the state andpendingStagedrefer to the same file or to no file" is now the shape of the code rather than a rule two branches have to keep.Mutations
Each kills exactly the test it should:
reports the progress it publishedrunAttemptCountignored → alwaysWaitingnever run is simply startingrunAttemptCountignored → neverWaitingalready run is waiting to retryConvertednamed no file is a failureblank falls back like missingblank message falls backlands wherever the caller saidBLOCKEDremappedblocked looks like one that is startingENQUEUEDneeded two tests and two mutations: either one alone passes against a mapping that ignoresrunAttemptCountentirely.Two things the gate caught
@UnstableApi— lint'sUnsafeOptInUsageErrorflagged 16 usages. Carried rather than swallowed with@OptIn, perCLAUDE.md.@Suppress("ReturnCount")turned out to be unnecessary. Removed rather than left in place: detekt is clean without it, andCLAUDE.mdis explicit that a bare@Suppressis not the answer. The file now carries none.502 → 516 tests, 87.1% → 87.7% line, 69.1% → 70.4% branch. Gate green:
assembleDebug,testDebugUnitTest,compileDebugAndroidTestKotlin,ktlintCheck,detekt,lintDebug.Next in the wave: #155, the join side, which should come out the same shape as this one.