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

Cancel has only ever been tested on the side where it does nothing #192

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

Wave 4, filed from a coverage read on main @ 54ca2dd, 2026-09-02. The other eleven are #193-#203 of the same read; the shared filter note is on #194.

Cancel is a primary affordance and nothing proves it reaches WorkManager

app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt:581-583
app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt:374-376

Both are one line of body:

fun cancel() {
    activeWorkId?.let(workManager::cancelWorkById)
}

The only test of either is the null side. SettingsEditsTest.kt:190:

@Test
fun `cancelling with no active job does nothing rather than throwing`() {
    // `activeWorkId?.let(...)` -- the null side. A user can reach Cancel through a state that
    // has already finished, and taking the app down for it would be worse than doing nothing.
    viewModel.cancel()
    assertEquals(ConversionState.Idle, viewModel.state.value)
}

Its own comment names it. JoinViewModel.cancel() has no test at all.

Why the affordance tests do not cover this

JoinStateAffordancesTest and ConverterStateAffordancesTest click TestTags.CANCEL and assert the action fires — into a stub that records "cancel". ScreenWiringTest drives converterActions/joinActions and asserts the action calls viewModel.cancel(). Both halves are pinned and the join between them is not: nothing in 584 tests connects cancel() to WorkManager.

Why this is a warm method with a cold arm, not an uncovered line

JaCoCo reports 0 missed lines on both methods and mi=11, ci=7, mb=1, cb=1 at method level. A ci == 0 filter cannot see it. This is the case that motivated the second filter in the wave's filter note.

The fixture, which is the actual work

The synchronous test WorkManager finishes a job inline, which is why only the null half is covered — by the time cancel() could be called, activeWorkId's job is already terminal.

Use the reattach path instead: enqueue a request with setInitialDelay, which TestScheduler holds until setInitialDelayMet. JobSnapshots.jobSnapshots() (work/JobSnapshots.kt:25) is a tag query with no state filter, so it returns the job as ENQUEUED at attempt 0; Reattachment.choose ranks it QUEUED; observe maps it to Converting(input, 0). The ViewModel now holds a live activeWorkId.

Do not reach for a retrying worker to hold a job open. TestScheduler does not model backoff, and SynchronousExecutor risks a re-run loop.

Acceptance: the mutation that must go red

Make cancel() a no-op in each ViewModel. getWorkInfoById(id).state must stay ENQUEUED instead of becoming CANCELLED. Restore, confirm green.

Rides along

JoinViewModel.kt:269's activeWorkId != null arm — a reattachment landing over a job the user already started — is reachable from the same fixture and belongs in the same PR.

Not the acceptance

The coverage number. Both methods already report every line covered.

_Wave 4, filed from a coverage read on `main` @ `54ca2dd`, 2026-09-02. The other eleven are #193-#203 of the same read; the shared filter note is on #194._ ## Cancel is a primary affordance and nothing proves it reaches WorkManager ``` app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt:581-583 app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt:374-376 ``` Both are one line of body: ```kotlin fun cancel() { activeWorkId?.let(workManager::cancelWorkById) } ``` **The only test of either is the null side.** `SettingsEditsTest.kt:190`: ```kotlin @Test fun `cancelling with no active job does nothing rather than throwing`() { // `activeWorkId?.let(...)` -- the null side. A user can reach Cancel through a state that // has already finished, and taking the app down for it would be worse than doing nothing. viewModel.cancel() assertEquals(ConversionState.Idle, viewModel.state.value) } ``` Its own comment names it. `JoinViewModel.cancel()` has **no test at all**. ## Why the affordance tests do not cover this `JoinStateAffordancesTest` and `ConverterStateAffordancesTest` click `TestTags.CANCEL` and assert the *action* fires — into a stub that records `"cancel"`. `ScreenWiringTest` drives `converterActions`/`joinActions` and asserts the action calls `viewModel.cancel()`. Both halves are pinned and the join between them is not: nothing in 584 tests connects `cancel()` to WorkManager. ## Why this is a warm method with a cold arm, not an uncovered line JaCoCo reports **0 missed lines** on both methods and `mi=11, ci=7, mb=1, cb=1` at method level. A `ci == 0` filter cannot see it. This is the case that motivated the second filter in the wave's filter note. ## The fixture, which is the actual work The synchronous test WorkManager finishes a job inline, which is *why* only the null half is covered — by the time `cancel()` could be called, `activeWorkId`'s job is already terminal. Use the reattach path instead: enqueue a request with `setInitialDelay`, which `TestScheduler` holds until `setInitialDelayMet`. `JobSnapshots.jobSnapshots()` (`work/JobSnapshots.kt:25`) is a tag query with no state filter, so it returns the job as ENQUEUED at attempt 0; `Reattachment.choose` ranks it QUEUED; `observe` maps it to `Converting(input, 0)`. The ViewModel now holds a live `activeWorkId`. **Do not** reach for a retrying worker to hold a job open. `TestScheduler` does not model backoff, and `SynchronousExecutor` risks a re-run loop. ## Acceptance: the mutation that must go red Make `cancel()` a no-op in each ViewModel. `getWorkInfoById(id).state` must stay ENQUEUED instead of becoming CANCELLED. Restore, confirm green. ## Rides along `JoinViewModel.kt:269`'s `activeWorkId != null` arm — a reattachment landing over a job the user already started — is reachable from the same fixture and belongs in the same PR. ## Not the acceptance The coverage number. Both methods already report every line covered.
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#192