Closes#155. Stacked on #162 (W1), which this mirrors — same refactor, same shape, and it should not land first.
The mapping
Five arms had never been chosen by any test, for the same reason as W1: a real ConcatWorker only ever reaches a terminal state with well-formed output. RUNNING/BLOCKED, both sides of the retry check, a success naming no file, a blank failure, and CANCELLED all ran never.
valueOf throws on a name this build does not define, and this runs inside a viewModelScope collect with no handler — so it does not become a Failed state, it takes the process down.
Not theoretical. WorkManager keeps finished work for about a week, so a downgrade or rollback hands this build a job enqueued by another one — the premise WorkerEnumFallbackTest and JobTags are both written on. ConcatWorker writes result.strategy.name into the output Data, so a build that added a third strategy would leave this one crashing on its own completed joins.
The codebase had already made this exact fix one file over, in ConcatWorker.kt, and wrote down why:
// Looked up rather than valueOf -- see the same three reads in ConversionWorker. This one is above the try as well, so a format name this build does not define used to throw past the catch
The matching read on the ViewModel side had not been changed with it. It is now ConcatStrategy.entries.firstOrNull { it.name == name } ?: REENCODE.
Proven, not asserted
Restoring valueOf and running the new test:
RED: an unknown strategy name is read as a re-encode rather than thrown
java.lang.IllegalArgumentException:
No enum constant org.libremediaconverter.model.ConcatStrategy.SMART_CONCAT_V2
REENCODE is the conservative default rather than an arbitrary one: it is the answer for inputs that do not match, so a job whose strategy cannot be read is described as the more cautious of the two rather than claimed as a lossless stream copy. That matters to the user — the join screen tells them whether their files were stream-copied or re-encoded, which is the difference between lossless and lossy. Mutating the fallback to STREAM_COPY reddens two tests.
Mutations
mutation
red
unknown strategy → STREAM_COPY
2 tests
runAttemptCount ignored, both directions
2 tests
success with no path → empty Joined
1 test
BLOCKED unfolded from RUNNING
1 test
blank error no longer falls back
1 test
cancellation ignores the caller's landing state
1 test
Note on the arity guard
#155 also lists JoinViewModel's "fewer than two files" guard. That is now pinned by SharedFailureMessagesTest from #161 (W5), which drives onInputsPicked for real and asserts the ViewModel and the worker refuse with the same sentence — so it is covered, and duplicating it here would add nothing. cancel() is covered by the existing join tests.
Closes #155. Stacked on #162 (W1), which this mirrors — same refactor, same shape, and it should not land first.
## The mapping
Five arms had never been chosen by any test, for the same reason as W1: a real `ConcatWorker` only ever reaches a terminal state with well-formed output. `RUNNING`/`BLOCKED`, both sides of the retry check, a success naming no file, a blank failure, and `CANCELLED` all ran never.
## What the seam turned up — a real crash
The `SUCCEEDED` arm contained:
```kotlin
info.outputData.getString(ConcatWorker.KEY_STRATEGY)
?.let(ConcatStrategy::valueOf) ?: ConcatStrategy.REENCODE
```
**`valueOf` throws on a name this build does not define**, and this runs inside a `viewModelScope` collect with no handler — so it does not become a `Failed` state, it takes the process down.
Not theoretical. WorkManager keeps finished work for about a week, so a downgrade or rollback hands this build a job enqueued by another one — the premise `WorkerEnumFallbackTest` and `JobTags` are both written on. `ConcatWorker` writes `result.strategy.name` into the output `Data`, so a build that added a third strategy would leave this one crashing on its own completed joins.
**The codebase had already made this exact fix one file over**, in `ConcatWorker.kt`, and wrote down why:
> `// Looked up rather than valueOf -- see the same three reads in ConversionWorker. This one is above the try as well, so a format name this build does not define used to throw past the catch`
The matching read on the ViewModel side had not been changed with it. It is now `ConcatStrategy.entries.firstOrNull { it.name == name } ?: REENCODE`.
### Proven, not asserted
Restoring `valueOf` and running the new test:
```
RED: an unknown strategy name is read as a re-encode rather than thrown
java.lang.IllegalArgumentException:
No enum constant org.libremediaconverter.model.ConcatStrategy.SMART_CONCAT_V2
```
`REENCODE` is the conservative default rather than an arbitrary one: it is the answer for inputs that do not match, so a job whose strategy cannot be read is described as the more cautious of the two rather than claimed as a lossless stream copy. That matters to the user — the join screen tells them whether their files were stream-copied or re-encoded, which is the difference between lossless and lossy. Mutating the fallback to `STREAM_COPY` reddens two tests.
## Mutations
| mutation | red |
|---|---|
| unknown strategy → `STREAM_COPY` | 2 tests |
| `runAttemptCount` ignored, both directions | 2 tests |
| success with no path → empty `Joined` | 1 test |
| `BLOCKED` unfolded from `RUNNING` | 1 test |
| blank error no longer falls back | 1 test |
| cancellation ignores the caller's landing state | 1 test |
## Note on the arity guard
#155 also lists `JoinViewModel`'s "fewer than two files" guard. That is now pinned by `SharedFailureMessagesTest` from #161 (W5), which drives `onInputsPicked` for real and asserts the ViewModel and the worker refuse with the same sentence — so it is covered, and duplicating it here would add nothing. `cancel()` is covered by the existing join tests.
**516 → 530 tests, 87.7% → 88.0% line, 70.4% → 71.3% branch.** Gate green: `assembleDebug`, `testDebugUnitTest`, `compileDebugAndroidTestKotlin`, `ktlintCheck`, `detekt`, `lintDebug`.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #155. Stacked on #162 (W1), which this mirrors — same refactor, same shape, and it should not land first.
The mapping
Five arms had never been chosen by any test, for the same reason as W1: a real
ConcatWorkeronly ever reaches a terminal state with well-formed output.RUNNING/BLOCKED, both sides of the retry check, a success naming no file, a blank failure, andCANCELLEDall ran never.What the seam turned up — a real crash
The
SUCCEEDEDarm contained:valueOfthrows on a name this build does not define, and this runs inside aviewModelScopecollect with no handler — so it does not become aFailedstate, it takes the process down.Not theoretical. WorkManager keeps finished work for about a week, so a downgrade or rollback hands this build a job enqueued by another one — the premise
WorkerEnumFallbackTestandJobTagsare both written on.ConcatWorkerwritesresult.strategy.nameinto the outputData, so a build that added a third strategy would leave this one crashing on its own completed joins.The codebase had already made this exact fix one file over, in
ConcatWorker.kt, and wrote down why:The matching read on the ViewModel side had not been changed with it. It is now
ConcatStrategy.entries.firstOrNull { it.name == name } ?: REENCODE.Proven, not asserted
Restoring
valueOfand running the new test:REENCODEis the conservative default rather than an arbitrary one: it is the answer for inputs that do not match, so a job whose strategy cannot be read is described as the more cautious of the two rather than claimed as a lossless stream copy. That matters to the user — the join screen tells them whether their files were stream-copied or re-encoded, which is the difference between lossless and lossy. Mutating the fallback toSTREAM_COPYreddens two tests.Mutations
STREAM_COPYrunAttemptCountignored, both directionsJoinedBLOCKEDunfolded fromRUNNINGNote on the arity guard
#155 also lists
JoinViewModel's "fewer than two files" guard. That is now pinned bySharedFailureMessagesTestfrom #161 (W5), which drivesonInputsPickedfor real and asserts the ViewModel and the worker refuse with the same sentence — so it is covered, and duplicating it here would add nothing.cancel()is covered by the existing join tests.516 → 530 tests, 87.7% → 88.0% line, 70.4% → 71.3% branch. Gate green:
assembleDebug,testDebugUnitTest,compileDebugAndroidTestKotlin,ktlintCheck,detekt,lintDebug.