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

C4 — ConcatWorker's cancellation and foreground-denied arms have no test, though ConversionWorker's twin does #138

New Issue
Closed
opened 2026-08-27 03:06:46 +00:00 by JMR-dev · 1 comment
JMR-dev commented 2026-08-27 03:06:46 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Child 4 of 7 decomposing #132 — item 4 there. Independent. Scope note: getForegroundInfo is deliberately excluded — see the bottom.

Why this exists

app/src/main/java/org/libremediaconverter/work/ConcatWorker.kt:92-113

ConversionWorker has WorkerCancellationTest and DeniedForegroundStartTest. Its twin has
neither, and two of its three failure exits are cold:

} catch (e: CancellationException) {
    staged.delete()      // :95  — never executed
    throw e              // :96  — never executed
} catch (e: Throwable) {
    staged.delete()
    when (FailureOutcome.forFailure(stopReason, e, runAttemptCount)) {
        FailureOutcome.RETRY -> ...              // covered
        FailureOutcome.FOREGROUND_DENIED -> ...  // :105-106 — never executed
        FailureOutcome.FAIL -> ...               // covered

The cancellation arm is the rule defect-audit.md D10 exists for: a CancellationException
caught and converted to a Result tells WorkManager the work finished, when it was interrupted. The
comment at :93-94 points at ConversionWorker for the reasoning and the delete; nothing checks
that ConcatWorker kept it.

FOREGROUND_DENIED is the only one of the three FailureOutcome arms with no test on this worker,
and it is the one that produces a message the user must act on.

Scope

Two tests, modelled directly on the ConversionWorker originals — WorkerCancellationTest for the
first, DeniedForegroundStartTest's runAttemptCount setup for the second. WorkerStubs.kt's
NamingPublisher already records what a join staged, which is how the existing enum-fallback test
asserts against a worker whose ConcatEngine call cannot succeed on the JVM.

Done means

Cancellation: the staged file is gone and the exception propagates rather than becoming a
Result. Foreground denial: Result.failure carrying FailureOutcome.FOREGROUND_DENIED_MESSAGE,
and the staged file gone.

Mutation: change the cancellation arm to return Result.failure(...) instead of rethrowing —
the test must go red. Separately, delete staged.delete() from that arm and the same test must go
red on the leftover file.

Not in scope

getForegroundInfo (:133-137). #88 investigated both workers' copies and closed on the finding
that there is no seam worth cutting — a constant title, a constant 0, a constant flag, five lines
of construction with no branch. It is a named exemption, not a gap, and re-covering it here would
undo a decision that was made deliberately and written down.

_Child 4 of 7 decomposing #132 — item 4 there. Independent. **Scope note: `getForegroundInfo` is deliberately excluded — see the bottom.**_ ### Why this exists ``` app/src/main/java/org/libremediaconverter/work/ConcatWorker.kt:92-113 ``` `ConversionWorker` has `WorkerCancellationTest` and `DeniedForegroundStartTest`. **Its twin has neither**, and two of its three failure exits are cold: ```kotlin } catch (e: CancellationException) { staged.delete() // :95 — never executed throw e // :96 — never executed } catch (e: Throwable) { staged.delete() when (FailureOutcome.forFailure(stopReason, e, runAttemptCount)) { FailureOutcome.RETRY -> ... // covered FailureOutcome.FOREGROUND_DENIED -> ... // :105-106 — never executed FailureOutcome.FAIL -> ... // covered ``` The cancellation arm is the rule `defect-audit.md` **D10** exists for: a `CancellationException` caught and converted to a `Result` tells WorkManager the work finished, when it was interrupted. The comment at `:93-94` points at `ConversionWorker` for the reasoning and the delete; nothing checks that `ConcatWorker` kept it. `FOREGROUND_DENIED` is the only one of the three `FailureOutcome` arms with no test on this worker, and it is the one that produces a message the user must act on. ### Scope Two tests, modelled directly on the `ConversionWorker` originals — `WorkerCancellationTest` for the first, `DeniedForegroundStartTest`'s `runAttemptCount` setup for the second. `WorkerStubs.kt`'s `NamingPublisher` already records what a join staged, which is how the existing enum-fallback test asserts against a worker whose `ConcatEngine` call cannot succeed on the JVM. ### Done means Cancellation: the staged file is gone **and** the exception propagates rather than becoming a `Result`. Foreground denial: `Result.failure` carrying `FailureOutcome.FOREGROUND_DENIED_MESSAGE`, and the staged file gone. **Mutation:** change the cancellation arm to `return Result.failure(...)` instead of rethrowing — the test must go red. Separately, delete `staged.delete()` from that arm and the same test must go red on the leftover file. ### Not in scope `getForegroundInfo` (`:133-137`). **#88 investigated both workers' copies and closed on the finding that there is no seam worth cutting** — a constant title, a constant 0, a constant flag, five lines of construction with no branch. It is a named exemption, not a gap, and re-covering it here would undo a decision that was made deliberately and written down.
JMR-dev commented 2026-09-02 01:40:34 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Already done, and verified on main today — closing.

This stayed open through a bookkeeping failure, not an unfinished one. PR #147 carried Closes #138, but GitHub only fires a closing keyword when the PR merges into the default branch. #147 merged into its stack base instead (the async-retarget race written up on #160 and now in CLAUDE.md), so the keyword never ran. The content reached main later via #160, which did not carry the keywords.

Verified against main at d354f64 just now, by re-running this ticket's own named mutation rather than by checking the files exist:

ConcatWorker cancellation arm: throw e -> Result.failure()
  red: a cancelled join propagates instead of being turned into a Result

Gate green on main: 546 tests in 76 classes, 0 failures.

**Already done, and verified on `main` today — closing.** This stayed open through a bookkeeping failure, not an unfinished one. PR #147 carried `Closes #138`, but **GitHub only fires a closing keyword when the PR merges into the default branch.** #147 merged into its stack base instead (the async-retarget race written up on #160 and now in `CLAUDE.md`), so the keyword never ran. The content reached `main` later via #160, which did not carry the keywords. Verified against `main` at `d354f64` just now, by re-running **this ticket's own named mutation** rather than by checking the files exist: ``` ConcatWorker cancellation arm: throw e -> Result.failure() red: a cancelled join propagates instead of being turned into a Result ``` Gate green on `main`: 546 tests in 76 classes, 0 failures.
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 enhancement sev:low
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#138