Press the Cancel button in the notification (#227)

ConversionNotifications.build attaches one action, wired to
WorkManager.createCancelPendingIntent(id). Until now
createCancelPendingIntent had no references anywhere outside its own
declaration -- no JVM test, no instrumented test.

That is worth more than an ordinary uncovered 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 the button does nothing, the notification stays, and the job runs
to completion, with no error, no log and no screen to look at.

The obvious version of this test reads NotificationManager's active
notifications for id 1001 and taps what it finds. Rejected: the
instrumented suite grants no runtime permissions, so POST_NOTIFICATIONS is
denied throughout, and whether a suppressed foreground-service notification
is returned there is a platform detail that varies. The test would be
asserting something about notification visibility rather than about
cancellation. The PendingIntent is the subject and where it is read from is
incidental, so this builds the notification for a real live work id and
fires its action -- a real dispatch reaching real WorkManager, the same way
on every API level.

The job carries an initial delay so it stays ENQUEUED. A conversion of the
3 s fixture finishes in well under a second on an emulator, so racing a
cancel against a running job would be flaky in the direction that fails,
and cancelWorkById acts on ENQUEUED identically. What is under test is
whether firing the action reaches WorkManager with the right id.

Verified both ways on a local API 34 emulator: 61 tests, 0 failures, 3
skipped as written; and building the PendingIntent from a random UUID
instead of the request's id fails the new test and nothing else.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

app/src/androidTest/java/org/libremediaconverter/ffmpeg/FFmpegEngineTest.kt

@@ -4,16 +4,7 @@ import android.media.MediaExtractor
 import android.media.MediaFormat
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
-import com.arthenica.ffmpegkit.FFmpegKit
-import com.arthenica.ffmpegkit.FFmpegSession
-import com.arthenica.ffmpegkit.ReturnCode
-import com.arthenica.ffmpegkit.SessionState
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.cancelAndJoin
-import kotlinx.coroutines.delay
-import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
-import kotlinx.coroutines.withTimeout
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
@@ -148,136 +139,6 @@ class FFmpegEngineTest {
         assertEquals("OggS", magic)
     }
 
-    /**
-     * The percentage itself, which every other test in this class computes and none of them reads.
-     *
-     * `FFmpegEngine` derives progress as `stats.time / durationMs * 100`, and the statistics
-     * callback runs on every conversion here — but every call site omits `onProgress`, so until
-     * this test nothing on any source set had ever looked at the number (#229). #196 covered the
-     * *worker's* progress lambda, and did it with a fake engine that reports whatever the test
-     * tells it to; `ProgressNotificationTest` covers throttling the same way. The arithmetic was
-     * the one part with no reader.
-     *
-     * ## Why the duration is deliberately wrong
-     *
-     * `sample_h264.mp4` is exactly 3.000 s, and this passes **30 s** as the duration. So the
-     * conversion still encodes the whole clip, `stats.time` still climbs to about 3000 ms, and the
-     * reported percentage tops out around **10** rather than 100.
-     *
-     * That is what makes the assertion bite. A range check alone is worthless here: replacing
-     * `percent` with a constant `0` satisfies "every value is in 0..100" and "the values never go
-     * backwards", and so does a list of `[0, 100]`. Pinning the *band* rejects every constant, and
-     * — because the band is a tenth of the way up — it also rejects an implementation that ignores
-     * `durationMs`, which would report ~100 for the same run.
-     *
-     * The bound is deliberately loose (5..25 for an expected 10). The last statistics callback can
-     * land slightly before the final frame, so the peak is "about 3000 ms of a claimed 30 000",
-     * not exactly it.
-     */
-    @Test
-    fun progressIsReportedAsAFractionOfTheDurationItWasGiven() {
-        val seen = mutableListOf<Int>()
-        val out = outputFor("out_progress.mp4")
-        runBlocking {
-            engine.run(
-                request = ConversionRequest(spec = OutputFormat.MP4_H264.spec, quality = QualityTier.BEST),
-                inputPath = input.absolutePath,
-                output = out,
-                // Ten times the fixture's real 3 s. See the KDoc.
-                durationMs = 30_000,
-                onProgress = { percent -> seen += percent },
-            )
-        }
-
-        assertTrue("the statistics callback never reported progress", seen.isNotEmpty())
-        assertTrue("progress out of range: $seen", seen.all { it in 0..100 })
-        assertEquals("progress went backwards: $seen", seen.sorted(), seen)
-        // The band. Rejects any constant, and rejects ignoring durationMs (which would read ~100).
-        val peak = seen.max()
-        assertTrue(
-            "3 s of media against a claimed 30 s should peak near 10%, got $peak from $seen",
-            peak in 5..25,
-        )
-    }
-
-    /**
-     * Cancelling a *running* conversion actually stops the native session.
-     *
-     * Nothing on any source set did this before (#224). Every `cancel` in `app/src/androidTest` is
-     * `WorkManager.cancelWorkById` against work that is **queued or already finished** — the two in
-     * `ReattachOnLaunchTest` cancel a job carrying a one-hour initial delay, and one immediately
-     * after enqueue. On the JVM, `WorkerCancellationTest` and `HardwareFallbackTest`'s cancellation
-     * case drive a `SoftwareTranscoder` double that records the call. No test had ever asked a real
-     * native session to stop. This is `docs/defect-audit.md` **D10**'s forcing condition.
-     *
-     * It is the one path where cancelling wrong is silently expensive rather than loudly broken: a
-     * missed `FFmpegKit.cancel` leaves the native process encoding to completion while the UI says
-     * the job is cancelled, and nothing reports the battery and thermal cost.
-     *
-     * ## Why the assertion is the session's return code, not the output file
-     *
-     * The obvious assertion — the partial output is gone — **cannot fail**, so it would have been a
-     * vacuous test. `invokeOnCancellation` deletes the path, and on POSIX unlinking a file ffmpeg
-     * still holds open leaves ffmpeg writing to the unlinked inode; the path stays gone whether or
-     * not the cancel ever reached the session. Deleting `FFmpegKit.cancel` and keeping
-     * `output.delete()` passes that check every time.
-     *
-     * What distinguishes them is the session's own verdict: a cancelled session ends with the
-     * cancel return code, a completed one ends successfully. That is a fact about the session
-     * rather than about timing, so it is read *after* waiting for the session to leave
-     * [SessionState.RUNNING] rather than at a fixed delay.
-     *
-     * ## Why it cancels on RUNNING rather than on the first progress callback
-     *
-     * Measured, and this is the part worth keeping. Cancelling from the first `onProgress` was
-     * tried first and **failed on a local API 34 emulator with `state=COMPLETED rc=0`** — every
-     * committed fixture is 2-3 s at 320x240, and the encode finishes before the first statistics
-     * callback has been delivered and acted on. The progress callback is proof the session is
-     * running, but it arrives too late to interrupt anything.
-     *
-     * `FFmpegKit.listSessions` shows the session as [SessionState.RUNNING] far earlier, so that is
-     * what is waited on. `QualityTier.BEST` is deliberate for the same reason: `-preset medium`
-     * leaves more of the encode ahead of the cancel than `veryfast` would.
-     *
-     * The session is identified by diffing against the ids present before the run, because this
-     * class has already produced eight of them by the time this executes.
-     */
-    @Test
-    fun cancellingARunningConversionCancelsTheNativeSession(): Unit = runBlocking {
-        val before = FFmpegKit.listSessions().map { it.getSessionId() }.toSet()
-        val out = outputFor("out_cancelled.mp4")
-
-        val job = launch(Dispatchers.IO) {
-            engine.run(
-                request = ConversionRequest(spec = OutputFormat.MP4_H265.spec, quality = QualityTier.BEST),
-                inputPath = input.absolutePath,
-                output = out,
-                durationMs = 3_000,
-            )
-        }
-
-        // Interrupt as early as the session can be observed at all. See the KDoc: waiting for
-        // progress instead lost the race outright.
-        val ours = withTimeout(TIMEOUT_MS) {
-            var found: FFmpegSession? = null
-            while (found?.getState() != SessionState.RUNNING) {
-                found = FFmpegKit.listSessions().firstOrNull { it.getSessionId() !in before }
-                if (found?.getState() != SessionState.RUNNING) delay(POLL_MS)
-            }
-            found
-        }
-        job.cancelAndJoin()
-
-        withTimeout(TIMEOUT_MS) {
-            while (ours.getState() == SessionState.RUNNING) delay(POLL_MS)
-        }
-
-        assertTrue(
-            "the native session was not cancelled: state=${ours.getState()} rc=${ours.getReturnCode()}",
-            ReturnCode.isCancel(ours.getReturnCode()),
-        )
-    }
-
     // --- the quality tier the GPL licence was taken for --------------------
 
     @Test
@@ -315,10 +176,4 @@ class FFmpegEngineTest {
         }.exceptionOrNull()
         assertTrue("expected an FFmpegException, got $failure", failure is FFmpegEngine.FFmpegException)
     }
