C5: the two jobs ConversionWorker refuses before converting #148

Merged
JMR-dev merged 4 commits from test/refused-jobs into test/concatworker-failure-arms 2026-08-27 13:56:36 +00:00
JMR-dev commented 2026-08-27 03:37:25 +00:00 (Migrated from github.com)

Closes #139.

Two cold exits in ConversionWorker.doWork, both reachable for the same underlying reason: a job does not have to come from the picker. WorkManager keeps queued and finished work for about a week, so a downgrade or a rollback hands this build a job enqueued by another one — the premise WorkerEnumFallbackTest and JobTags are both written on — and ConversionWorker.request(...) is callable directly.

:62 — a missing KEY_INPUT_URI was untested everywhere, JVM and device. The nearest e2e test, ForcedFailureTest.aMissingInputFailsRatherThanCrashing, passes a URI pointing at a file that does not exist, which reaches the engine and fails much later with a different message.

:124-126 — the Validation.Invalid refusal had no test at all, though its comment names both arrival paths it exists for.

Why a new file

Both are about the message. A refusal that fails with empty output Data renders the UI's generic "Conversion failed." with nothing else to say — the defect shape DeniedForegroundStartTest records from the device pass. Asserting the verdict alone would pass against exactly that, so Failure.equals comparing output data is doing real work here.

Two of the five tests exist to stop the others passing wrongly

  • a refused spec never reaches an engine — says it failed before converting rather than during. Without it, a worker that ran the job and then reported the validation message would pass. It would also have spent the user's battery on a file it was going to refuse.
  • a valid spec is not refused — the control. Every other assertion here is about a refusal, so without this they would all still pass against a worker that refused everything.

The invalid fixture asks ContainerCapabilities for the expected message rather than hard-coding it, and asserts up front that the fixture really is invalid — so this cannot quietly become a test about a spec that has since become valid.

Mutations — three run, three red

mutation reddens
change the no-input message no-input message test
drop the validation refusal entirely both refusal tests
validate but keep converting anyway both refusal tests

Coverage

L61-62 and L123-126 are now fully covered, branches included (mb=0 throughout).

Local gate green: ktlintCheck, detekt, testDebugUnitTest, compileDebugAndroidTestKotlin.

🤖 Generated with Claude Code

Closes #139. Two cold exits in `ConversionWorker.doWork`, both reachable for the same underlying reason: **a job does not have to come from the picker.** WorkManager keeps queued and finished work for about a week, so a downgrade or a rollback hands this build a job enqueued by another one — the premise `WorkerEnumFallbackTest` and `JobTags` are both written on — and `ConversionWorker.request(...)` is callable directly. **`:62` — a missing `KEY_INPUT_URI`** was untested everywhere, JVM *and* device. The nearest e2e test, `ForcedFailureTest.aMissingInputFailsRatherThanCrashing`, passes a URI pointing at a file that does not exist, which reaches the engine and fails much later with a different message. **`:124-126` — the `Validation.Invalid` refusal** had no test at all, though its comment names both arrival paths it exists for. ### Why a new file Both are about the **message**. A refusal that fails with empty output `Data` renders the UI's generic "Conversion failed." with nothing else to say — the defect shape `DeniedForegroundStartTest` records from the device pass. Asserting the verdict alone would pass against exactly that, so `Failure.equals` comparing output data is doing real work here. ### Two of the five tests exist to stop the others passing wrongly - `a refused spec never reaches an engine` — says it failed *before* converting rather than during. Without it, a worker that ran the job and then reported the validation message would pass. It would also have spent the user's battery on a file it was going to refuse. - `a valid spec is not refused` — the control. Every other assertion here is about a refusal, so without this they would all still pass against a worker that refused everything. The invalid fixture asks `ContainerCapabilities` for the expected message rather than hard-coding it, and asserts up front that the fixture really is invalid — so this cannot quietly become a test about a spec that has since become valid. ### Mutations — three run, three red | mutation | reddens | |---|---| | change the no-input message | no-input message test | | drop the validation refusal entirely | both refusal tests | | validate but keep converting anyway | both refusal tests | ### Coverage `L61-62` and `L123-126` are now fully covered, branches included (`mb=0` throughout). Local gate green: `ktlintCheck`, `detekt`, `testDebugUnitTest`, `compileDebugAndroidTestKotlin`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
JMR-dev commented 2026-08-27 04:16:48 +00:00 (Migrated from github.com)

Pushed one addition beyond C5's original scope, found by the residual-gap audit over the merged batch (full mapping on #132).

ConcatWorker.kt:42 — the refusal of a join with fewer than two inputs — was covered by nothing in either source set. Its neighbour four lines up, "No input files.", is covered, on the device by UnopenableUriTest. JaCoCo shows both cold and cannot distinguish them, because it does not see androidTest; only grepping the androidTest source separates the e2e-covered arm from the untested one.

It belongs here rather than in a ninth PR because this file's header already states the charter and the reachability argument. request(...) adds one of its own: it takes a List<Uri> and checks nothing about its length, so a one-item join is a well-formed call rather than a corrupted queue entry.

Both mutations bite, each killing exactly the test it should — the guard deleted reddens the single-file test, < 2 → < 3 reddens the two-file control. Gate green locally.

Pushed one addition beyond C5's original scope, found by the residual-gap audit over the merged batch (full mapping on #132). `ConcatWorker.kt:42` — the refusal of a join with fewer than two inputs — was covered by nothing in either source set. Its neighbour four lines up, `"No input files."`, *is* covered, on the device by `UnopenableUriTest`. JaCoCo shows both cold and cannot distinguish them, because it does not see androidTest; only grepping the androidTest source separates the e2e-covered arm from the untested one. It belongs here rather than in a ninth PR because this file's header already states the charter and the reachability argument. `request(...)` adds one of its own: it takes a `List<Uri>` and checks nothing about its length, so a one-item join is a well-formed call rather than a corrupted queue entry. Both mutations bite, each killing exactly the test it should — the guard deleted reddens the single-file test, `< 2` → `< 3` reddens the two-file control. Gate green locally.
Sign in to join this conversation.