C4 (#138): ConcatWorker's cancellation and give-up arms

ConversionWorker has WorkerCancellationTest and DeniedForegroundStartTest.
Its twin had the retry case only -- `a join whose foreground start is
denied` already existed -- so two of ConcatWorker's three failure exits
were cold: the CancellationException arm, and FOREGROUND_DENIED.

Four tests, added to the files that own each rule rather than to a new
ConcatWorker file, which is how this suite is organised: a file per rule,
tested across both workers.

The cancellation seam is worth a look in review. The conversion twin
cancels inside the engine, which is honest there because
ConversionDependencies has a seam for it. ConcatWorker calls ConcatEngine
directly and has none -- it is native and nothing here gets past it -- so
the cancellation is injected at the only other point inside the try,
setForeground. That is a real shape rather than a contrivance: a job
cancelled while WorkManager is promoting it is exactly when that window is
open, and the catch arm cannot tell where in the try it came from.

FailedFuture moved to WorkerStubs.kt on the way. Two tests now inject two
different failures through it, and Kotlin will not take two file-private
top-level classes of one name in one package.

Four mutations, four red, each isolated:

  cancellation arm -> Result.failure     propagation test only
  drop delete on cancellation            cancellation-partial test only
  FOREGROUND_DENIED -> Result.retry      past-the-bound test only
  drop delete on the Throwable path      give-up-partial test only

ConcatWorker's :92, :95-96 and :105-106 are covered; missed branches 4 -> 3.
What is left is what the ticket scoped out: the two input guards (e2e), the
ConcatEngine success path (native), and getForegroundInfo (#88's named
exemption).

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

app/src/test/java/org/libremediaconverter/work/DeniedForegroundStartTest.kt

@@ -27,9 +27,6 @@ import org.robolectric.RobolectricTestRunner
 import org.robolectric.RuntimeEnvironment
 import java.io.File
 import java.util.UUID
-import java.util.concurrent.ExecutionException
-import java.util.concurrent.Executor
-import java.util.concurrent.TimeUnit
 
 /**
  * That a refused foreground-service start does not end the job.
@@ -125,6 +122,40 @@ class DeniedForegroundStartTest {
         )
     }
 
+    @Test
+    fun `a join denied past the attempt bound fails with a message the user can act on`() {
+        // The join twin of the conversion case above. ConcatWorker reaches the same FailureOutcome
+        // through its own `when`, and that arm was the only one of its three with no test -- so a
+        // join that gave up silently, or gave up with an empty Data, would have looked identical to
+        // one that retried.
+        val worker = concatWorker(runAttemptCount = FailureOutcome.MAX_FOREGROUND_START_ATTEMPTS)
+
+        val result = runBlocking { worker.doWork() }
+
+        assertEquals(
+            ListenableWorker.Result.failure(
+                workDataOf(ConcatWorker.KEY_ERROR to FailureOutcome.FOREGROUND_DENIED_MESSAGE),
+            ),
+            result,
+        )
+    }
+
+    @Test
+    fun `a join that gives up collects the partial it had already staged`() {
+        // The delete lives on ConcatWorker's `catch (e: Throwable)` path, which every give-up goes
+        // through. Written first so a missing delete cannot pass by asking whether a file nobody
+        // wrote is absent.
+        concatStagedFile().writeBytes(ByteArray(PARTIAL_BYTES))
+
+        runBlocking { concatWorker(runAttemptCount = FailureOutcome.MAX_FOREGROUND_START_ATTEMPTS).doWork() }
+
+        assertEquals(
+            "a join that gave up must not orphan what it staged",
+            emptyList<String>(),
+            stagedNames(),
+        )
+    }
+
     private fun conversionWorker(runAttemptCount: Int = 0): ConversionWorker =
         TestListenableWorkerBuilder<ConversionWorker>(
             context = app,
@@ -141,18 +172,22 @@ class DeniedForegroundStartTest {
             .setForegroundUpdater(DenyingForegroundUpdater)
             .build()
 
-    private fun concatWorker(): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
+    private fun concatWorker(runAttemptCount: Int = 0): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
         context = app,
         inputData = workDataOf(
             ConcatWorker.KEY_INPUT_URIS to arrayOf(INPUT.toString(), "content://test/second.mp4"),
             ConcatWorker.KEY_TOTAL_BYTES to INPUT_BYTES,
-            ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name,
+            ConcatWorker.KEY_FORMAT to CONCAT_FORMAT.name,
         ),
-        runAttemptCount = 0,
+        runAttemptCount = runAttemptCount,
     ).setId(CONCAT_ID)
         .setForegroundUpdater(DenyingForegroundUpdater)
         .build()
 
+    /** The staging path the join will compute, asked for rather than spelled out here. */
+    private fun concatStagedFile(): File =
+        publisher.createStagingFile(StagingNames.forJob(CONCAT_ID, CONCAT_FORMAT.extension))
+
     /** The staging path the worker will compute, asked for rather than spelled out here. */
     private fun stagedFile(): File = publisher.createStagingFile(StagingNames.forJob(CONVERSION_ID, SPEC.extension))
 
@@ -164,6 +199,7 @@ class DeniedForegroundStartTest {
         const val INPUT_BYTES = 1024L
         const val PARTIAL_BYTES = 2048
         val SPEC = OutputFormat.MP4_H265.spec
+        val CONCAT_FORMAT = OutputFormat.MP4_H264
         val CONVERSION_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000001")
         val CONCAT_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000002")
     }
@@ -182,18 +218,3 @@ private object DenyingForegroundUpdater : ForegroundUpdater {
         ),
     )
 }
