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

C1 — readSpec's three enum fallbacks are cold, and WorkerEnumFallbackTest exists for exactly this defect #135

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

Child 1 of 7 decomposing #132 — item 1 there. Independent: depends on nothing, blocks nothing. The sharpest item in the parent.

Why this exists

app/src/main/java/org/libremediaconverter/work/ConversionWorker.kt:328-340

readSpec() has three ?: return fallback arms, one per axis, and all three are cold:

val container = inputData.getString(KEY_CONTAINER)
    ?.let { name -> Container.entries.firstOrNull { it.name == name } }
    ?: return fallback          // :332  — never executed
val video = ...                 // :335  — never executed
val audio = ...                 // :338  — never executed

WorkerEnumFallbackTest exists for this exact defect class, and its KDoc states the signature
precisely:

All three sit above the workers' try, so an unknown name threw IllegalArgumentException out of
doWork() entirely: WorkManager logged FAILURE with reschedule = false, the output Data
reached the UI with zero entries so the screen said "Conversion failed." with nothing else, and the
staged file was never deleted.

It covers 2 of the 5 above-the-try reads — KEY_QUALITY and KEY_ENGINE_PREFERENCE.
readSpec() is called at :69, also above the try, and its three are the ones left.

This is not a malformed-input hypothetical. WorkManager keeps queued and finished work for about a
week, so a downgrade — or any rollback with work still in the queue — hands this build a job enqueued
by another one. That is the premise WorkerEnumFallbackTest is written on, and JobTags too.

Scope

Three tests in WorkerEnumFallbackTest, beside the two already there and using the same
conversionWorker(...) builder, which needs one parameter per axis added.

Done means

Each of KEY_CONTAINER, KEY_VIDEO_CODEC, KEY_AUDIO_CODEC independently carrying a name this
build does not define, and each producing OutputFormat.MP4_H265.spec — without throwing, and
with the job still reaching a Result.

Assert the spec that ran, not just that a Result came back. A fallback to something arbitrary
would silently convert to a format nobody asked for, which is the half WorkerEnumFallbackTest's
existing quality test is careful to pin (assertEquals(listOf(QualityTier.FAST), transcoder.qualities)).

Mutation: change one ?: return fallback to ?: error("…"). The test for that axis must go red
naming it — and must fail as an assertion, not as a timeout.

_Child 1 of 7 decomposing #132 — item 1 there. Independent: depends on nothing, blocks nothing. **The sharpest item in the parent.**_ ### Why this exists ``` app/src/main/java/org/libremediaconverter/work/ConversionWorker.kt:328-340 ``` `readSpec()` has three `?: return fallback` arms, one per axis, and **all three are cold**: ```kotlin val container = inputData.getString(KEY_CONTAINER) ?.let { name -> Container.entries.firstOrNull { it.name == name } } ?: return fallback // :332 — never executed val video = ... // :335 — never executed val audio = ... // :338 — never executed ``` `WorkerEnumFallbackTest` exists **for this exact defect class**, and its KDoc states the signature precisely: > All three sit above the workers' `try`, so an unknown name threw `IllegalArgumentException` out of > `doWork()` entirely: WorkManager logged FAILURE with `reschedule = false`, the output `Data` > reached the UI with zero entries so the screen said "Conversion failed." with nothing else, and the > staged file was never deleted. It covers **2 of the 5** above-the-`try` reads — `KEY_QUALITY` and `KEY_ENGINE_PREFERENCE`. `readSpec()` is called at `:69`, also above the `try`, and its three are the ones left. This is not a malformed-input hypothetical. WorkManager keeps queued and finished work for about a week, so a downgrade — or any rollback with work still in the queue — hands this build a job enqueued by another one. That is the premise `WorkerEnumFallbackTest` is written on, and `JobTags` too. ### Scope Three tests in `WorkerEnumFallbackTest`, beside the two already there and using the same `conversionWorker(...)` builder, which needs one parameter per axis added. ### Done means Each of `KEY_CONTAINER`, `KEY_VIDEO_CODEC`, `KEY_AUDIO_CODEC` independently carrying a name this build does not define, and each producing `OutputFormat.MP4_H265.spec` — *without* throwing, and with the job still reaching a `Result`. Assert the **spec that ran**, not just that a Result came back. A fallback to something arbitrary would silently convert to a format nobody asked for, which is the half `WorkerEnumFallbackTest`'s existing quality test is careful to pin (`assertEquals(listOf(QualityTier.FAST), transcoder.qualities)`). **Mutation:** change one `?: return fallback` to `?: error("…")`. The test for that axis must go red naming it — and must fail as an assertion, not as a timeout.
JMR-dev commented 2026-09-02 01:40:29 +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 #145 carried Closes #135, but GitHub only fires a closing keyword when the PR merges into the default branch. #145 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:

KEY_CONTAINER    ?: return fallback -> ?: error(...)
  red: a container this build does not define falls back to the default spec
KEY_VIDEO_CODEC  ?: return fallback -> ?: error(...)
  red: a video codec this build does not define falls back to the default spec
KEY_AUDIO_CODEC  ?: return fallback -> ?: error(...)
  red: an audio codec this build does not define falls back to the default spec

Each axis reddens only its own test, which is what this ticket asked for — a single test covering all three would pass against a fallback that fired on the wrong key.

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 #145 carried `Closes #135`, but **GitHub only fires a closing keyword when the PR merges into the default branch.** #145 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: ``` KEY_CONTAINER ?: return fallback -> ?: error(...) red: a container this build does not define falls back to the default spec KEY_VIDEO_CODEC ?: return fallback -> ?: error(...) red: a video codec this build does not define falls back to the default spec KEY_AUDIO_CODEC ?: return fallback -> ?: error(...) red: an audio codec this build does not define falls back to the default spec ``` Each axis reddens **only its own test**, which is what this ticket asked for — a single test covering all three would pass against a fallback that fired on the wrong key. 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#135