Compare commits
18
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
795b456d94 | ||
|
|
2c0bc4a583 | ||
|
|
7a47285f37 | ||
|
|
8a2bc86cac | ||
|
|
9b3b9f952b | ||
|
|
0e2525195b | ||
|
|
713d813a65 | ||
|
|
c360e82a10 | ||
|
|
699d608b47 | ||
|
|
ad47ce6c96 | ||
|
|
c60d5d54c6 | ||
|
|
d59e9acce5 | ||
|
|
324c9a4555 | ||
|
|
44493d9943 | ||
|
|
de6d9526ba | ||
|
|
bb3358f209 | ||
|
|
04850a0415 | ||
|
|
8ab433b647 |
@@ -5,6 +5,7 @@ import android.net.Uri
|
|||||||
import android.provider.DocumentsContract
|
import android.provider.DocumentsContract
|
||||||
import android.provider.OpenableColumns
|
import android.provider.OpenableColumns
|
||||||
import java.io.File
|
import java.io.File
|
||||||
|
import java.io.OutputStream
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* What a save has to say when the staged file is not there any more.
|
* What a save has to say when the staged file is not there any more.
|
||||||
@@ -170,7 +171,7 @@ open class OutputPublisher(private val context: Context) {
|
|||||||
open fun publish(staged: File, destination: Uri) {
|
open fun publish(staged: File, destination: Uri) {
|
||||||
val destinationWasEmpty = destinationIsKnownEmpty(destination)
|
val destinationWasEmpty = destinationIsKnownEmpty(destination)
|
||||||
try {
|
try {
|
||||||
val out = context.contentResolver.openOutputStream(destination)
|
val out = openDestination(destination)
|
||||||
?: error("Could not open destination for writing: $destination")
|
?: error("Could not open destination for writing: $destination")
|
||||||
out.use { sink -> staged.inputStream().use { source -> source.copyTo(sink) } }
|
out.use { sink -> staged.inputStream().use { source -> source.copyTo(sink) } }
|
||||||
} catch (failure: Throwable) {
|
} catch (failure: Throwable) {
|
||||||
@@ -179,6 +180,22 @@ open class OutputPublisher(private val context: Context) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Opens [destination] for writing, or null when the provider will not.
|
||||||
|
*
|
||||||
|
* A seam, and a narrow one: it exists because `openOutputStream` has **two** ways of refusing
|
||||||
|
* and only one of them is reachable from a test otherwise. A provider that has gone away throws
|
||||||
|
* `FileNotFoundException` from inside the call; a provider that is present and declines returns
|
||||||
|
* null. The two are not interchangeable here — the `?: error(...)` above is the only thing that
|
||||||
|
* turns the second into a failure rather than an NPE further down — and no fake provider can be
|
||||||
|
* asked to produce a null return on demand.
|
||||||
|
*
|
||||||
|
* `protected open` rather than injected, matching `hasSpaceFor` and `createStagingFile`:
|
||||||
|
* `WorkerStubs.kt`'s publishers already override one method to force one condition.
|
||||||
|
*/
|
||||||
|
protected open fun openDestination(destination: Uri): OutputStream? =
|
||||||
|
context.contentResolver.openOutputStream(destination)
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* True only when the destination is *positively known* to hold no bytes yet.
|
* True only when the destination is *positively known* to hold no bytes yet.
|
||||||
*
|
*
|
||||||
@@ -256,7 +273,7 @@ open class OutputPublisher(private val context: Context) {
|
|||||||
open fun sweepStaging(nowMs: Long = System.currentTimeMillis()) {
|
open fun sweepStaging(nowMs: Long = System.currentTimeMillis()) {
|
||||||
val dir = stagingDir
|
val dir = stagingDir
|
||||||
val listing = dir.listFiles() ?: return
|
val listing = dir.listFiles() ?: return
|
||||||
val entries = listing.map { StagingSweep.Entry(it.name, it.lastModified()) }
|
val entries = snapshot(listing)
|
||||||
StagingSweep.collectable(entries, nowMs).forEach { name ->
|
StagingSweep.collectable(entries, nowMs).forEach { name ->
|
||||||
val file = File(dir, name)
|
val file = File(dir, name)
|
||||||
// Re-read the timestamp rather than trusting the snapshot above. Between the
|
// Re-read the timestamp rather than trusting the snapshot above. Between the
|
||||||
@@ -268,6 +285,22 @@ open class OutputPublisher(private val context: Context) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The name and age of everything [sweepStaging] found, read once.
|
||||||
|
*
|
||||||
|
* A seam for the *race*, not for the clock — [sweepStaging] already takes `nowMs`, so the clock
|
||||||
|
* is the caller's. What has no seam otherwise is the window between this snapshot and the
|
||||||
|
* per-file re-read below it, and that window is the entire reason the re-read exists.
|
||||||
|
*
|
||||||
|
* **It has to be here and not around `listFiles()`.** A test that changes a file before the
|
||||||
|
* listing, or during it, changes what `StagingSweep.collectable` is given — so the file is
|
||||||
|
* never proposed for deletion and the re-read is never reached. The race being modelled is a
|
||||||
|
* file that *was* collectable when the snapshot was taken and is not by the time the delete
|
||||||
|
* comes round, which is exactly one worker resuming in this same process.
|
||||||
|
*/
|
||||||
|
protected open fun snapshot(listing: Array<File>): List<StagingSweep.Entry> =
|
||||||
|
listing.map { StagingSweep.Entry(it.name, it.lastModified()) }
|
||||||
|
|
||||||
private fun File.canonicalOrAbsolute(): File = runCatching { canonicalFile }.getOrDefault(absoluteFile)
|
private fun File.canonicalOrAbsolute(): File = runCatching { canonicalFile }.getOrDefault(absoluteFile)
|
||||||
|
|
||||||
private companion object {
|
private companion object {
|
||||||
|
|||||||
@@ -20,6 +20,9 @@ import java.io.OutputStream
|
|||||||
/** What a destination volume says when it fills up mid-write. */
|
/** What a destination volume says when it fills up mid-write. */
|
||||||
private const val NO_SPACE = "No space left on device"
|
private const val NO_SPACE = "No space left on device"
|
||||||
|
|
||||||
|
/** How far a failing copy gets before the volume "fills up". Any value below the payload does. */
|
||||||
|
private const val PARTIAL_BYTES = 512
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* A sink that behaves like a volume filling up.
|
* A sink that behaves like a volume filling up.
|
||||||
*
|
*
|
||||||
@@ -75,6 +78,8 @@ class OutputPublisherPublishTest {
|
|||||||
|
|
||||||
private val payload = ByteArray(8192) { (it % 251).toByte() }
|
private val payload = ByteArray(8192) { (it % 251).toByte() }
|
||||||
|
|
||||||
|
/** How far a failing copy gets before the volume "fills up". Any value below the payload does. */
|
||||||
|
|
||||||
private val documentUri: Uri = Uri.parse("content://$DOCUMENTS_AUTHORITY/document/holiday.mp4")
|
private val documentUri: Uri = Uri.parse("content://$DOCUMENTS_AUTHORITY/document/holiday.mp4")
|
||||||
private val plainUri: Uri = Uri.parse("content://$PLAIN_AUTHORITY/document/holiday_plain.mp4")
|
private val plainUri: Uri = Uri.parse("content://$PLAIN_AUTHORITY/document/holiday_plain.mp4")
|
||||||
private val deadUri: Uri = Uri.parse("content://org.libremediaconverter.nonexistent/document/gone.mp4")
|
private val deadUri: Uri = Uri.parse("content://org.libremediaconverter.nonexistent/document/gone.mp4")
|
||||||
@@ -203,6 +208,70 @@ class OutputPublisherPublishTest {
|
|||||||
assertEquals(emptyList<Uri>(), FakeSafProvider.deleteRequests)
|
assertEquals(emptyList<Uri>(), FakeSafProvider.deleteRequests)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a destination whose size cannot be determined is never deleted`() {
|
||||||
|
// The three short-circuits in destinationIsKnownEmpty, and the reason its KDoc gives for
|
||||||
|
// each of them answering false:
|
||||||
|
//
|
||||||
|
// "this decides whether a delete is allowed and 'I could not tell' must never authorise
|
||||||
|
// one."
|
||||||
|
//
|
||||||
|
// The contrast is `a copy that fails partway leaves nothing at the destination` above: a
|
||||||
|
// provider that *does* say zero gets the delete. These say nothing, so they must not.
|
||||||
|
// Getting this backwards costs the user a file they already had, on a save that failed.
|
||||||
|
//
|
||||||
|
// Named exemption: of the three conjuncts, `size >= 0` cannot be falsified behaviourally.
|
||||||
|
// Measured -- getColumnIndex returns -1 for an absent column, and isNull(-1) throws
|
||||||
|
// CursorIndexOutOfBoundsException, which the surrounding runCatching already turns into
|
||||||
|
// `?: false`. So relaxing it to `size >= -1` leaves this test green: same answer, reached
|
||||||
|
// by the exception path instead. The guard should stay -- control flow through an exception
|
||||||
|
// is worse than a comparison, and another Cursor implementation need not throw -- but no
|
||||||
|
// assertion here pins it, and saying so beats implying the missing-column case covers it.
|
||||||
|
// `!row.isNull(size)` and `row.moveToFirst()` do both bite.
|
||||||
|
listOf(
|
||||||
|
RowShape.NO_SIZE_COLUMN to "a cursor with no SIZE column",
|
||||||
|
RowShape.NULL_SIZE to "a cursor whose SIZE cell is null",
|
||||||
|
RowShape.NO_ROWS to "a cursor holding no rows",
|
||||||
|
).forEach { (shape, description) ->
|
||||||
|
FakeSafProvider.deleteRequests.clear()
|
||||||
|
FakeSafProvider.backingFile(documentUri).writeBytes(ByteArray(0))
|
||||||
|
FakeSafProvider.rowShape = shape
|
||||||
|
failMidCopy(documentUri, afterBytes = PARTIAL_BYTES)
|
||||||
|
|
||||||
|
assertThrows(IOException::class.java) { publisher.publish(staged, documentUri) }
|
||||||
|
|
||||||
|
assertEquals(
|
||||||
|
"$description must not authorise a delete",
|
||||||
|
emptyList<Uri>(),
|
||||||
|
FakeSafProvider.deleteRequests,
|
||||||
|
)
|
||||||
|
assertTrue(
|
||||||
|
"$description must leave the destination where it was",
|
||||||
|
FakeSafProvider.backingFile(documentUri).exists(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a provider that declines by returning null fails with the destination named`() {
|
||||||
|
// openOutputStream has two ways of refusing, and only one of them is otherwise reachable.
|
||||||
|
// `a destination the provider will not open...` above drives the throwing one -- a provider
|
||||||
|
// that has gone away. This is the other: a provider that is present, answers, and hands
|
||||||
|
// back null. Without the `?: error(...)` that becomes an NPE inside `use`, which reaches
|
||||||
|
// the user as "Conversion failed." with a null message.
|
||||||
|
val nullOpening = object : OutputPublisher(context) {
|
||||||
|
override fun openDestination(destination: Uri): OutputStream? = null
|
||||||
|
}
|
||||||
|
|
||||||
|
val failure = runCatching { nullOpening.publish(staged, documentUri) }.exceptionOrNull()
|
||||||
|
|
||||||
|
assertTrue("a null stream must not appear to succeed, got $failure", failure != null)
|
||||||
|
assertTrue(
|
||||||
|
"the failure must name the destination rather than being a bare NPE; got ${failure?.message}",
|
||||||
|
failure?.message?.contains("Could not open destination for writing") == true,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `a copy that succeeds delivers every byte and deletes nothing`() {
|
fun `a copy that succeeds delivers every byte and deletes nothing`() {
|
||||||
shadowOf(context.contentResolver).registerOutputStreamSupplier(documentUri) {
|
shadowOf(context.contentResolver).registerOutputStreamSupplier(documentUri) {
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
package org.libremediaconverter.convert
|
package org.libremediaconverter.convert
|
||||||
|
|
||||||
|
import android.app.Application
|
||||||
import org.junit.Assert.assertFalse
|
import org.junit.Assert.assertFalse
|
||||||
|
import org.junit.Assert.assertNull
|
||||||
import org.junit.Assert.assertTrue
|
import org.junit.Assert.assertTrue
|
||||||
import org.junit.Before
|
import org.junit.Before
|
||||||
import org.junit.Test
|
import org.junit.Test
|
||||||
@@ -26,14 +28,18 @@ import java.util.UUID
|
|||||||
@RunWith(RobolectricTestRunner::class)
|
@RunWith(RobolectricTestRunner::class)
|
||||||
class OutputPublisherStagingTest {
|
class OutputPublisherStagingTest {
|
||||||
|
|
||||||
|
private lateinit var app: Application
|
||||||
private lateinit var cacheDir: File
|
private lateinit var cacheDir: File
|
||||||
private lateinit var publisher: OutputPublisher
|
private lateinit var publisher: OutputPublisher
|
||||||
|
|
||||||
@Before
|
@Before
|
||||||
fun setUp() {
|
fun setUp() {
|
||||||
val context = RuntimeEnvironment.getApplication()
|
// Held as a field rather than a local: the race test below builds an anonymous
|
||||||
cacheDir = context.cacheDir
|
// OutputPublisher, and inside that `object` expression a bare `context` resolves to the
|
||||||
publisher = OutputPublisher(context)
|
// superclass's own constructor property, which is not initialised at the super call.
|
||||||
|
app = RuntimeEnvironment.getApplication()
|
||||||
|
cacheDir = app.cacheDir
|
||||||
|
publisher = OutputPublisher(app)
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@@ -99,4 +105,103 @@ class OutputPublisherStagingTest {
|
|||||||
|
|
||||||
publisher.sweepStaging()
|
publisher.sweepStaging()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `the sweep tolerates a staging path that is not a directory`() {
|
||||||
|
// The other half of `listFiles() ?: return`, and not the same as the case above: a missing
|
||||||
|
// directory is created by `stagingDir`'s own mkdirs() and lists as empty. Only a path that
|
||||||
|
// cannot be a directory makes listFiles() answer null, and a sweep that dereferenced that
|
||||||
|
// would take the app down on a launch rather than on a conversion -- AppStartSweepTest is
|
||||||
|
// where this runs from.
|
||||||
|
val stagingPath = stagingPathAsRegularFile()
|
||||||
|
|
||||||
|
publisher.sweepStaging()
|
||||||
|
|
||||||
|
assertTrue("the sweep must not have replaced the fixture", stagingPath.isFile)
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Makes `cacheDir/conversions` a regular file, which is the whole precondition of the test
|
||||||
|
* above -- and does it in a loop, because a single delete-then-write loses a race that CI
|
||||||
|
* caught and this machine does not reproduce.
|
||||||
|
*
|
||||||
|
* `LibreMediaConverterApp.onCreate` ends with
|
||||||
|
* `appScope.launch { OutputPublisher(...).sweepStaging() }` on `Dispatchers.IO`, and
|
||||||
|
* `sweepStaging` reads `stagingDir`, whose getter calls `mkdirs()`. Robolectric instantiates
|
||||||
|
* the application for every test that asks for one, so that background `mkdirs()` is in flight
|
||||||
|
* across the whole suite, on a thread the paused main looper does not control. Between deleting
|
||||||
|
* this path and writing it there is a window where the path does not exist and that `mkdirs()`
|
||||||
|
* can win, which is `FileNotFoundException: ... (Is a directory)` out of `writeBytes` -- run
|
||||||
|
* 33069641674 on #149, once, against 468 tests that pass here.
|
||||||
|
*
|
||||||
|
* Retrying closes it rather than narrowing it, because the race is not symmetric: `mkdirs()`
|
||||||
|
* fails on an existing regular file, so the invariant only has to survive being *established*.
|
||||||
|
* Once a write lands, nothing in the suite can turn this back into a directory.
|
||||||
|
*
|
||||||
|
* The wider problem -- application-scope IO work racing every Robolectric test that shares
|
||||||
|
* `cacheDir` -- is #159, and is deliberately not fixed here.
|
||||||
|
*/
|
||||||
|
private fun stagingPathAsRegularFile(): File {
|
||||||
|
val stagingPath = File(cacheDir, "conversions")
|
||||||
|
repeat(FIXTURE_ATTEMPTS) {
|
||||||
|
if (stagingPath.isFile) return stagingPath
|
||||||
|
stagingPath.deleteRecursively()
|
||||||
|
runCatching { stagingPath.writeBytes(ByteArray(FIXTURE_BYTES)) }
|
||||||
|
}
|
||||||
|
check(stagingPath.isFile) {
|
||||||
|
"the fixture needs $stagingPath to be a regular file and it is a directory; " +
|
||||||
|
"something recreated it $FIXTURE_ATTEMPTS times -- see #159"
|
||||||
|
}
|
||||||
|
return stagingPath
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a file that stops being collectable between the listing and the delete survives`() {
|
||||||
|
// The race the second timestamp read exists for, and the only branch of it that had never
|
||||||
|
// run. The comment in sweepStaging states the cost precisely: a worker resumed by
|
||||||
|
// WorkManager -- in this same process -- could have started writing this very file, and
|
||||||
|
// unlinking an inode a running job still holds open ends with the job reporting success for
|
||||||
|
// a path that no longer exists.
|
||||||
|
//
|
||||||
|
// So: a file old enough to collect at listing time, touched to now before the delete is
|
||||||
|
// reached. StagingSweep.collectable already said yes; isCollectable has to say no.
|
||||||
|
val orphan = publisher.createStagingFile(
|
||||||
|
StagingNames.forJob(UUID.randomUUID(), "mp4"),
|
||||||
|
).apply { writeBytes(ByteArray(4096)) }
|
||||||
|
assertTrue(orphan.setLastModified(System.currentTimeMillis() - StagingSweep.GRACE_PERIOD_MS - 60_000))
|
||||||
|
|
||||||
|
// Touched *after* the snapshot is taken, which is the only window that reaches the
|
||||||
|
// re-read. Doing it around listFiles() instead changes what StagingSweep.collectable is
|
||||||
|
// given, so the file is never proposed for deletion and the guard is never exercised --
|
||||||
|
// measured, and the reason the seam sits where it does.
|
||||||
|
val racing = object : OutputPublisher(app) {
|
||||||
|
override fun snapshot(listing: Array<File>): List<StagingSweep.Entry> =
|
||||||
|
super.snapshot(listing).also { orphan.setLastModified(System.currentTimeMillis()) }
|
||||||
|
}
|
||||||
|
|
||||||
|
racing.sweepStaging()
|
||||||
|
|
||||||
|
assertTrue(
|
||||||
|
"a file a live job started writing after the listing must not be unlinked",
|
||||||
|
orphan.exists(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `discarding a file with no parent at all is refused`() {
|
||||||
|
// A relative name has no parent directory, so `staged.parentFile` is null. The handle
|
||||||
|
// reaches the ViewModel as a path string out of WorkInfo.outputData and is turned straight
|
||||||
|
// into a File, so this is not a shape the caller can rule out -- and the guard has to
|
||||||
|
// answer false rather than dereference it.
|
||||||
|
val parentless = File("holiday.mp4")
|
||||||
|
assertNull("the fixture is supposed to have no parent", parentless.parentFile)
|
||||||
|
|
||||||
|
assertFalse("a file with no parent is not in staging", publisher.discardStaged(parentless))
|
||||||
|
}
|
||||||
|
|
||||||
|
private companion object {
|
||||||
|
/** Enough to outlast a burst of application-scope sweeps; one attempt is what CI lost. */
|
||||||
|
const val FIXTURE_ATTEMPTS = 50
|
||||||
|
const val FIXTURE_BYTES = 8
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -358,4 +358,120 @@ class ContainerCapabilitiesTest {
|
|||||||
assertEquals(emptyList<VideoCodec>(), ContainerCapabilities.encodableVideo(container))
|
assertEquals(emptyList<VideoCodec>(), 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,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -27,9 +27,6 @@ import org.robolectric.RobolectricTestRunner
|
|||||||
import org.robolectric.RuntimeEnvironment
|
import org.robolectric.RuntimeEnvironment
|
||||||
import java.io.File
|
import java.io.File
|
||||||
import java.util.UUID
|
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.
|
* 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 =
|
private fun conversionWorker(runAttemptCount: Int = 0): ConversionWorker =
|
||||||
TestListenableWorkerBuilder<ConversionWorker>(
|
TestListenableWorkerBuilder<ConversionWorker>(
|
||||||
context = app,
|
context = app,
|
||||||
@@ -141,18 +172,22 @@ class DeniedForegroundStartTest {
|
|||||||
.setForegroundUpdater(DenyingForegroundUpdater)
|
.setForegroundUpdater(DenyingForegroundUpdater)
|
||||||
.build()
|
.build()
|
||||||
|
|
||||||
private fun concatWorker(): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
|
private fun concatWorker(runAttemptCount: Int = 0): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
|
||||||
context = app,
|
context = app,
|
||||||
inputData = workDataOf(
|
inputData = workDataOf(
|
||||||
ConcatWorker.KEY_INPUT_URIS to arrayOf(INPUT.toString(), "content://test/second.mp4"),
|
ConcatWorker.KEY_INPUT_URIS to arrayOf(INPUT.toString(), "content://test/second.mp4"),
|
||||||
ConcatWorker.KEY_TOTAL_BYTES to INPUT_BYTES,
|
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)
|
).setId(CONCAT_ID)
|
||||||
.setForegroundUpdater(DenyingForegroundUpdater)
|
.setForegroundUpdater(DenyingForegroundUpdater)
|
||||||
.build()
|
.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. */
|
/** 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))
|
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 INPUT_BYTES = 1024L
|
||||||
const val PARTIAL_BYTES = 2048
|
const val PARTIAL_BYTES = 2048
|
||||||
val SPEC = OutputFormat.MP4_H265.spec
|
val SPEC = OutputFormat.MP4_H265.spec
|
||||||
|
val CONCAT_FORMAT = OutputFormat.MP4_H264
|
||||||
val CONVERSION_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000001")
|
val CONVERSION_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000001")
|
||||||
val CONCAT_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000002")
|
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)
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -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<String>(), 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<Uri>` 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<ConversionWorker>(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<ConcatWorker>(
|
||||||
|
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<String> =
|
||||||
|
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<OutputSpec>()
|
||||||
|
|
||||||
|
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
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -1,12 +1,16 @@
|
|||||||
package org.libremediaconverter.work
|
package org.libremediaconverter.work
|
||||||
|
|
||||||
import android.app.Application
|
import android.app.Application
|
||||||
|
import android.content.Context
|
||||||
import android.net.Uri
|
import android.net.Uri
|
||||||
import androidx.media3.common.util.UnstableApi
|
import androidx.media3.common.util.UnstableApi
|
||||||
import androidx.work.Data
|
import androidx.work.Data
|
||||||
|
import androidx.work.ForegroundInfo
|
||||||
|
import androidx.work.ForegroundUpdater
|
||||||
import androidx.work.ListenableWorker
|
import androidx.work.ListenableWorker
|
||||||
import androidx.work.testing.TestListenableWorkerBuilder
|
import androidx.work.testing.TestListenableWorkerBuilder
|
||||||
import androidx.work.workDataOf
|
import androidx.work.workDataOf
|
||||||
|
import com.google.common.util.concurrent.ListenableFuture
|
||||||
import kotlinx.coroutines.CancellationException
|
import kotlinx.coroutines.CancellationException
|
||||||
import kotlinx.coroutines.runBlocking
|
import kotlinx.coroutines.runBlocking
|
||||||
import org.junit.After
|
import org.junit.After
|
||||||
@@ -18,6 +22,7 @@ import org.junit.runner.RunWith
|
|||||||
import org.libremediaconverter.convert.ConversionDependencies
|
import org.libremediaconverter.convert.ConversionDependencies
|
||||||
import org.libremediaconverter.convert.OutputPublisher
|
import org.libremediaconverter.convert.OutputPublisher
|
||||||
import org.libremediaconverter.convert.SoftwareTranscoder
|
import org.libremediaconverter.convert.SoftwareTranscoder
|
||||||
|
import org.libremediaconverter.convert.StagingNames
|
||||||
import org.libremediaconverter.convert.installTestWorkManager
|
import org.libremediaconverter.convert.installTestWorkManager
|
||||||
import org.libremediaconverter.model.ConversionRequest
|
import org.libremediaconverter.model.ConversionRequest
|
||||||
import org.libremediaconverter.model.DeviceCodecs
|
import org.libremediaconverter.model.DeviceCodecs
|
||||||
@@ -108,6 +113,54 @@ class WorkerCancellationTest {
|
|||||||
assertEquals("a failed attempt must not leave its partial behind", emptyList<String>(), stagedNames())
|
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.
|
* 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 DISPLAY_NAME = "holiday.mp4"
|
||||||
const val INPUT_BYTES = 1024L
|
const val INPUT_BYTES = 1024L
|
||||||
val SPEC = OutputFormat.MP4_H265.spec
|
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 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
|
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"))
|
||||||
|
}
|
||||||
|
|||||||
@@ -22,6 +22,7 @@ import org.libremediaconverter.model.DeviceCodecs
|
|||||||
import org.libremediaconverter.model.EnginePreference
|
import org.libremediaconverter.model.EnginePreference
|
||||||
import org.libremediaconverter.model.InputProbe
|
import org.libremediaconverter.model.InputProbe
|
||||||
import org.libremediaconverter.model.OutputFormat
|
import org.libremediaconverter.model.OutputFormat
|
||||||
|
import org.libremediaconverter.model.OutputSpec
|
||||||
import org.libremediaconverter.model.QualityTier
|
import org.libremediaconverter.model.QualityTier
|
||||||
import org.robolectric.RobolectricTestRunner
|
import org.robolectric.RobolectricTestRunner
|
||||||
import org.robolectric.RuntimeEnvironment
|
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. */
|
/** [ListenableWorker.Result.Success] compares its output data, which these tests do not pin. */
|
||||||
private fun stripOutput(result: ListenableWorker.Result): ListenableWorker.Result =
|
private fun stripOutput(result: ListenableWorker.Result): ListenableWorker.Result =
|
||||||
if (result is ListenableWorker.Result.Success) ListenableWorker.Result.success() else result
|
if (result is ListenableWorker.Result.Success) ListenableWorker.Result.success() else result
|
||||||
@@ -116,15 +161,18 @@ class WorkerEnumFallbackTest {
|
|||||||
private fun conversionWorker(
|
private fun conversionWorker(
|
||||||
quality: String = QualityTier.FAST.name,
|
quality: String = QualityTier.FAST.name,
|
||||||
preference: String = EnginePreference.FORCE_SOFTWARE.name,
|
preference: String = EnginePreference.FORCE_SOFTWARE.name,
|
||||||
|
container: String = SPEC.container.name,
|
||||||
|
video: String = SPEC.videoCodec.name,
|
||||||
|
audio: String = SPEC.audioCodec.name,
|
||||||
): ConversionWorker = TestListenableWorkerBuilder<ConversionWorker>(
|
): ConversionWorker = TestListenableWorkerBuilder<ConversionWorker>(
|
||||||
context = app,
|
context = app,
|
||||||
inputData = workDataOf(
|
inputData = workDataOf(
|
||||||
ConversionWorker.KEY_INPUT_URI to INPUT.toString(),
|
ConversionWorker.KEY_INPUT_URI to INPUT.toString(),
|
||||||
ConversionWorker.KEY_DISPLAY_NAME to DISPLAY_NAME,
|
ConversionWorker.KEY_DISPLAY_NAME to DISPLAY_NAME,
|
||||||
ConversionWorker.KEY_SIZE_BYTES to INPUT_BYTES,
|
ConversionWorker.KEY_SIZE_BYTES to INPUT_BYTES,
|
||||||
ConversionWorker.KEY_CONTAINER to SPEC.container.name,
|
ConversionWorker.KEY_CONTAINER to container,
|
||||||
ConversionWorker.KEY_VIDEO_CODEC to SPEC.videoCodec.name,
|
ConversionWorker.KEY_VIDEO_CODEC to video,
|
||||||
ConversionWorker.KEY_AUDIO_CODEC to SPEC.audioCodec.name,
|
ConversionWorker.KEY_AUDIO_CODEC to audio,
|
||||||
ConversionWorker.KEY_QUALITY to quality,
|
ConversionWorker.KEY_QUALITY to quality,
|
||||||
ConversionWorker.KEY_ENGINE_PREFERENCE to preference,
|
ConversionWorker.KEY_ENGINE_PREFERENCE to preference,
|
||||||
),
|
),
|
||||||
@@ -146,6 +194,12 @@ class WorkerEnumFallbackTest {
|
|||||||
const val DISPLAY_NAME = "holiday.mp4"
|
const val DISPLAY_NAME = "holiday.mp4"
|
||||||
const val INPUT_BYTES = 1024L
|
const val INPUT_BYTES = 1024L
|
||||||
val SPEC = OutputFormat.MP4_H265.spec
|
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 CONVERSION_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000021")
|
||||||
val CONCAT_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000022")
|
val CONCAT_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000022")
|
||||||
}
|
}
|
||||||
@@ -156,6 +210,9 @@ private class RequestRecordingTranscoder : SoftwareTranscoder {
|
|||||||
|
|
||||||
val qualities = mutableListOf<QualityTier>()
|
val qualities = mutableListOf<QualityTier>()
|
||||||
|
|
||||||
|
/** The spec each run was asked for. Which one ran is what the three readSpec tests assert. */
|
||||||
|
val specs = mutableListOf<OutputSpec>()
|
||||||
|
|
||||||
override suspend fun run(
|
override suspend fun run(
|
||||||
request: ConversionRequest,
|
request: ConversionRequest,
|
||||||
inputPath: String,
|
inputPath: String,
|
||||||
@@ -164,6 +221,7 @@ private class RequestRecordingTranscoder : SoftwareTranscoder {
|
|||||||
onProgress: (Int) -> Unit,
|
onProgress: (Int) -> Unit,
|
||||||
) {
|
) {
|
||||||
qualities += request.quality
|
qualities += request.quality
|
||||||
|
specs += request.spec
|
||||||
output.writeBytes(ByteArray(OUTPUT_BYTES))
|
output.writeBytes(ByteArray(OUTPUT_BYTES))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,10 +1,14 @@
|
|||||||
package org.libremediaconverter.work
|
package org.libremediaconverter.work
|
||||||
|
|
||||||
import android.content.Context
|
import android.content.Context
|
||||||
|
import com.google.common.util.concurrent.ListenableFuture
|
||||||
import org.libremediaconverter.convert.OutputPublisher
|
import org.libremediaconverter.convert.OutputPublisher
|
||||||
import org.libremediaconverter.convert.SoftwareTranscoder
|
import org.libremediaconverter.convert.SoftwareTranscoder
|
||||||
import org.libremediaconverter.model.ConversionRequest
|
import org.libremediaconverter.model.ConversionRequest
|
||||||
import java.io.File
|
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.
|
* Scaffolding more than one worker test needs.
|
||||||
@@ -68,3 +72,25 @@ object WritingTranscoder : SoftwareTranscoder {
|
|||||||
|
|
||||||
private const val OUTPUT_BYTES = 512
|
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)
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user