-
-/**
- * An already-failed future, written out rather than pulled from a futures library.
- *
- * `await()` takes the `isDone` fast path and unwraps the `ExecutionException`, which is what puts
- * the platform's own exception in front of the worker's catch rather than a wrapper.
- */
-private class FailedFuture(private val failure: Throwable) : ListenableFuture<Void> {
-    override fun addListener(listener: Runnable, executor: Executor): Unit = executor.execute(listener)
-    override fun cancel(mayInterruptIfRunning: Boolean): Boolean = false
-    override fun isCancelled(): Boolean = false
-    override fun isDone(): Boolean = true
-    override fun get(): Void = throw ExecutionException(failure)
-    override fun get(timeout: Long, unit: TimeUnit): Void = throw ExecutionException(failure)
-}

app/src/test/java/org/libremediaconverter/work/WorkerCancellationTest.kt

@@ -1,12 +1,16 @@
 package org.libremediaconverter.work
 
 import android.app.Application
+import android.content.Context
 import android.net.Uri
 import androidx.media3.common.util.UnstableApi
 import androidx.work.Data
+import androidx.work.ForegroundInfo
+import androidx.work.ForegroundUpdater
 import androidx.work.ListenableWorker
 import androidx.work.testing.TestListenableWorkerBuilder
 import androidx.work.workDataOf
+import com.google.common.util.concurrent.ListenableFuture
 import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.runBlocking
 import org.junit.After
@@ -18,6 +22,7 @@ import org.junit.runner.RunWith
 import org.libremediaconverter.convert.ConversionDependencies
 import org.libremediaconverter.convert.OutputPublisher
 import org.libremediaconverter.convert.SoftwareTranscoder
+import org.libremediaconverter.convert.StagingNames
 import org.libremediaconverter.convert.installTestWorkManager
 import org.libremediaconverter.model.ConversionRequest
 import org.libremediaconverter.model.DeviceCodecs
@@ -108,6 +113,54 @@ class WorkerCancellationTest {
         assertEquals("a failed attempt must not leave its partial behind", emptyList<String>(), stagedNames())
     }
 
