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

The notification's Cancel action has never been fired by any test #227

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

Filed from the 2026-09-05 e2e read of the instrumented suite on main @ 4b02294.

The Cancel button in the notification shade is the app's only control that lives outside its own UI, and nothing has ever pressed it.

app/src/main/java/org/libremediaconverter/work/ConversionNotifications.kt:46-50

.addAction(
    android.R.drawable.ic_menu_close_clear_cancel,
    context.getString(R.string.action_cancel),
    WorkManager.getInstance(context).createCancelPendingIntent(id),
)

grep -rn createCancelPendingIntent app/src returns that line and nothing else — no JVM test, no instrumented test.

Why it matters more than a normal untested line

A conversion runs in a foreground service and the user is invited to leave the app. Once they do, this action is the only way to stop it. If the PendingIntent carries the wrong id — a stale UUID, or the request's rather than the running work's — the button does nothing, the notification stays, and the job runs to completion. There is no error, no log, and no screen to look at.

The JVM notification tests do not reach it: ProgressNotificationTest covers throttling and that progress goes to WorkManager rather than a NotificationManager.notify; NotificationProgressTextTest covers the strings.

Two halves, and only one needs a device

Say which half this ticket is for before starting — they have different homes and the cheap one is not the interesting one.

  • The action exists and is shaped right — that the notification carries exactly one action, with the Cancel label. This is JVM-testable under Robolectric off ConversionNotifications.build(...) and belongs in the unit suite. It would catch the action being dropped, not the intent being wrong.
  • The intent actually cancels the job — fire notification.actions[0].actionIntent.send() and await WorkInfo.State.CANCELLED. This needs a real PendingIntent dispatch and real WorkManager, so it is instrumented. This is the half worth having, because it is the one that catches a wrong id.

Getting hold of the posted notification on device is the fiddly part: the worker posts through setForeground, so read it from the NotificationManager's active notifications by id (1001 for ConversionWorker, 1002 for ConcatWorker) rather than reconstructing it — reconstructing it would test the builder, which is the JVM half again.

Note the permission. The suite grants no runtime permissions at all, so POST_NOTIFICATIONS is denied and the notification does not reach the shade. getActiveNotifications still sees a foreground-service notification, but confirm that on the target API before building the test on it — if it does not, this needs the same GrantPermissionRule decision as the content:// ticket.

Mutation: build the PendingIntent from UUID.randomUUID() instead of id. The notification looks identical, the button does nothing, and nothing in either suite goes red today.

Related

  • The cancel-a-running-session ticket — different layer, same user action; that one is about the engine stopping, this is about the request reaching WorkManager at all
  • #192 — the Cancel button in the app had the same shape of gap and was closed on the JVM
_Filed from the 2026-09-05 e2e read of the instrumented suite on `main` @ `4b02294`._ The Cancel button in the notification shade is the app's only control that lives outside its own UI, and nothing has ever pressed it. ``` app/src/main/java/org/libremediaconverter/work/ConversionNotifications.kt:46-50 ``` ```kotlin .addAction( android.R.drawable.ic_menu_close_clear_cancel, context.getString(R.string.action_cancel), WorkManager.getInstance(context).createCancelPendingIntent(id), ) ``` `grep -rn createCancelPendingIntent app/src` returns **that line and nothing else** — no JVM test, no instrumented test. ## Why it matters more than a normal untested line A conversion runs in a foreground service and the user is invited to leave the app. Once they do, this action is the only way to stop it. If the `PendingIntent` carries the wrong id — a stale `UUID`, or the request's rather than the running work's — the button does nothing, the notification stays, and the job runs to completion. There is no error, no log, and no screen to look at. The JVM notification tests do not reach it: `ProgressNotificationTest` covers throttling and that progress goes to WorkManager rather than a `NotificationManager.notify`; `NotificationProgressTextTest` covers the strings. ## Two halves, and only one needs a device **Say which half this ticket is for before starting** — they have different homes and the cheap one is not the interesting one. - **The action exists and is shaped right** — that the notification carries exactly one action, with the Cancel label. This is JVM-testable under Robolectric off `ConversionNotifications.build(...)` and belongs in the unit suite. It would catch the action being dropped, not the intent being wrong. - **The intent actually cancels the job** — fire `notification.actions[0].actionIntent.send()` and await `WorkInfo.State.CANCELLED`. This needs a real `PendingIntent` dispatch and real WorkManager, so it is instrumented. **This is the half worth having**, because it is the one that catches a wrong id. Getting hold of the posted notification on device is the fiddly part: the worker posts through `setForeground`, so read it from the `NotificationManager`'s active notifications by id (`1001` for `ConversionWorker`, `1002` for `ConcatWorker`) rather than reconstructing it — reconstructing it would test the builder, which is the JVM half again. **Note the permission.** The suite grants no runtime permissions at all, so `POST_NOTIFICATIONS` is denied and the notification does not reach the shade. `getActiveNotifications` still sees a foreground-service notification, but confirm that on the target API before building the test on it — if it does not, this needs the same `GrantPermissionRule` decision as the `content://` ticket. *Mutation:* build the `PendingIntent` from `UUID.randomUUID()` instead of `id`. The notification looks identical, the button does nothing, and nothing in either suite goes red today. ## Related - The cancel-a-running-session ticket — different layer, same user action; that one is about the engine stopping, this is about the request reaching WorkManager at all - #192 — the Cancel *button in the app* had the same shape of gap and was closed on the JVM
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 bug sev:medium
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#227