diff --git a/CLAUDE.md b/CLAUDE.md index c8282ab..fd473ed 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -130,8 +130,9 @@ install for code that can never run — and on API 37 the full APK does not fit - The `model` package is excluded from `ReturnCount` and `CyclomaticComplexMethod` only. It is the decision layer, where one branch is one documented user-visible outcome and the metric counts answers rather than complexity. Every other rule still applies there. -- **Coverage is reported, not gated** — **84.9% of lines (1971/2321), 63.8% of branches**, - measured 2026-08-26 with `./gradlew :app:jacocoTestReport`, against 454 JVM tests in 67 classes. +- **Coverage is reported, not gated** — **87.1% of lines (2025/2324), 69.1% of branches + (974/1410)**, measured 2026-08-27 with `./gradlew :app:jacocoTestReport`, against 502 JVM tests + in 71 classes. **Every figure this file carried before 2026-08-24 was an artifact, roughly half the real one.** Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo @@ -147,12 +148,19 @@ install for code that can never run — and on API 37 the full APK does not fit disproportionately Robolectric, so each one added denominator and no numerator — the measurement was punishing exactly the tests that were hardest to write. - Two things still hold. A floor needs a baseline that has settled, and this one has not: it moved - 39 points in a single build change on 2026-08-24, then another 16 as the #52 test push and the + Two things still hold. A floor needs a baseline that has settled, and this one has not. It moved + 39 points in a single build change on 2026-08-24; then another 16 as the #52 test push and the fixes it turned up landed — 69.2% -> 84.9% line, 53.2% -> 63.8% branch — while the denominator - grew 2194 -> 2321, because that work added production code of its own. And **re-measure before - quoting**: this entry was written quoting 81.4%, measured four hours earlier, and was already - three points stale by the time it was ready to merge. + grew 2194 -> 2321, because that work added production code of its own; then again on 2026-08-27 + as #132 and #133's ten children landed — 84.9% -> 87.1% line, 63.8% -> **69.1%** branch, 454 -> + 502 tests. **Branch moved four times as far as line that last time**, and that is the shape to + expect from this kind of work rather than a surprise: those children targeted decision code — + enum fallbacks, refusal arms, cursor shapes, a `when` over container rules — where one test + chooses a branch the suite had never taken. Line coverage barely notices; branch coverage is the + whole point. + + And **re-measure before quoting**: this entry was once written quoting 81.4%, measured four hours + earlier, and was already three points stale by the time it was ready to merge. - **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit tests before it counts as done. If it is e2e testable, it gets e2e tests. Both clauses apply — a change that is both needs both. diff --git a/app/src/main/java/org/libremediaconverter/convert/MediaProbe.kt b/app/src/main/java/org/libremediaconverter/convert/MediaProbe.kt index de4ba3d..d878163 100644 --- a/app/src/main/java/org/libremediaconverter/convert/MediaProbe.kt +++ b/app/src/main/java/org/libremediaconverter/convert/MediaProbe.kt @@ -92,7 +92,12 @@ object MediaProbe { else -> InputKind.UNPARSEABLE } - private class Extracted( + /** + * `internal` rather than `private` so [extractedFrom] can be named from a test. The JVM test + * source set is a friend of `main`, so this stays invisible outside the module — the precedent + * is `MainActivity`'s `Destination`, and [containerFrom] beside it. + */ + internal class Extracted( val videoCodec: String?, val audioCodec: String?, val durationMs: Long, @@ -100,33 +105,55 @@ object MediaProbe { val height: Int, ) + /** + * What a set of track formats says about a file. + * + * Split out of [probeWithExtractor] so the rules below can be tested against tracks a test + * *chooses*, rather than against whatever the committed fixtures happen to contain. The device + * tests exercise this through real files; none of them can construct a two-video-track input, + * a track that omits its duration, or an audio-before-video ordering on purpose. + * + * Three rules live here, and each is a decision rather than plumbing: + * + * - **First track of a type wins.** `video == null` is the whole guard. A file with two video + * tracks must report the first, because that is the one an engine will transcode. + * - **Duration is the maximum across tracks**, not the first one found or the last. A file + * whose audio outlasts its video is ordinary, and reporting the video's length would cut the + * progress bar short. + * - **A track that omits `KEY_DURATION` contributes nothing** rather than zero. `MediaExtractor` + * omits it for plenty of real tracks — see `MediaProbeTrackFieldsTest` — and `maxOf` against a + * fabricated 0 would still be correct here, but reading a key that is absent is not. + */ + internal fun extractedFrom(formats: List): Extracted { + var video: String? = null + var audio: String? = null + var durationUs = 0L + var width = 0 + var height = 0 + + for (format in formats) { + val mime = format.getString(MediaFormat.KEY_MIME).orEmpty() + if (format.containsKey(MediaFormat.KEY_DURATION)) { + durationUs = maxOf(durationUs, format.getLong(MediaFormat.KEY_DURATION)) + } + when { + mime.startsWith("video/") && video == null -> { + video = shortName(mime) + width = format.intOr(MediaFormat.KEY_WIDTH) + height = format.intOr(MediaFormat.KEY_HEIGHT) + } + + mime.startsWith("audio/") && audio == null -> audio = shortName(mime) + } + } + return Extracted(video, audio, durationUs / US_PER_MS, width, height) + } + private fun probeWithExtractor(context: Context, uri: Uri): Extracted? { val extractor = MediaExtractor() return try { extractor.setDataSource(context, uri, null) - var video: String? = null - var audio: String? = null - var durationUs = 0L - var width = 0 - var height = 0 - - for (i in 0 until extractor.trackCount) { - val format = extractor.getTrackFormat(i) - val mime = format.getString(MediaFormat.KEY_MIME).orEmpty() - if (format.containsKey(MediaFormat.KEY_DURATION)) { - durationUs = maxOf(durationUs, format.getLong(MediaFormat.KEY_DURATION)) - } - when { - mime.startsWith("video/") && video == null -> { - video = shortName(mime) - width = format.intOr(MediaFormat.KEY_WIDTH) - height = format.intOr(MediaFormat.KEY_HEIGHT) - } - - mime.startsWith("audio/") && audio == null -> audio = shortName(mime) - } - } - Extracted(video, audio, durationUs / US_PER_MS, width, height) + extractedFrom(extractor.trackFormats()) } catch (e: Exception) { Log.i(TAG, "Platform extractor could not read $uri.", e) null @@ -269,25 +296,7 @@ object MediaProbe { val extractor = MediaExtractor() return try { extractor.setDataSource(context, uri, null) - var video: String? = null - var audio: String? = null - var width = 0 - var height = 0 - var fps = 0 - - for (i in 0 until extractor.trackCount) { - val format = extractor.getTrackFormat(i) - val mime = format.getString(MediaFormat.KEY_MIME).orEmpty() - if (mime.startsWith("video/") && video == null) { - video = shortName(mime) - width = format.intOr(MediaFormat.KEY_WIDTH) - height = format.intOr(MediaFormat.KEY_HEIGHT) - fps = format.intOr(MediaFormat.KEY_FRAME_RATE) - } else if (mime.startsWith("audio/") && audio == null) { - audio = shortName(mime) - } - } - ConcatInput(video, audio, width, height, fps) + concatInputFrom(extractor.trackFormats()) } catch (e: Exception) { Log.i(TAG, "Could not probe $uri for concat; will re-encode.", e) ConcatInput(null, null, 0, 0, 0) @@ -296,6 +305,45 @@ object MediaProbe { } } + /** + * The join flow's read of the same track formats. See [extractedFrom] for why this is separate + * from the extractor. + * + * Deliberately **not** folded into [extractedFrom] despite the overlap. This one reads frame + * rate and does not read duration; that one reads duration and does not read frame rate. A + * merged version would have to compute both for every caller, and `ConcatPlanner` treats an + * unknown frame rate as "cannot prove a match" — so a field this flow does not need must not + * start arriving as a number. + */ + internal fun concatInputFrom(formats: List): ConcatInput { + var video: String? = null + var audio: String? = null + var width = 0 + var height = 0 + var fps = 0 + + for (format in formats) { + val mime = format.getString(MediaFormat.KEY_MIME).orEmpty() + if (mime.startsWith("video/") && video == null) { + video = shortName(mime) + width = format.intOr(MediaFormat.KEY_WIDTH) + height = format.intOr(MediaFormat.KEY_HEIGHT) + fps = format.intOr(MediaFormat.KEY_FRAME_RATE) + } else if (mime.startsWith("audio/") && audio == null) { + audio = shortName(mime) + } + } + return ConcatInput(video, audio, width, height, fps) + } + + /** + * Every track format this extractor holds, read once. + * + * The thin edge the two pure functions above leave behind: a `trackCount` and a + * `getTrackFormat` per index, which is the whole of what needs a real `MediaExtractor`. + */ + private fun MediaExtractor.trackFormats(): List = (0 until trackCount).map(::getTrackFormat) + /** * One track property as an Int, or [fallback] when the format has no Int to give. * diff --git a/app/src/test/java/org/libremediaconverter/convert/MediaProbeTrackWalkTest.kt b/app/src/test/java/org/libremediaconverter/convert/MediaProbeTrackWalkTest.kt new file mode 100644 index 0000000..f7fbabb --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/convert/MediaProbeTrackWalkTest.kt @@ -0,0 +1,221 @@ +package org.libremediaconverter.convert + +import android.media.MediaFormat +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * The rules `MediaProbe` applies to a set of track formats. + * + * ## Why this exists, and what it revises + * + * Issue #84 classified `probeWithExtractor` and `probeForConcat` as device-bound and explicitly not + * a gap: + * + * > These are exercised by `RemuxTest`, `ConcatEngineTest` and `RealMediaBenchmark` in + * > `androidTest` … **Do not read their 0% as untested.** + * + * That was right about the measurement boundary and right about FFprobe. It was not right that + * these are only orchestration. The track walk is a **branch matrix**, and `androidTest` reaches it + * only through whatever the committed fixtures happen to contain — so none of the rules below is + * *chosen* by any test there. A fixture with two video tracks, a track that omits its duration, or + * an audio-before-video ordering is not something a device test would produce on purpose. + * + * The seam is the answer #133 preferred over driving `ShadowMediaExtractor`: the walk is a pure + * function over `List`, and what is left needing a device — `setDataSource`, + * `getTrackFormat`, `release` — is the thin edge `androidTest` should be covering. This is the + * `work/FailureOutcome.kt` pattern `CLAUDE.md` names. + * + * `MediaFormat` is a real one throughout, not a stub. `MediaProbeTrackFieldsTest` records why that + * matters: it is a heterogeneous map whose getters throw rather than coerce, and a hand-rolled + * double would not reproduce that. + */ +@RunWith(RobolectricTestRunner::class) +class MediaProbeTrackWalkTest { + + // --- extractedFrom: the conversion flow's read --------------------------- + + @Test + fun `the first video track wins when a file carries two`() { + // `video == null` is the entire guard. A file with two video tracks must report the first, + // because that is the one an engine will transcode -- and the width and height must come + // from the same track, not be mixed across them. + val extracted = MediaProbe.extractedFrom( + listOf( + video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1920, height = 1080), + video(MediaFormat.MIMETYPE_VIDEO_HEVC, width = 640, height = 480), + ), + ) + + assertEquals("h264", extracted.videoCodec) + assertEquals(1920, extracted.width) + assertEquals(1080, extracted.height) + } + + @Test + fun `the first audio track wins when a file carries two`() { + val extracted = MediaProbe.extractedFrom( + listOf( + audio(MediaFormat.MIMETYPE_AUDIO_AAC), + audio(MediaFormat.MIMETYPE_AUDIO_OPUS), + ), + ) + + assertEquals("aac", extracted.audioCodec) + } + + @Test + fun `duration is the longest track, not the first or the last`() { + // A file whose audio outlasts its video is ordinary. Taking the video's length would cut + // the progress bar short; taking the last track's would be right only by accident of order. + val extracted = MediaProbe.extractedFrom( + listOf( + video(MediaFormat.MIMETYPE_VIDEO_AVC, durationUs = 10_000_000), + audio(MediaFormat.MIMETYPE_AUDIO_AAC, durationUs = 12_500_000), + audio(MediaFormat.MIMETYPE_AUDIO_OPUS, durationUs = 1_000_000), + ), + ) + + assertEquals(12_500L, extracted.durationMs) + } + + @Test + fun `a track that does not declare its duration contributes nothing to it`() { + // MediaExtractor omits KEY_DURATION for plenty of real tracks -- MediaProbeTrackFieldsTest + // records the same for KEY_FRAME_RATE. Reading a key that is absent is what containsKey + // stands between us and. + val extracted = MediaProbe.extractedFrom( + listOf( + video(MediaFormat.MIMETYPE_VIDEO_AVC), + audio(MediaFormat.MIMETYPE_AUDIO_AAC, durationUs = 7_000_000), + ), + ) + + assertEquals(7_000L, extracted.durationMs) + } + + @Test + fun `declaring audio before video changes nothing`() { + // Track order is a property of the container, not of the content. Both orderings have to + // reach the same answer or the same file remuxed twice would probe differently. + val videoFirst = MediaProbe.extractedFrom( + listOf( + video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1280, height = 720), + audio(MediaFormat.MIMETYPE_AUDIO_AAC), + ), + ) + val audioFirst = MediaProbe.extractedFrom( + listOf( + audio(MediaFormat.MIMETYPE_AUDIO_AAC), + video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1280, height = 720), + ), + ) + + assertEquals(videoFirst.videoCodec, audioFirst.videoCodec) + assertEquals(videoFirst.audioCodec, audioFirst.audioCodec) + assertEquals(videoFirst.width, audioFirst.width) + assertEquals(videoFirst.height, audioFirst.height) + } + + @Test + fun `a track that is neither audio nor video is ignored`() { + // Subtitle and timed-metadata tracks are common in MKV and MP4. Neither prefix matches, so + // neither slot is filled -- and, importantly, a subtitle track must not be mistaken for the + // absence of an audio track by some later `else`. + val extracted = MediaProbe.extractedFrom( + listOf( + MediaFormat().apply { setString(MediaFormat.KEY_MIME, "text/vtt") }, + video(MediaFormat.MIMETYPE_VIDEO_AVC), + ), + ) + + assertEquals("h264", extracted.videoCodec) + assertNull(extracted.audioCodec) + } + + @Test + fun `a file with no tracks reports nothing rather than zero-width video`() { + val extracted = MediaProbe.extractedFrom(emptyList()) + + assertNull(extracted.videoCodec) + assertNull(extracted.audioCodec) + assertEquals(0L, extracted.durationMs) + assertEquals(0, extracted.width) + assertEquals(0, extracted.height) + } + + @Test + fun `an audio-only file reports no video codec at all`() { + // The distinction MediaProbe.classify turns into InputKind.AUDIO_ONLY, and the reason + // `hasVideo` exists: an audio file and a corrupt file must not look alike. + val extracted = MediaProbe.extractedFrom(listOf(audio(MediaFormat.MIMETYPE_AUDIO_AAC))) + + assertNull(extracted.videoCodec) + assertEquals("aac", extracted.audioCodec) + assertEquals(0, extracted.width) + } + + // --- concatInputFrom: the join flow's read ------------------------------- + + @Test + fun `the join read takes frame rate from the first video track`() { + val input = MediaProbe.concatInputFrom( + listOf( + video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1920, height = 1080, frameRate = 30), + video(MediaFormat.MIMETYPE_VIDEO_HEVC, width = 640, height = 480, frameRate = 60), + audio(MediaFormat.MIMETYPE_AUDIO_AAC), + ), + ) + + assertEquals("h264", input.videoCodec) + assertEquals("aac", input.audioCodec) + assertEquals(1920, input.width) + assertEquals(1080, input.height) + assertEquals(30, input.frameRate) + } + + @Test + fun `a video track with no declared frame rate reports zero rather than guessing`() { + // ConcatPlanner treats 0 as "cannot prove a match" and re-encodes. A guessed 30 would read + // as agreement and produce a stream copy of clips that do not actually match -- the failure + // its KDoc says the whole flow is arranged to avoid. + val input = MediaProbe.concatInputFrom(listOf(video(MediaFormat.MIMETYPE_VIDEO_AVC))) + + assertEquals(0, input.frameRate) + } + + @Test + fun `a file with no tracks joins as entirely unknown`() { + val input = MediaProbe.concatInputFrom(emptyList()) + + assertNull(input.videoCodec) + assertNull(input.audioCodec) + assertEquals(0, input.width) + assertEquals(0, input.height) + assertEquals(0, input.frameRate) + } + + private fun video( + mime: String, + width: Int = 1920, + height: Int = 1080, + durationUs: Long? = null, + frameRate: Int? = null, + ): MediaFormat = MediaFormat.createVideoFormat(mime, width, height).apply { + durationUs?.let { setLong(MediaFormat.KEY_DURATION, it) } + frameRate?.let { setInteger(MediaFormat.KEY_FRAME_RATE, it) } + } + + private fun audio(mime: String, durationUs: Long? = null): MediaFormat = + MediaFormat.createAudioFormat(mime, SAMPLE_RATE, CHANNELS).apply { + durationUs?.let { setLong(MediaFormat.KEY_DURATION, it) } + } + + private companion object { + const val SAMPLE_RATE = 48_000 + const val CHANNELS = 2 + } +} diff --git a/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt b/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt index daae30b..30a44e5 100644 --- a/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt +++ b/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt @@ -358,4 +358,120 @@ class ContainerCapabilitiesTest { assertEquals(emptyList(), ContainerCapabilities.encodableVideo(container)) } } + + // --- the audio axis ----------------------------------------------------- + // + // Every rule below has a video twin already tested above. The two halves of `validate` were + // written together and only one of them was ever checked, so these are deliberately shaped like + // their twins rather than as a fresh idea about what to assert. + + @Test + fun `an unidentifiable source audio codec cannot be copied`() { + // The audio twin of `an unidentifiable source codec cannot be copied`. Never guess: a copy + // of an unidentified codec is how you ship a file that does not play. + val unknownAudio = InputProbe(videoCodec = "h264", audioCodec = null, container = Container.MP4) + val spec = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.COPY) + + val invalid = ContainerCapabilities.validate(spec, unknownAudio) as? Validation.Invalid + ?: throw AssertionError("copying an unidentified audio codec must be refused") + + assertTrue(invalid.message, invalid.message.contains("could not be identified")) + assertEverySuggestionValid(invalid, unknownAudio) + } + + @Test + fun `copying an audio codec the container cannot hold is refused`() { + // MP4 carries AAC, MP3, Opus and FLAC. Vorbis lives in Ogg and Matroska, so a stream copy + // out of a Vorbis source into MP4 has nowhere to put the track. + val vorbisAudio = InputProbe(videoCodec = "h264", audioCodec = "vorbis", container = Container.MKV) + val spec = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.COPY) + + val invalid = ContainerCapabilities.validate(spec, vorbisAudio) as? Validation.Invalid + ?: throw AssertionError("Vorbis copied into MP4 must be refused") + + assertEquals("MP4 cannot hold Vorbis audio.", invalid.message) + assertEverySuggestionValid(invalid, vorbisAudio) + } + + @Test + fun `an audio codec the container cannot hold is refused on the encode path too`() { + // WAV carries PCM and nothing else. The twin is `H265 in AVI is refused`. + val spec = OutputSpec(Container.WAV, VideoCodec.NONE, AudioCodec.AAC) + + val invalid = ContainerCapabilities.validate(spec, mp3Source) as? Validation.Invalid + ?: throw AssertionError("AAC in WAV must be refused") + + assertEquals("WAV cannot hold AAC audio.", invalid.message) + assertEverySuggestionValid(invalid, mp3Source) + } + + @Test + fun `an audio codec this app cannot encode is refused, and copying is offered instead`() { + // Matroska carries Vorbis; nothing here encodes it. The refusal has to say so *and* say + // what would work, which is the audio twin of `copying is offered as the fix when the codec + // is right but unencodable`. + val spec = OutputSpec(Container.MKV, VideoCodec.H264, AudioCodec.VORBIS) + + val invalid = ContainerCapabilities.validate(spec, h264Source) as? Validation.Invalid + ?: throw AssertionError("encoding Vorbis must be refused") + + assertEquals( + "This app cannot encode Vorbis audio. It can still be copied from a Vorbis source.", + invalid.message, + ) + assertEverySuggestionValid(invalid, h264Source) + } + + @Test + fun `copying a video codec the container cannot hold is refused`() { + // Not the audio axis, but the one video refusal with no test: AVI predates H.265, so a + // stream copy out of an HEVC source into AVI has nowhere to put the track. `H265 in AVI is + // refused` covers the matrix; this covers what validate() does with it. + val h265Source = InputProbe(videoCodec = "hevc", audioCodec = "mp3", container = Container.MP4) + val spec = OutputSpec(Container.AVI, VideoCodec.COPY, AudioCodec.MP3) + + val invalid = ContainerCapabilities.validate(spec, h265Source) as? Validation.Invalid + ?: throw AssertionError("H.265 copied into AVI must be refused") + + assertEquals("AVI cannot hold H.265 video.", invalid.message) + assertEverySuggestionValid(invalid, h265Source) + } + + @Test + fun `no audio track is accepted by every container in both modes`() { + // The audio twin of VideoCodec.NONE -> true. A container that refused "no audio" would make + // every video-only output invalid. + Container.entries.forEach { container -> + listOf(CodecMode.COPY, CodecMode.ENCODE).forEach { mode -> + assertTrue( + "$container should accept no audio track ($mode)", + ContainerCapabilities.accepts(container, AudioCodec.NONE, mode), + ) + } + } + } + + @Test + fun `resolving audio COPY before asking the matrix is required`() { + // The audio twin of `resolving COPY before asking the matrix is required`, and the reason is + // identical: silently answering "false" would refuse a perfectly good remux. + runCatching { ContainerCapabilities.accepts(Container.MP4, AudioCodec.COPY, CodecMode.COPY) } + .onSuccess { throw AssertionError("expected audio COPY to be rejected by the matrix") } + } + + /** + * Every alternative a refusal offers has to be one the same input could actually take. + * + * `Validation.Invalid` promises exactly this and names this class as the proof. The global + * property test walks the presets; these paths reach `suggestions()` through `validateAudio`, + * which no preset does. + */ + private fun assertEverySuggestionValid(invalid: Validation.Invalid, probe: InputProbe) { + invalid.suggestions.forEach { + assertTrue( + "suggestion $it is itself invalid, so the chip leads to a second error", + ContainerCapabilities.validate(it, probe).isValid, + ) + } + } } 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/RefusedJobTest.kt b/app/src/test/java/org/libremediaconverter/work/RefusedJobTest.kt new file mode 100644 index 0000000..f988c06 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/work/RefusedJobTest.kt @@ -0,0 +1,276 @@ +package org.libremediaconverter.work + +import android.app.Application +import android.net.Uri +import androidx.media3.common.util.UnstableApi +import androidx.work.Data +import androidx.work.ListenableWorker +import androidx.work.testing.TestListenableWorkerBuilder +import androidx.work.workDataOf +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.libremediaconverter.convert.ConversionDependencies +import org.libremediaconverter.convert.OutputPublisher +import org.libremediaconverter.convert.SoftwareTranscoder +import org.libremediaconverter.convert.installTestWorkManager +import org.libremediaconverter.model.AudioCodec +import org.libremediaconverter.model.ContainerCapabilities +import org.libremediaconverter.model.ConversionRequest +import org.libremediaconverter.model.DeviceCodecs +import org.libremediaconverter.model.EnginePreference +import org.libremediaconverter.model.InputProbe +import org.libremediaconverter.model.OutputFormat +import org.libremediaconverter.model.OutputSpec +import org.libremediaconverter.model.Validation +import org.libremediaconverter.model.VideoCodec +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import java.io.File +import java.util.UUID + +/** + * Jobs the worker refuses before it converts anything, and what it says about them. + * + * Two exits, both cold before this file, and both reachable for the same underlying reason: **a job + * does not have to come from the picker.** WorkManager keeps queued and finished work for about a + * week, so a downgrade or a rollback hands this build a job enqueued by another one — the premise + * `WorkerEnumFallbackTest` and `JobTags` are both written on — and `ConversionWorker.request(...)` + * is callable directly. + * + * What makes these worth their own file rather than another case in an existing one is that both + * are about the *message*. A refusal that fails with empty output `Data` renders the UI's generic + * "Conversion failed." with nothing else to say, which is the defect shape `DeniedForegroundStartTest` + * records from the device pass. Asserting the verdict alone would pass against exactly that. + */ +@UnstableApi +@RunWith(RobolectricTestRunner::class) +class RefusedJobTest { + + private lateinit var app: Application + private lateinit var publisher: OutputPublisher + private lateinit var engine: RefusingTranscoder + + @Before + fun setUp() { + app = RuntimeEnvironment.getApplication() + publisher = AlwaysRoomPublisher(app) + engine = RefusingTranscoder() + ConversionDependencies.publisher = { publisher } + ConversionDependencies.software = { engine } + // Neither test is about probing or about this machine's codecs; both would otherwise decide + // the outcome for reasons no assertion mentions. See WorkerCancellationTest's setUp. + ConversionDependencies.probe = { _, _ -> InputProbe() } + ConversionDependencies.deviceCodecs = { DeviceCodecs.PERMISSIVE } + installTestWorkManager(app, Data.EMPTY) + } + + @After + fun tearDown() { + ConversionDependencies.reset() + } + + @Test + fun `a job with no input URI fails with a message rather than a bare failure`() { + val result = runBlocking { workerWithout(ConversionWorker.KEY_INPUT_URI).doWork() } + + // `Failure.equals` compares output data, so this pins the message and the verdict together. + assertEquals( + ListenableWorker.Result.failure(workDataOf(ConversionWorker.KEY_ERROR to "No input file.")), + result, + ) + } + + @Test + fun `a job with no input URI stages nothing`() { + // The URI read is the first thing doWork does -- above the space check, above the staging + // name, above the try. A refusal there must not have reserved anything. + runBlocking { workerWithout(ConversionWorker.KEY_INPUT_URI).doWork() } + + assertEquals("a job refused for having no input must not stage a file", emptyList(), stagedNames()) + } + + @Test + fun `a spec the picker would never have allowed is refused with the reason`() { + // WAV carries PCM and nothing else. The picker cannot produce this combination today, which + // is exactly why the worker checks: the job can arrive from a queue written before the + // settings changed, or from a direct request(...) call. + val expected = ContainerCapabilities.validate(REFUSED_SPEC, InputProbe()) as? Validation.Invalid + ?: throw AssertionError("the fixture spec is supposed to be invalid; ContainerCapabilities disagrees") + + val result = runBlocking { worker(REFUSED_SPEC).doWork() } + + assertEquals( + ListenableWorker.Result.failure(workDataOf(ConversionWorker.KEY_ERROR to expected.message)), + result, + ) + } + + @Test + fun `a refused spec never reaches an engine`() { + // The half that says it failed *before* converting rather than during. Without this, a + // worker that ran the job and then reported the validation message would pass the test + // above -- and would have spent the user's battery on a file it was going to refuse. + runBlocking { worker(REFUSED_SPEC).doWork() } + + assertTrue("a refused spec must be refused before any engine runs", engine.invocations.isEmpty()) + } + + @Test + fun `a valid spec is not refused`() { + // The control. Every assertion above is about a refusal, so without this they would all + // still pass against a worker that refused everything. + val result = runBlocking { worker(OutputFormat.MP4_H265.spec).doWork() } + + assertEquals(ListenableWorker.Result.success(), stripOutput(result)) + assertEquals(listOf(OutputFormat.MP4_H265.spec), engine.invocations) + } + + // --- the same refusal, on the join side ---------------------------------- + + @Test + fun `a join of a single file is refused with a message rather than joined`() { + // The arm beside it -- a job with no URI array at all -- is covered on the device by + // `UnopenableUriTest.aJoinWithNoInputArrayFailsWithAMessage`. This one was covered by + // nothing in either source set, which a coverage report cannot say because it cannot see + // androidTest: the two arms are adjacent lines and only one of them had a test. + // + // Reachable for the reason this file's header gives, plus one of its own: `request(...)` + // takes a `List` and checks nothing about its length, so a single-item join is a + // well-formed call, not a corrupted queue entry. + val result = runBlocking { joinWorker(INPUT).doWork() } + + assertEquals( + ListenableWorker.Result.failure( + workDataOf(ConcatWorker.KEY_ERROR to "Pick at least two files to join."), + ), + result, + ) + } + + @Test + fun `a join of two files is not refused for its count`() { + // The control, and the half that makes the test above bite on the boundary rather than on + // the message: without it, `uris.size < 3` passes everything here. + // + // It refuses the space instead of letting the job run, because the next thing past the + // count guard is `ConcatEngine`, which is native -- `NamingPublisher`'s KDoc records that + // no JVM test gets past it. A refusal with the *space* message is proof that execution + // reached line 57, which is proof it got past line 42, and it costs no engine to say so. + val noRoom = NamingPublisher(app).apply { refuseSpace = true } + ConversionDependencies.publisher = { noRoom } + + val result = runBlocking { joinWorker(INPUT, SECOND_INPUT).doWork() } + + assertEquals( + ListenableWorker.Result.failure( + workDataOf(ConcatWorker.KEY_ERROR to "Not enough free space to join these files."), + ), + result, + ) + } + + /** [ListenableWorker.Result.Success] compares its output data, which these tests do not pin. */ + private fun stripOutput(result: ListenableWorker.Result): ListenableWorker.Result = + if (result is ListenableWorker.Result.Success) ListenableWorker.Result.success() else result + + private fun worker(spec: OutputSpec): ConversionWorker = build( + workDataOf( + ConversionWorker.KEY_INPUT_URI to INPUT.toString(), + ConversionWorker.KEY_DISPLAY_NAME to DISPLAY_NAME, + ConversionWorker.KEY_SIZE_BYTES to INPUT_BYTES, + ConversionWorker.KEY_CONTAINER to spec.container.name, + ConversionWorker.KEY_VIDEO_CODEC to spec.videoCodec.name, + ConversionWorker.KEY_AUDIO_CODEC to spec.audioCodec.name, + ConversionWorker.KEY_ENGINE_PREFERENCE to EnginePreference.FORCE_SOFTWARE.name, + ), + ) + + /** + * The ordinary input `Data`, less one key. + * + * Built by removal rather than by spelling out a shorter map, so the test cannot drift into + * omitting something else as well and passing for a reason it does not name. + */ + private fun workerWithout(key: String): ConversionWorker { + val full = OutputFormat.MP4_H265.spec + val entries = mapOf( + ConversionWorker.KEY_INPUT_URI to INPUT.toString(), + ConversionWorker.KEY_DISPLAY_NAME to DISPLAY_NAME, + ConversionWorker.KEY_SIZE_BYTES to INPUT_BYTES, + ConversionWorker.KEY_CONTAINER to full.container.name, + ConversionWorker.KEY_VIDEO_CODEC to full.videoCodec.name, + ConversionWorker.KEY_AUDIO_CODEC to full.audioCodec.name, + ConversionWorker.KEY_ENGINE_PREFERENCE to EnginePreference.FORCE_SOFTWARE.name, + ) - key + return build(Data.Builder().putAll(entries).build()) + } + + private fun build(data: Data): ConversionWorker = + TestListenableWorkerBuilder(context = app, inputData = data, runAttemptCount = 0) + .setId(JOB_ID) + .build() + + /** + * A join job carrying [inputs], a declared total, and a format. + * + * The total is declared so `hasRoomFor` takes its `hasSpaceFor` branch: the other branch is + * `hasSpaceForUnknownSize`, which `NamingPublisher` does not override and which would measure + * this machine's real disk. + */ + private fun joinWorker(vararg inputs: Uri): ConcatWorker = TestListenableWorkerBuilder( + context = app, + inputData = workDataOf( + ConcatWorker.KEY_INPUT_URIS to inputs.map(Uri::toString).toTypedArray(), + ConcatWorker.KEY_TOTAL_BYTES to INPUT_BYTES * inputs.size, + ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name, + ), + runAttemptCount = 0, + ).setId(JOB_ID).build() + + private fun stagedNames(): List = + publisher.createStagingFile("anything").parentFile?.listFiles().orEmpty().map { it.name }.sorted() + + private companion object { + val INPUT: Uri = Uri.parse("file:///tmp/holiday.mp4") + const val DISPLAY_NAME = "holiday.mp4" + const val INPUT_BYTES = 1024L + + /** A join needs two, and "two" is the boundary the count guard is about. */ + val SECOND_INPUT: Uri = Uri.parse("file:///tmp/holiday-2.mp4") + + /** WAV carries PCM and nothing else, so AAC in WAV has nowhere to go. */ + val REFUSED_SPEC = OutputSpec( + org.libremediaconverter.model.Container.WAV, + VideoCodec.NONE, + AudioCodec.AAC, + ) + val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000005") + } +} + +/** An engine that records what it was asked for and writes an output, so a success is a success. */ +private class RefusingTranscoder : SoftwareTranscoder { + + /** Every spec that actually reached an engine. Empty is the assertion for a refused job. */ + val invocations = mutableListOf() + + override suspend fun run( + request: ConversionRequest, + inputPath: String, + output: File, + durationMs: Long, + onProgress: (Int) -> Unit, + ) { + invocations += request.spec + output.writeBytes(ByteArray(OUTPUT_BYTES)) + } + + private companion object { + const val OUTPUT_BYTES = 512 + } +} 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/WorkerEnumFallbackTest.kt b/app/src/test/java/org/libremediaconverter/work/WorkerEnumFallbackTest.kt index 32cea02..ce32a1e 100644 --- a/app/src/test/java/org/libremediaconverter/work/WorkerEnumFallbackTest.kt +++ b/app/src/test/java/org/libremediaconverter/work/WorkerEnumFallbackTest.kt @@ -22,6 +22,7 @@ import org.libremediaconverter.model.DeviceCodecs import org.libremediaconverter.model.EnginePreference import org.libremediaconverter.model.InputProbe import org.libremediaconverter.model.OutputFormat +import org.libremediaconverter.model.OutputSpec import org.libremediaconverter.model.QualityTier import org.robolectric.RobolectricTestRunner import org.robolectric.RuntimeEnvironment @@ -109,6 +110,50 @@ class WorkerEnumFallbackTest { ) } + @Test + fun `a container this build does not define falls back to the default spec`() { + assertFallsBackToDefault(container = "HOLOTAPE") + } + + @Test + fun `a video codec this build does not define falls back to the default spec`() { + assertFallsBackToDefault(video = "H267") + } + + @Test + fun `an audio codec this build does not define falls back to the default spec`() { + assertFallsBackToDefault(audio = "SUPER_AAC") + } + + /** + * Drives a job whose spec is [NOT_THE_FALLBACK] on every axis but the one named, and asserts the + * whole spec came back as [DEFAULT_SPEC]. + * + * **The baseline is the point.** `readSpec` returns the *entire* fallback spec the moment any + * one axis fails to resolve, so a test starting from `MP4_H265` -- which is itself the fallback + * -- could not tell a worker that read the spec correctly from one that gave up on it. Starting + * from MKV/H.264 makes the difference visible on two axes at once. + * + * Asserting the spec that *ran*, rather than only that a `Result` came back, is the other half: + * the defect these three are written for threw out of `doWork` entirely, so "a Result at all" + * would pass against a fallback to something arbitrary. + */ + private fun assertFallsBackToDefault( + container: String = NOT_THE_FALLBACK.container.name, + video: String = NOT_THE_FALLBACK.videoCodec.name, + audio: String = NOT_THE_FALLBACK.audioCodec.name, + ) { + val transcoder = RequestRecordingTranscoder() + ConversionDependencies.software = { transcoder } + + val result = runBlocking { + conversionWorker(container = container, video = video, audio = audio).doWork() + } + + assertEquals(ListenableWorker.Result.success(), stripOutput(result)) + assertEquals(listOf(DEFAULT_SPEC), transcoder.specs) + } + /** [ListenableWorker.Result.Success] compares its output data, which these tests do not pin. */ private fun stripOutput(result: ListenableWorker.Result): ListenableWorker.Result = if (result is ListenableWorker.Result.Success) ListenableWorker.Result.success() else result @@ -116,15 +161,18 @@ class WorkerEnumFallbackTest { private fun conversionWorker( quality: String = QualityTier.FAST.name, preference: String = EnginePreference.FORCE_SOFTWARE.name, + container: String = SPEC.container.name, + video: String = SPEC.videoCodec.name, + audio: String = SPEC.audioCodec.name, ): ConversionWorker = TestListenableWorkerBuilder( context = app, inputData = workDataOf( ConversionWorker.KEY_INPUT_URI to INPUT.toString(), ConversionWorker.KEY_DISPLAY_NAME to DISPLAY_NAME, ConversionWorker.KEY_SIZE_BYTES to INPUT_BYTES, - ConversionWorker.KEY_CONTAINER to SPEC.container.name, - ConversionWorker.KEY_VIDEO_CODEC to SPEC.videoCodec.name, - ConversionWorker.KEY_AUDIO_CODEC to SPEC.audioCodec.name, + ConversionWorker.KEY_CONTAINER to container, + ConversionWorker.KEY_VIDEO_CODEC to video, + ConversionWorker.KEY_AUDIO_CODEC to audio, ConversionWorker.KEY_QUALITY to quality, ConversionWorker.KEY_ENGINE_PREFERENCE to preference, ), @@ -146,6 +194,12 @@ class WorkerEnumFallbackTest { const val DISPLAY_NAME = "holiday.mp4" const val INPUT_BYTES = 1024L val SPEC = OutputFormat.MP4_H265.spec + + /** What `readSpec` returns when any axis fails to resolve. */ + val DEFAULT_SPEC = OutputFormat.MP4_H265.spec + + /** A spec that differs from [DEFAULT_SPEC] on container *and* video codec. See the helper. */ + val NOT_THE_FALLBACK = OutputFormat.MKV_H264.spec val CONVERSION_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000021") val CONCAT_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000022") } @@ -156,6 +210,9 @@ private class RequestRecordingTranscoder : SoftwareTranscoder { val qualities = mutableListOf() + /** The spec each run was asked for. Which one ran is what the three readSpec tests assert. */ + val specs = mutableListOf() + override suspend fun run( request: ConversionRequest, inputPath: String, @@ -164,6 +221,7 @@ private class RequestRecordingTranscoder : SoftwareTranscoder { onProgress: (Int) -> Unit, ) { qualities += request.quality + specs += request.spec output.writeBytes(ByteArray(OUTPUT_BYTES)) } 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) +}