+    @Test
+    fun `a cancelled join propagates instead of being turned into a Result`() {
+        val thrown = runCatching { runBlocking { concatWorker().doWork() } }.exceptionOrNull()
+
+        assertTrue(
+            "cancellation must leave doWork as cancellation, not as a Result; got $thrown",
+            thrown is CancellationException,
+        )
+    }
+
+    @Test
+    fun `a cancelled join still deletes the partial it had already staged`() {
+        // Written first, so a missing delete cannot pass by asking whether a file nobody wrote is
+        // absent -- the same reason PartialThenFailingTranscoder writes before it throws.
+        concatStagedFile().writeBytes(ByteArray(PARTIAL_STAGED_BYTES))
+
+        runCatching { runBlocking { concatWorker().doWork() } }
+
+        assertEquals("a cancelled join must not leave its partial behind", emptyList<String>(), stagedNames())
+    }
+
+    /**
+     * A join whose foreground start is cancelled rather than denied.
+     *
+     * The conversion twin cancels *inside the engine*, which is the honest shape there because
+     * `ConversionDependencies` has a seam for it. `ConcatWorker` calls `ConcatEngine` directly and
+     * has no such seam -- it is native, and nothing here gets past it -- so the cancellation is
+     * injected at the only other point inside the `try`: `setForeground`. That is not a contrivance.
+     * A job cancelled while WorkManager is promoting it to the foreground is precisely when the
+     * window is open, and what is being tested is the `catch` arm, which cannot tell where in the
+     * `try` the cancellation came from.
+     */
+    private fun concatWorker(): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
+        context = app,
+        inputData = workDataOf(
+            ConcatWorker.KEY_INPUT_URIS to arrayOf(INPUT.toString(), "file:///tmp/second.mp4"),
+            ConcatWorker.KEY_TOTAL_BYTES to INPUT_BYTES,
+            ConcatWorker.KEY_FORMAT to CONCAT_FORMAT.name,
+        ),
+        runAttemptCount = 0,
+    ).setId(CONCAT_ID)
+        .setForegroundUpdater(CancellingForegroundUpdater)
+        .build()
+
+    /** The staging path the join will compute, asked for rather than spelled out here. */
+    private fun concatStagedFile(): File =
+        publisher.createStagingFile(StagingNames.forJob(CONCAT_ID, CONCAT_FORMAT.extension))
+
     /**
      * A worker routed to the software engine, which is [failure] and nothing else.
      *
@@ -142,7 +195,10 @@ class WorkerCancellationTest {
         const val DISPLAY_NAME = "holiday.mp4"
         const val INPUT_BYTES = 1024L
         val SPEC = OutputFormat.MP4_H265.spec
+        val CONCAT_FORMAT = OutputFormat.MP4_H264
+        const val PARTIAL_STAGED_BYTES = 2048
         val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000003")
+        val CONCAT_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000004")
     }
 }
 
@@ -167,3 +223,19 @@ private class PartialThenFailingTranscoder(private val failure: () -> Nothing) :
         const val PARTIAL_BYTES = 2048
     }
 }
+
+/**
+ * Stands in for a job cancelled while WorkManager is promoting it to the foreground.
+ *
+ * The mechanism `DeniedForegroundStartTest` documents, carrying a different exception:
+ * `WorkForegroundUpdater` propagates whatever the future failed with, and
+ * `ListenableFuture.await()` unwraps the `ExecutionException`, so the worker meets a bare
+ * `CancellationException` exactly where a real cancellation would put one.
+ */
+private object CancellingForegroundUpdater : ForegroundUpdater {
+    override fun setForegroundAsync(
+        context: Context,
+        id: UUID,
+        foregroundInfo: ForegroundInfo,
+    ): ListenableFuture<Void> = FailedFuture(CancellationException("cancelled while going foreground"))
+}

app/src/test/java/org/libremediaconverter/work/WorkerStubs.kt

@@ -1,10 +1,14 @@
 package org.libremediaconverter.work
 
 import android.content.Context
+import com.google.common.util.concurrent.ListenableFuture
 import org.libremediaconverter.convert.OutputPublisher
 import org.libremediaconverter.convert.SoftwareTranscoder
 import org.libremediaconverter.model.ConversionRequest
 import java.io.File
+import java.util.concurrent.ExecutionException
+import java.util.concurrent.Executor
+import java.util.concurrent.TimeUnit
 
 /**
  * Scaffolding more than one worker test needs.
@@ -68,3 +72,25 @@ object WritingTranscoder : SoftwareTranscoder {
 
     private const val OUTPUT_BYTES = 512
 }
+
+/**
+ * An already-failed future, written out rather than pulled from a futures library.
+ *
+ * `await()` takes the `isDone` fast path and unwraps the `ExecutionException`, which is what puts
+ * the original exception in front of the worker's `catch` rather than a wrapper. That is the whole
+ * mechanism behind driving a `ForegroundUpdater` to fail: `WorkForegroundUpdater` propagates
+ * whatever the future failed with rather than swallowing it, so `setForeground()` throws exactly
+ * what is handed here.
+ *
+ * Shared because two tests inject two different failures through it -- a denied foreground start
+ * and a cancellation -- and Kotlin will not take two file-private top-level classes of one name in
+ * one package.
+ */
+internal class FailedFuture(private val failure: Throwable) : ListenableFuture<Void> {
+    override fun addListener(listener: Runnable, executor: Executor): Unit = executor.execute(listener)
+    override fun cancel(mayInterruptIfRunning: Boolean): Boolean = false
+    override fun isCancelled(): Boolean = false
+    override fun isDone(): Boolean = true
+    override fun get(): Void = throw ExecutionException(failure)
+    override fun get(timeout: Long, unit: TimeUnit): Void = throw ExecutionException(failure)
+}