diff --git a/app/src/test/java/org/libremediaconverter/work/DeniedForegroundStartTest.kt b/app/src/test/java/org/libremediaconverter/work/DeniedForegroundStartTest.kt index 2308a44..a8e4c9b 100644 --- a/app/src/test/java/org/libremediaconverter/work/DeniedForegroundStartTest.kt +++ b/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(), + stagedNames(), + ) + } + private fun conversionWorker(runAttemptCount: Int = 0): ConversionWorker = TestListenableWorkerBuilder( context = app, @@ -141,18 +172,22 @@ class DeniedForegroundStartTest { .setForegroundUpdater(DenyingForegroundUpdater) .build() - private fun concatWorker(): ConcatWorker = TestListenableWorkerBuilder( + private fun concatWorker(runAttemptCount: Int = 0): ConcatWorker = TestListenableWorkerBuilder( 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 { - 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) -} diff --git a/app/src/test/java/org/libremediaconverter/work/WorkerCancellationTest.kt b/app/src/test/java/org/libremediaconverter/work/WorkerCancellationTest.kt index a04f345..0a79eb8 100644 --- a/app/src/test/java/org/libremediaconverter/work/WorkerCancellationTest.kt +++ b/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(), 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(), 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( + 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 = FailedFuture(CancellationException("cancelled while going foreground")) +} diff --git a/app/src/test/java/org/libremediaconverter/work/WorkerStubs.kt b/app/src/test/java/org/libremediaconverter/work/WorkerStubs.kt index 848c04b..14f3abe 100644 --- a/app/src/test/java/org/libremediaconverter/work/WorkerStubs.kt +++ b/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 { + 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) +}