JMR-dev/LibreMediaConverter
1
0
Fork 0
Code Issues 16 Pull Requests Actions Packages Projects Releases Wiki Activity

Three failure messages the app can show have never been produced by anything #193

New Issue
Closed
opened 2026-09-02 12:46:07 +00:00 by JMR-dev · 0 comments
JMR-dev commented 2026-09-02 12:46:07 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Wave 4, filed from a coverage read on main @ 54ca2dd, 2026-09-02. The shared filter note is on #194.

Three sentences a user can be shown, and none of them has ever been produced

app/src/main/java/org/libremediaconverter/work/ConversionWorker.kt:316
app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt:631
app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt:416

Result.failure(workDataOf(KEY_ERROR to (cause.message ?: GENERIC_FAILURE_MESSAGE)))   // :316
_state.value = ConversionState.Failed(e.message ?: SAVE_FAILED_MESSAGE, pending)      // :631
_state.value = JoinState.Failed(e.message ?: SAVE_FAILED_MESSAGE, pending)            // :416

All three lines report ci == 0 — never executed. Every existing test throws with a message, so the elvis right side has never been taken anywhere in the suite. A Throwable with a null message is not exotic: RuntimeException(), IOException() and most platform exceptions raised without an argument all have one.

:316 has a second line of defence downstream, and it will eat a careless test

This is the part worth reading before writing anything.

ConversionStateMappingTest's "a failure with nothing said still says something" looks like it already covers :316 and does not. It drives the read side — map(FAILED, data = Data.EMPTY) — and asserts the same constant that :316 writes. And the read side has its own fallback (ConversionViewModel.kt:147-149):

update.outputData.getString(ConversionWorker.KEY_ERROR)
    ?.takeIf { it.isNotBlank() }
    ?: ConversionWorker.GENERIC_FAILURE_MESSAGE

So mutating :316 to .orEmpty() writes KEY_ERROR to "", and the ViewModel turns that straight back into GENERIC_FAILURE_MESSAGE. A test that asserts on the resulting Failed state stays green under the mutation — the two fallbacks mask each other, which is why the write-side one has survived three waves in the first place.

Assert on the worker's own Result.failure output Data, not on the state the ViewModel derives from it. RefusedJobTest and ForcedFailureTest both read KEY_ERROR off the result directly; follow those.

The work

Three small tests, each belonging where the surrounding behaviour already lives rather than in a new "null message" class:

  • :316 — a software transcoder that throws a bare RuntimeException(); assert result.outputData.getString(KEY_ERROR) == GENERIC_FAILURE_MESSAGE. ConversionDependencies.software is the seam; ForcedFailureTest's fakes are the model.
  • :631 / :416 — a publisher that throws a message-less exception from publish; assert the Failed state's text is SAVE_FAILED_MESSAGE and that pending still travels on the state, because the retry offer depends on it. FailedSaveRetryTest's RecordingPublisher is the model, and that file already owns "what each state carries". No downstream fallback exists on this path — these two set _state.value directly — so the state assertion is the right one here.

Acceptance: the mutation that must go red

Replace the constant with .orEmpty() at each site. Restore, confirm green.

?: "something else" is not an acceptable mutation: it proves only that the test reads a constant, which is the vacuous shape wave 3 rejected twice.

For :316, confirm the mutation reddens the worker assertion. If it does not, the test is reading the ViewModel and needs moving — see above.

_Wave 4, filed from a coverage read on `main` @ `54ca2dd`, 2026-09-02. The shared filter note is on #194._ ## Three sentences a user can be shown, and none of them has ever been produced ``` app/src/main/java/org/libremediaconverter/work/ConversionWorker.kt:316 app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt:631 app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt:416 ``` ```kotlin Result.failure(workDataOf(KEY_ERROR to (cause.message ?: GENERIC_FAILURE_MESSAGE))) // :316 _state.value = ConversionState.Failed(e.message ?: SAVE_FAILED_MESSAGE, pending) // :631 _state.value = JoinState.Failed(e.message ?: SAVE_FAILED_MESSAGE, pending) // :416 ``` All three lines report `ci == 0` — never executed. Every existing test throws *with* a message, so the elvis right side has never been taken anywhere in the suite. A `Throwable` with a null message is not exotic: `RuntimeException()`, `IOException()` and most platform exceptions raised without an argument all have one. ## `:316` has a second line of defence downstream, and it will eat a careless test This is the part worth reading before writing anything. `ConversionStateMappingTest`'s *"a failure with nothing said still says something"* looks like it already covers `:316` and does not. It drives the **read** side — `map(FAILED, data = Data.EMPTY)` — and asserts the same constant that `:316` writes. And the read side has its own fallback (`ConversionViewModel.kt:147-149`): ```kotlin update.outputData.getString(ConversionWorker.KEY_ERROR) ?.takeIf { it.isNotBlank() } ?: ConversionWorker.GENERIC_FAILURE_MESSAGE ``` So mutating `:316` to `.orEmpty()` writes `KEY_ERROR to ""`, and the ViewModel turns that straight back into `GENERIC_FAILURE_MESSAGE`. **A test that asserts on the resulting `Failed` state stays green under the mutation** — the two fallbacks mask each other, which is why the write-side one has survived three waves in the first place. **Assert on the worker's own `Result.failure` output `Data`, not on the state the ViewModel derives from it.** `RefusedJobTest` and `ForcedFailureTest` both read `KEY_ERROR` off the result directly; follow those. ## The work Three small tests, each belonging where the surrounding behaviour already lives rather than in a new "null message" class: - **`:316`** — a software transcoder that throws a bare `RuntimeException()`; assert `result.outputData.getString(KEY_ERROR) == GENERIC_FAILURE_MESSAGE`. `ConversionDependencies.software` is the seam; `ForcedFailureTest`'s fakes are the model. - **`:631` / `:416`** — a publisher that throws a message-less exception from `publish`; assert the `Failed` state's text is `SAVE_FAILED_MESSAGE` **and** that `pending` still travels on the state, because the retry offer depends on it. `FailedSaveRetryTest`'s `RecordingPublisher` is the model, and that file already owns "what each state carries". No downstream fallback exists on this path — these two set `_state.value` directly — so the state assertion is the right one here. ## Acceptance: the mutation that must go red Replace the constant with `.orEmpty()` at each site. Restore, confirm green. `?: "something else"` is **not** an acceptable mutation: it proves only that the test reads a constant, which is the vacuous shape wave 3 rejected twice. For `:316`, confirm the mutation reddens the *worker* assertion. If it does not, the test is reading the ViewModel and needs moving — see above.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
above-cut
Worked autonomously overnight: local, JVM-verifiable, no product decision
 accessibility
Barrier affecting people with disabilities
 backlog
Held for manual review: product/UX call, CI/workflow, hardware, or unverifiable here
 bug
Something isn't working
 confirmed
Reviewer demonstrated the defect
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 plausible
Reviewer could not fully demonstrate it; treat as unproven
 question
Further information is requested
 sev:high
High severity
 sev:low
Low severity
 sev:medium
Medium severity
 wontfix
This will not be worked on
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
JMR-dev
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#193