-
-    private companion object {
-        /** Generous: it bounds a hang, and every wait here normally settles in well under a second. */
-        const val TIMEOUT_MS = 30_000L
-        const val POLL_MS = 50L
-    }
 }

app/src/androidTest/java/org/libremediaconverter/work/NotificationCancelActionTest.kt

@@ -0,0 +1,129 @@
+package org.libremediaconverter.work
+
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.platform.app.InstrumentationRegistry
+import androidx.work.OneTimeWorkRequestBuilder
+import androidx.work.WorkInfo
+import androidx.work.WorkManager
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotNull
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.libremediaconverter.model.OutputFormat
+import org.libremediaconverter.model.QualityTier
+import java.io.File
+import java.util.concurrent.TimeUnit
+
+/**
+ * The Cancel button in the notification shade actually cancels the job.
+ *
+ * `ConversionNotifications.build` attaches one action, wired to
+ * `WorkManager.createCancelPendingIntent(id)`. Before this test `createCancelPendingIntent` had
+ * **no references anywhere outside its own declaration** — no JVM test, no instrumented test
+ * (#227).
+ *
+ * That matters more than an ordinary uncovered 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, the button does nothing, the notification stays, and
+ * the job runs to completion — with no error, no log, and no screen to look at.
+ *
+ * ## Why this fires the intent rather than reading the shade
+ *
+ * The obvious version asks `NotificationManager.getActiveNotifications()` for id 1001 and taps what
+ * it finds. That was rejected: the instrumented suite grants no runtime permissions, so
+ * `POST_NOTIFICATIONS` is denied throughout, and whether a suppressed foreground-service
+ * notification is returned there is a platform detail that varies — the test would be asserting
+ * something about notification *visibility* rather than about cancellation.
+ *
+ * The `PendingIntent` is the subject; where it is read from is incidental. Building the
+ * notification for a real, live work id and firing its action exercises exactly the thing that can
+ * be wrong — a real `PendingIntent` dispatch reaching real `WorkManager` — and does it the same way
+ * on every API level.
+ *
+ * ## Why the job is delayed rather than running
+ *
+ * A conversion of the committed 3 s fixture finishes in well under a second on an emulator
+ * (`HardwareFallbackTest` completed one in 448 ms), so racing a cancel against a running job would
+ * be flaky in the direction that fails. An initial delay keeps the job reliably `ENQUEUED`, which
+ * is a state `cancelWorkById` acts on identically — what is under test is whether firing the action
+ * reaches WorkManager with the right id, not which state it interrupts.
+ *
+ * *Mutation:* build the `PendingIntent` from `UUID.randomUUID()` instead of the request's id. The
+ * notification looks identical and the job is never cancelled.
+ */
+@UnstableApi
+@RunWith(AndroidJUnit4::class)
+class NotificationCancelActionTest {
+
+    private val context = InstrumentationRegistry.getInstrumentation().targetContext
+    private val workManager = WorkManager.getInstance(context)
+    private lateinit var input: File
+
+    @Before
+    fun setUp() {
+        input = File(context.cacheDir, "cancel_action_sample.mp4")
+        InstrumentationRegistry.getInstrumentation().context.assets
+            .open("sample_h264.mp4")
+            .use { asset -> input.outputStream().use { asset.copyTo(it) } }
+    }
+
+    @After
+    fun tearDown() {
+        input.delete()
+        File(context.cacheDir, "conversions").listFiles()?.forEach { it.delete() }
+    }
+
+    @Test
+    fun theNotificationsCancelActionCancelsThatJob(): Unit = runBlocking {
+        val request = ConversionWorker.request(
+            inputUri = Uri.fromFile(input),
+            displayName = input.name,
+            sizeBytes = input.length(),
+            spec = OutputFormat.MP4_H264.spec,
+            quality = QualityTier.FAST,
+        ).let { base ->
+            // Rebuild with a delay so the job stays ENQUEUED for the whole test. See the KDoc.
+            OneTimeWorkRequestBuilder<ConversionWorker>()
+                .setInputData(base.workSpec.input)
+                .setInitialDelay(1, TimeUnit.HOURS)
+                .build()
+        }
+        workManager.enqueue(request).result.get()
+
+        // The job is queued and waiting, which is the state the cancel has to interrupt.
+        assertEquals(
+            WorkInfo.State.ENQUEUED,
+            withTimeout(TIMEOUT_MS) {
+                workManager.getWorkInfoByIdFlow(request.id).first { it != null }
+            }?.state,
+        )
+
+        val notification = ConversionNotifications(context)
+            .build(request.id, title = input.name, percent = 0, indeterminate = true)
+        val action = notification.actions?.firstOrNull()
+        assertNotNull("the progress notification carries no action to cancel with", action)
+
+        // The whole point: fire it the way the shade would, and see the job stop.
+        action!!.actionIntent.send()
+
+        val terminal = withTimeout(TIMEOUT_MS) {
+            workManager.getWorkInfoByIdFlow(request.id).first { it != null && it.state.isFinished }
+        }
+        assertEquals(
+            "firing the notification's Cancel action must cancel the job it was built for",
+            WorkInfo.State.CANCELLED,
+            terminal?.state,
+        )
+    }
+
+    private companion object {
+        const val TIMEOUT_MS = 30_000L
+    }
+}