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

W2: the join side's state mapping, and the arity guard that duplicates ConcatWorker's message #155

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

The join side of W1, plus one guard above it that is more interesting than its size suggests.

The state machine

Same shape as W1 and the same reasoning, on a smaller matrix. JoinViewModel.observe maps WorkInfo onto JoinState, and these arms are never chosen:

arm line(s)
RUNNING / BLOCKED → Joining 254
ENQUEUED, runAttemptCount > 0 → Waiting 256–257
ENQUEUED, runAttemptCount == 0 → Joining 259
FAILED with a blank or missing message 298
CANCELLED 301

cancel() (307–309) has no coverage either.

Do this after W1 so the two seams come out the same shape. They are the same refactor twice and should not diverge; if the join side needs a different split, that is worth a comment on both tickets rather than a quiet divergence.

The arity guard, and why it is not just three cold lines

onInputsPicked refuses fewer than two inputs at 196–198:

if (uris.size < 2) {
    _state.value = JoinState.Failed("Pick at least two files to join.")
    return
}

That string is emitted in two files. ConcatWorker.kt:42 emits the identical literal, and it was pinned by a test on #148 during the residual-gap audit. This copy is pinned by nothing — so today, changing the ViewModel's wording breaks no test while changing the worker's does, for one message the user sees from one condition.

That is W5's subject. Land W5 first, then pin whatever survives here. Writing a test against this literal now would freeze the duplication in place, which is the opposite of useful.

Worth noting while you are in the file: JoinScreen.kt:45 filters uris.isNotEmpty() before calling this, so the picker cannot deliver zero — but it can deliver one, which is exactly the case this guard is for. Do not conclude from the screen that the guard is unreachable.

Done when

Every arm above is chosen by a test and killed by a mutation, cancel() is covered, and the arity guard is pinned against a shared constant rather than a literal. As on W1, ENQUEUED needs both sides or it proves nothing about runAttemptCount.

The join side of W1, plus one guard above it that is more interesting than its size suggests. ## The state machine Same shape as W1 and the same reasoning, on a smaller matrix. `JoinViewModel.observe` maps `WorkInfo` onto `JoinState`, and these arms are never chosen: | arm | line(s) | |---|---| | `RUNNING` / `BLOCKED` → `Joining` | 254 | | `ENQUEUED`, `runAttemptCount > 0` → `Waiting` | 256–257 | | `ENQUEUED`, `runAttemptCount == 0` → `Joining` | 259 | | `FAILED` with a blank or missing message | 298 | | `CANCELLED` | 301 | `cancel()` (307–309) has no coverage either. Do this **after** W1 so the two seams come out the same shape. They are the same refactor twice and should not diverge; if the join side needs a different split, that is worth a comment on both tickets rather than a quiet divergence. ## The arity guard, and why it is not just three cold lines `onInputsPicked` refuses fewer than two inputs at 196–198: ```kotlin if (uris.size < 2) { _state.value = JoinState.Failed("Pick at least two files to join.") return } ``` **That string is emitted in two files.** `ConcatWorker.kt:42` emits the identical literal, and it was pinned by a test on #148 during the residual-gap audit. This copy is pinned by nothing — so today, changing the ViewModel's wording breaks no test while changing the worker's does, for one message the user sees from one condition. That is W5's subject. **Land W5 first**, then pin whatever survives here. Writing a test against this literal now would freeze the duplication in place, which is the opposite of useful. Worth noting while you are in the file: `JoinScreen.kt:45` filters `uris.isNotEmpty()` before calling this, so the picker cannot deliver zero — but it can deliver one, which is exactly the case this guard is for. Do not conclude from the screen that the guard is unreachable. ## Done when Every arm above is chosen by a test and killed by a mutation, `cancel()` is covered, and the arity guard is pinned against a shared constant rather than a literal. As on W1, `ENQUEUED` needs both sides or it proves nothing about `runAttemptCount`.
JMR-dev commented 2026-08-29 15:37:32 +00:00 (Migrated from github.com)
Copy Link
Copy Source

PR #163. One thing this ticket did not anticipate, found by cutting the seam.

JoinViewModel's SUCCEEDED arm read the join strategy with ConcatStrategy::valueOf, which throws IllegalArgumentException on a name this build does not define. It runs inside a viewModelScope collect with no handler, so the throw is not a Failed state — it takes the process down.

Reachable on this ticket's own premise. WorkManager keeps finished work about a week, and ConcatWorker writes result.strategy.name, so a build that added a third strategy leaves this one crashing on its own completed joins after a rollback.

The repo had already fixed exactly this one file over. ConcatWorker.kt:49-54 reads KEY_FORMAT by lookup rather than valueOf, with a comment saying why — "a format name this build does not define used to throw past the catch". The matching read on the ViewModel side was never changed with it, and nothing pointed at the pair.

Proven rather than argued — restoring valueOf:

java.lang.IllegalArgumentException:
No enum constant org.libremediaconverter.model.ConcatStrategy.SMART_CONCAT_V2

This is the second time in this wave that cutting a seam exposed a defect rather than only a coverage gap, which is worth noting for #153's remaining children: the value of the seam is not just that the branches become testable, it is that the branches become readable — this line had been in front of every reader of observe and its hazard was invisible until it stood alone.

On the ticket's other two items: the arity guard is pinned by SharedFailureMessagesTest from #161, which drives onInputsPicked for real, so it is not repeated here; cancel() is covered by the existing join tests.

**PR #163.** One thing this ticket did not anticipate, found by cutting the seam. `JoinViewModel`'s `SUCCEEDED` arm read the join strategy with **`ConcatStrategy::valueOf`**, which throws `IllegalArgumentException` on a name this build does not define. It runs inside a `viewModelScope` collect with no handler, so the throw is not a `Failed` state — **it takes the process down.** Reachable on this ticket's own premise. WorkManager keeps finished work about a week, and `ConcatWorker` writes `result.strategy.name`, so a build that added a third strategy leaves this one crashing on its own completed joins after a rollback. **The repo had already fixed exactly this one file over.** `ConcatWorker.kt:49-54` reads `KEY_FORMAT` by lookup rather than `valueOf`, with a comment saying why — *"a format name this build does not define used to throw past the catch"*. The matching read on the ViewModel side was never changed with it, and nothing pointed at the pair. Proven rather than argued — restoring `valueOf`: ``` java.lang.IllegalArgumentException: No enum constant org.libremediaconverter.model.ConcatStrategy.SMART_CONCAT_V2 ``` **This is the second time in this wave that cutting a seam exposed a defect rather than only a coverage gap**, which is worth noting for #153's remaining children: the value of the seam is not just that the branches become testable, it is that the branches become *readable* — this line had been in front of every reader of `observe` and its hazard was invisible until it stood alone. On the ticket's other two items: the arity guard is pinned by `SharedFailureMessagesTest` from #161, which drives `onInputsPicked` for real, so it is not repeated here; `cancel()` is covered by the existing join tests.
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#155