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

A retry-save's file type is unpinned, and the exemption saying so is out of date #201

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

Wave 4, filed from a coverage read on main @ 54ca2dd, 2026-09-02. The shared filter note is on #194.

A retry-save opens with the wrong file type, and the line that prevents it has never run

app/src/main/java/org/libremediaconverter/convert/ConverterScreen.kt:80
app/src/main/java/org/libremediaconverter/join/JoinScreen.kt:52

val destinationMime = state.pendingSave()?.mimeType ?: settings.spec.mimeType

The elvis left side has never executed. Its comment (ConverterScreen.kt:72-79) records what it is for:

Through pendingSave() rather than a cast to Converted, so a retry offered after a failed save opens the dialog with the type its first attempt used — the cast answered null for a Failed, and the fallback below is the current picker, which a reattached job never set.

So the untested half is the fix, and the tested half is the fallback it was added to stop being used.

This revises a named exemption, deliberately

FailedSaveRetryTest's KDoc lists this line under "Not asserted here, so each is a decision rather than an omission":

ConverterScreen's destinationMime line itself. It lives in the entry point, above the ScreenContent seam, and reaching it needs a real ViewModel inside a composition. What it reads — pendingSave()?.mimeType — is asserted directly instead, which is why that derivation was moved out of the entry point in the first place.

That was true when it was written. AdaptiveShellTest (#173) then established exactly that capability — createAndroidComposeRule composing AppRoot with its default content, reaching the real screens with real ViewModels — so the reason the exemption gave no longer holds.

Same shape as #141 revising #84's boundary: the close was right about its evidence and the evidence moved. Correct that KDoc in the same PR, or the next reader takes the exemption at face value.

Behaviour the test asserts

With a completed job whose KEY_MIME_TYPE differs from settings.spec.mimeType, the CreateDocument intent's type is the job's, not the picker's.

The fixture

ScreenWiringTest already drives a real ViewModel to a completed job through installTestWorkManager; extend that shape with output data carrying KEY_OUTPUT_PATH, KEY_SUGGESTED_NAME and KEY_MIME_TYPE to "video/x-matroska", against the default MP4 picker so the two differ visibly.

Reading the launched intent needs shadowOf(activity).nextStartedActivityForResult, the same mechanic #200 depends on. Take #200 first and reuse whatever it establishes; if that mechanic does not work under Robolectric, this ticket inherits the answer.

Acceptance: the mutation that must go red

Collapse :80 to settings.spec.mimeType. Restore, confirm green.

Not in scope

JoinScreen.kt:52 is the same line in the join screen and should move with it if the fixture generalises cheaply. If it does not, say so — one screen pinned is better than two half-pinned.

_Wave 4, filed from a coverage read on `main` @ `54ca2dd`, 2026-09-02. The shared filter note is on #194._ ## A retry-save opens with the wrong file type, and the line that prevents it has never run ``` app/src/main/java/org/libremediaconverter/convert/ConverterScreen.kt:80 app/src/main/java/org/libremediaconverter/join/JoinScreen.kt:52 ``` ```kotlin val destinationMime = state.pendingSave()?.mimeType ?: settings.spec.mimeType ``` The elvis **left** side has never executed. Its comment (`ConverterScreen.kt:72-79`) records what it is for: > Through `pendingSave()` rather than a cast to `Converted`, so a retry offered after a failed save opens the dialog with the type its first attempt used — the cast answered null for a `Failed`, and the fallback below is the current picker, which a reattached job never set. So the untested half is the fix, and the tested half is the fallback it was added to stop being used. ## This revises a named exemption, deliberately `FailedSaveRetryTest`'s KDoc lists this line under "Not asserted here, so each is a decision rather than an omission": > **`ConverterScreen`'s `destinationMime` line itself.** It lives in the entry point, above the `ScreenContent` seam, and reaching it needs a real ViewModel inside a composition. What it reads — `pendingSave()?.mimeType` — is asserted directly instead, which is why that derivation was moved out of the entry point in the first place. That was true when it was written. **`AdaptiveShellTest` (#173) then established exactly that capability** — `createAndroidComposeRule` composing `AppRoot` with its default content, reaching the real screens with real ViewModels — so the reason the exemption gave no longer holds. Same shape as #141 revising #84's boundary: the close was right about its evidence and the evidence moved. **Correct that KDoc in the same PR**, or the next reader takes the exemption at face value. ## Behaviour the test asserts With a completed job whose `KEY_MIME_TYPE` differs from `settings.spec.mimeType`, the `CreateDocument` intent's `type` is the **job's**, not the picker's. ## The fixture `ScreenWiringTest` already drives a real ViewModel to a completed job through `installTestWorkManager`; extend that shape with output data carrying `KEY_OUTPUT_PATH`, `KEY_SUGGESTED_NAME` and `KEY_MIME_TYPE to "video/x-matroska"`, against the default MP4 picker so the two differ visibly. Reading the launched intent needs `shadowOf(activity).nextStartedActivityForResult`, the same mechanic #200 depends on. **Take #200 first** and reuse whatever it establishes; if that mechanic does not work under Robolectric, this ticket inherits the answer. ## Acceptance: the mutation that must go red Collapse `:80` to `settings.spec.mimeType`. Restore, confirm green. ## Not in scope `JoinScreen.kt:52` is the same line in the join screen and should move with it if the fixture generalises cheaply. If it does not, say so — one screen pinned is better than two half-pinned.
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#201