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

ConcatWorker's failure path is untested on every source set, because nothing can get past ConcatEngine #176

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

ConcatWorker has no injectable engine, and the repo has already measured what that costs

ConcatWorker.kt:80 constructs ConcatEngine(applicationContext) directly, unlike
ConversionWorker, which reaches its engines through ConversionDependencies.hardware /
.software. Nothing in a JVM test can get past that line, because ConcatEngine is native.

The cost is not hypothetical. PerJobStagingTest's own KDoc
(app/src/test/java/org/libremediaconverter/work/PerJobStagingTest.kt:39-42) records that
reverting ConcatWorker to a constant staging name left all 257 tests green — because nothing
gets past ConcatEngine. RefusedJobTest.kt:161-163 says the same thing from the other side:
"the next thing past the count guard is ConcatEngine, which is native."

What is untested because of it

ConcatWorker.kt:97-113, the ordinary failure path — untested on any source set, JVM or device:

  • the FailureOutcome.FAIL arm's Log.e + Result.failure mapping (:108-111);
  • e.message ?: GENERIC_FAILURE_MESSAGE (:110) for a null-message exception;
  • staged.delete() on that path (:98).

ConcatEngineTest (androidTest) tests ConcatEngine.join() directly, bypassing the worker;
ConcatWorkerTest (androidTest) covers only the too-few-inputs guard and the happy path. Nothing
provokes a genuine mid-join engine failure through doWork().

The seam

ConversionDependencies.concat: (Context) -> ConcatJoiner, mirroring the two providers that already
exist in app/src/main/java/org/libremediaconverter/convert/Transcoders.kt. One seam unlocks all
three behaviours above.

Fold in: the refusal that does not need a device at all

ConcatWorker.kt:39-40 — ?: return Result.failure(workDataOf(KEY_ERROR to "No input files.")).
Covered today only by androidTest/fallback/UnopenableUriTest.kt:127, although it runs before
staging and before any native code. It is the exact sibling of RefusedJobTest's ConversionWorker
twin (a job with no input URI fails with a message rather than a bare failure), and belongs beside
it. Note while there that this message is a bare literal while its neighbour is
TOO_FEW_INPUTS_MESSAGE — the convention W5 (#158) established.

Acceptance: the mutations that must go red

Make the fake joiner throw, then: change GENERIC_FAILURE_MESSAGE handling to e.message!!;
delete staged.delete() on the failure path; delete the "No input files." return.

Sequencing

Last in the wave's stack. It is the largest production change proposed, so a review question here
must not block the cheap tickets behind it.

## ConcatWorker has no injectable engine, and the repo has already measured what that costs `ConcatWorker.kt:80` constructs `ConcatEngine(applicationContext)` directly, unlike `ConversionWorker`, which reaches its engines through `ConversionDependencies.hardware` / `.software`. Nothing in a JVM test can get past that line, because `ConcatEngine` is native. The cost is not hypothetical. `PerJobStagingTest`'s own KDoc (`app/src/test/java/org/libremediaconverter/work/PerJobStagingTest.kt:39-42`) records that **reverting `ConcatWorker` to a constant staging name left all 257 tests green** — because nothing gets past `ConcatEngine`. `RefusedJobTest.kt:161-163` says the same thing from the other side: *"the next thing past the count guard is `ConcatEngine`, which is native."* ## What is untested because of it `ConcatWorker.kt:97-113`, the ordinary failure path — untested on **any** source set, JVM or device: - the `FailureOutcome.FAIL` arm's `Log.e` + `Result.failure` mapping (`:108-111`); - `e.message ?: GENERIC_FAILURE_MESSAGE` (`:110`) for a null-message exception; - `staged.delete()` on that path (`:98`). `ConcatEngineTest` (androidTest) tests `ConcatEngine.join()` directly, bypassing the worker; `ConcatWorkerTest` (androidTest) covers only the too-few-inputs guard and the happy path. Nothing provokes a genuine mid-join engine failure through `doWork()`. ## The seam `ConversionDependencies.concat: (Context) -> ConcatJoiner`, mirroring the two providers that already exist in `app/src/main/java/org/libremediaconverter/convert/Transcoders.kt`. One seam unlocks all three behaviours above. ## Fold in: the refusal that does not need a device at all `ConcatWorker.kt:39-40` — `?: return Result.failure(workDataOf(KEY_ERROR to "No input files."))`. Covered today only by `androidTest/fallback/UnopenableUriTest.kt:127`, although it runs before staging and before any native code. It is the exact sibling of `RefusedJobTest`'s ConversionWorker twin (`a job with no input URI fails with a message rather than a bare failure`), and belongs beside it. Note while there that this message is a bare literal while its neighbour is `TOO_FEW_INPUTS_MESSAGE` — the convention W5 (#158) established. ## Acceptance: the mutations that must go red Make the fake joiner throw, then: change `GENERIC_FAILURE_MESSAGE` handling to `e.message!!`; delete `staged.delete()` on the failure path; delete the `"No input files."` return. ## Sequencing Last in the wave's stack. It is the largest production change proposed, so a review question here must not block the cheap tickets behind it.
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#176