diff --git a/app/src/test/java/org/libremediaconverter/AppStartSweepTest.kt b/app/src/test/java/org/libremediaconverter/AppStartSweepTest.kt new file mode 100644 index 0000000..e5a8646 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/AppStartSweepTest.kt @@ -0,0 +1,98 @@ +package org.libremediaconverter + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.libremediaconverter.convert.StagingSweep +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import java.io.File +import java.util.concurrent.TimeUnit + +/** + * That process start actually sweeps. + * + * [StagingSweepTest][org.libremediaconverter.convert.StagingSweepTest] pins the age rule and + * `OutputPublisherStagingTest` pins the sweep against a real filesystem; neither says anything + * about whether anything calls it, and deleting the one line that does left the whole suite green. + * That line is the only reason this Application class exists, and it is the backstop for every leak + * `discardStaged` cannot reach — a process reclaimed before a save, a worker that failed before its + * output ever became a `Converted` state, a `reset()` whose delete was cancelled with the Activity. + * + * `onCreate()` is called again rather than a second Application being built: it is what the + * framework calls at process start, the scope it launches on is already there, and the first test + * below is what pins that the framework calls it on *this* class. + */ +@RunWith(RobolectricTestRunner::class) +class AppStartSweepTest { + + private lateinit var app: LibreMediaConverterApp + private lateinit var stagingDir: File + + @Before + fun setUp() { + // The cast is an assertion in itself: Robolectric builds the Application named in the + // merged manifest, so this fails if `android:name` ever stops pointing here -- in which + // case the sweep below would be perfectly correct code that never runs. + app = RuntimeEnvironment.getApplication() as LibreMediaConverterApp + stagingDir = File(app.cacheDir, "conversions").apply { mkdirs() } + stagingDir.listFiles()?.forEach { it.delete() } + } + + @Test + fun `the application the manifest starts is the one that sweeps`() { + assertEquals(LibreMediaConverterApp::class.java, RuntimeEnvironment.getApplication().javaClass) + } + + @Test + fun `process start collects an abandoned staged file and leaves a live one alone`() { + val abandoned = stagedFile("abandoned.mp4") + val live = stagedFile("live.mp4") + // Set explicitly. Relying on a file being written "long enough ago" is not something a test + // can arrange, and the grace period is a day. + assertTrue( + abandoned.setLastModified(System.currentTimeMillis() - StagingSweep.GRACE_PERIOD_MS - ONE_MINUTE_MS), + ) + + // Both files are still here on the way in. The Application was already constructed once + // before this test ran, so without this the sweep that call started could be the one that + // collected the file, and the assertion below would be about the wrong process start. + assertTrue(abandoned.exists() && live.exists()) + + app.onCreate() + + awaitGone(abandoned) + // The other half, and the one that says the sweep is a sweep rather than a + // `clearStaging()`: the directory is shared by the convert tab, the join tab and + // ConcatEngine's list file, so deleting everything could take a file from a running job. + assertTrue("a file written moments ago belongs to a live job", live.exists()) + } + + /** + * Waits for [file] to be deleted. + * + * The sweep runs on `Dispatchers.IO`, deliberately: it lists a directory and stats every entry + * on the path that decides how long the launcher icon stays unresponsive. So there is nothing + * to join, and the wait is a bounded poll — long enough for a directory listing, short enough + * that a sweep which never happens fails rather than hangs. + */ + private fun awaitGone(file: File) { + val deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(AWAIT_TIMEOUT_SECONDS) + while (System.nanoTime() < deadline) { + if (!file.exists()) return + Thread.sleep(POLL_INTERVAL_MS) + } + fail("process start left ${file.name} in staging; nothing swept it") + } + + private fun stagedFile(name: String): File = File(stagingDir, name).apply { writeBytes(ByteArray(4096)) } + + private companion object { + const val ONE_MINUTE_MS = 60L * 1000 + const val AWAIT_TIMEOUT_SECONDS = 10L + const val POLL_INTERVAL_MS = 5L + } +} diff --git a/app/src/test/java/org/libremediaconverter/BackupExclusionsTest.kt b/app/src/test/java/org/libremediaconverter/BackupExclusionsTest.kt new file mode 100644 index 0000000..699ccd6 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/BackupExclusionsTest.kt @@ -0,0 +1,106 @@ +package org.libremediaconverter + +import android.content.res.XmlResourceParser +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.xmlpull.v1.XmlPullParser + +/** + * What the app lets leave the device. + * + * `data_extraction_rules.xml` is a resource rather than code, so nothing was checking it: reverting + * the whole file to the template's boilerplate left the unit tests green AND `lintDebug` green, and + * a future edit dropping the excludes would ship in silence. The failure it would cause is one + * nobody meets in development — a cloud restore or a device-to-device transfer. + * + * What is at stake is written in the file itself. WorkManager's queue is the app's entire backup + * payload, and every row in it references a `content://` URI granted to one install on one device + * and an output path under that install's `cacheDir`. Neither survives the transfer, and the rows + * are not inert when they arrive: reattachment queries WorkManager by tag on launch, so a fresh + * install would come up attached to a job the user never ran on it. + * + * Read out of the compiled resource table rather than off `src/main/res`, so what is asserted is + * what the APK actually carries. Note the limit of that: this pins the rules' content, not the + * `android:dataExtractionRules` attribute that points the system at them. + */ +@RunWith(RobolectricTestRunner::class) +class BackupExclusionsTest { + + @Test + fun `the work queue is excluded from cloud backup and from device transfer alike`() { + // Both sections, because they are separately honoured: `allowBackup` stays true and the + // exclusion is per-file, so an edit that dropped either half would leave the other looking + // like the whole answer. + assertEquals( + mapOf( + "cloud-backup" to WORK_MANAGER_STATE, + "device-transfer" to WORK_MANAGER_STATE, + ), + excludesBySection(), + ) + } + + /** + * Every `` in the rules, as `domain:path`, grouped by the section it sits in. + * + * Both halves of each entry, because an `` carrying no path is skipped unchecked by + * lint's own detector — so that spelling could protect nothing while still looking like a rule. + */ + private fun excludesBySection(): Map> { + // Both sections start present and empty, so a section deleted outright fails as an empty + // set rather than as a missing key -- the same finding either way, said the same way. + val found = SECTIONS.associateWith { mutableSetOf() } + var section: String? = null + RuntimeEnvironment.getApplication().resources.getXml(R.xml.data_extraction_rules).use { parser -> + while (parser.next() != XmlPullParser.END_DOCUMENT) { + section = parser.sectionAfter(section, found) + } + } + return found + } + + /** Folds one parse event into [found], and answers which section the parser is now inside. */ + private fun XmlResourceParser.sectionAfter(section: String?, found: Map>): String? = + when { + eventType == XmlPullParser.START_TAG && name in SECTIONS -> name + eventType == XmlPullParser.END_TAG && name == section -> null + eventType == XmlPullParser.START_TAG && name == "exclude" && section != null -> + section.also { found.getValue(it) += entry() } + + else -> section + } + + private fun XmlResourceParser.entry(): String = "${attribute("domain")}:${attribute("path")}" + + /** + * The value of the attribute called [name] on the current tag. + * + * Walked by index rather than looked up by namespace. These attributes carry the `android` + * namespace in the source file, but a parser over the *compiled* resource reports them with + * none, so `getAttributeValue(namespace, name)` answers null for every one of them. + */ + private fun XmlResourceParser.attribute(name: String): String? = + (0 until attributeCount).firstOrNull { getAttributeName(it) == name }?.let { getAttributeValue(it) } + + private companion object { + /** The two ways data leaves a device, both of which these rules have to answer. */ + val SECTIONS = setOf("cloud-backup", "device-transfer") + + /** + * WorkManager's own storage, spelled the way WorkManager spells it. + * + * The database is Room-backed and therefore in WAL mode, hence the two sidecars. Pinning + * the spelling is the point rather than a cost: a WorkManager release renaming its database + * would silently un-exclude the queue, and this failing is how anyone would find out. + */ + val WORK_MANAGER_STATE = setOf( + "database:androidx.work.workdb", + "database:androidx.work.workdb-wal", + "database:androidx.work.workdb-shm", + "sharedpref:androidx.work.util.preferences.xml", + ) + } +} diff --git a/app/src/test/java/org/libremediaconverter/work/PerJobStagingTest.kt b/app/src/test/java/org/libremediaconverter/work/PerJobStagingTest.kt index 0716e7b..7439dc1 100644 --- a/app/src/test/java/org/libremediaconverter/work/PerJobStagingTest.kt +++ b/app/src/test/java/org/libremediaconverter/work/PerJobStagingTest.kt @@ -10,11 +10,11 @@ import kotlinx.coroutines.runBlocking import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +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.installTestWorkManager import org.libremediaconverter.model.DeviceCodecs import org.libremediaconverter.model.EnginePreference @@ -35,19 +35,24 @@ import java.util.UUID * * These drive the real worker rather than the naming function, because the naming function was * never the part that was wrong. What was wrong is which name the worker asked for. + * + * Both workers, for the same reason. The join side collided harder — `joined.` is one string + * for every join of a format, where a conversion at least needed two inputs of the same name — and + * it was the half with no test at all: reverting `ConcatWorker` to that constant left all 257 + * tests green. */ @UnstableApi @RunWith(RobolectricTestRunner::class) class PerJobStagingTest { private lateinit var app: Application - private lateinit var publisher: OutputPublisher + private lateinit var publisher: NamingPublisher private lateinit var stagingDir: File @Before fun setUp() { app = RuntimeEnvironment.getApplication() - publisher = AlwaysRoomPublisher(app) + publisher = NamingPublisher(app) ConversionDependencies.publisher = { publisher } ConversionDependencies.probe = { _, _ -> InputProbe() } ConversionDependencies.deviceCodecs = { DeviceCodecs.PERMISSIVE } @@ -56,6 +61,9 @@ class PerJobStagingTest { stagingDir = publisher.createStagingFile("anything").parentFile!! stagingDir.listFiles()?.forEach { it.delete() } + // Asking for the directory above is itself a staging request; the tests are about the ones + // the workers make. + publisher.requestedNames.clear() } @After @@ -104,8 +112,34 @@ class PerJobStagingTest { ) } + @Test + fun `two joins of the same format stage under names of their own`() { + runBlocking { concatWorker(JOB_A).doWork() } + runBlocking { concatWorker(JOB_B).doWork() } + + // Read off what the worker asked for rather than off the directory, and not for + // convenience: ConcatEngine is native, so neither join gets past it here, and the catch on + // the way out deletes whatever was staged. The name is where the collision lived -- + // "joined.${format.extension}" is one string for every join of a format, so two joins were + // one file, exactly as two conversions of a same-named input were. + val names = publisher.requestedNames + assertEquals("each join must stage under a name of its own, got $names", 2, names.toSet().size) + assertTrue("the first join's name must carry its own job id, got ${names[0]}", names[0].contains("$JOB_A")) + assertTrue("the second join's name must carry its own job id, got ${names[1]}", names[1].contains("$JOB_B")) + } + private fun stagedNames(): List = stagingDir.listFiles().orEmpty().map { it.name }.sorted() + private fun concatWorker(id: UUID): 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 JOIN_FORMAT.name, + ), + runAttemptCount = 0, + ).setId(id).build() + private fun conversionWorker( id: UUID, runAttemptCount: Int = 0, @@ -129,6 +163,7 @@ class PerJobStagingTest { const val DISPLAY_NAME = "input.mp4" const val INPUT_BYTES = 1024L val SPEC = OutputFormat.MP4_H265.spec + val JOIN_FORMAT = OutputFormat.MP4_H264 val JOB_A: UUID = UUID.fromString("00000000-0000-4000-8000-00000000000a") val JOB_B: UUID = UUID.fromString("00000000-0000-4000-8000-00000000000b") } diff --git a/app/src/test/java/org/libremediaconverter/work/WorkerEnumFallbackTest.kt b/app/src/test/java/org/libremediaconverter/work/WorkerEnumFallbackTest.kt index 9b3ff3a..32cea02 100644 --- a/app/src/test/java/org/libremediaconverter/work/WorkerEnumFallbackTest.kt +++ b/app/src/test/java/org/libremediaconverter/work/WorkerEnumFallbackTest.kt @@ -1,7 +1,6 @@ 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 @@ -15,7 +14,6 @@ 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.StagingNames import org.libremediaconverter.convert.installTestWorkManager @@ -85,7 +83,7 @@ class WorkerEnumFallbackTest { fun `an engine preference this build does not define does not end the job`() { // Refused on space, which is the first thing below the three reads: it proves the reads // were reached and returned, without dragging in a routing decision this test is not about. - publisher.refuse = true + publisher.refuseSpace = true val result = runBlocking { conversionWorker(preference = "FORCE_QUANTUM").doWork() } @@ -153,25 +151,6 @@ class WorkerEnumFallbackTest { } } -/** - * A real [OutputPublisher] that records the staging names it is asked for, and can refuse on space. - * - * The name is the only place a join's format is legible from outside: the engine that would use it - * is native, and the worker deletes the staged file on its way out of a failed attempt. - */ -private class NamingPublisher(context: Context) : OutputPublisher(context) { - - val requestedNames = mutableListOf() - var refuse = false - - override fun hasSpaceFor(bytes: Long): Boolean = !refuse - - override fun createStagingFile(name: String): File { - requestedNames += name - return super.createStagingFile(name) - } -} - /** An engine that writes the output and remembers what it was asked to produce. */ private class RequestRecordingTranscoder : SoftwareTranscoder { diff --git a/app/src/test/java/org/libremediaconverter/work/WorkerStubs.kt b/app/src/test/java/org/libremediaconverter/work/WorkerStubs.kt index 6a16964..848c04b 100644 --- a/app/src/test/java/org/libremediaconverter/work/WorkerStubs.kt +++ b/app/src/test/java/org/libremediaconverter/work/WorkerStubs.kt @@ -24,6 +24,31 @@ open class AlwaysRoomPublisher(context: Context) : OutputPublisher(context) { override fun hasSpaceFor(bytes: Long): Boolean = true } +/** + * An [AlwaysRoomPublisher] that records the staging names it is asked for. + * + * For a conversion the staged file survives the job and a directory listing says everything. For a + * join it does not: `ConcatEngine` is native, so no test here gets past it, and the catch on the way + * out deletes what was staged. The name the worker *asked* for is then the only place its job id + * and its output format are legible at all — the same reason `SpaceCheckTest` records the question + * rather than the verdict. + */ +open class NamingPublisher(context: Context) : AlwaysRoomPublisher(context) { + + /** Every name passed to [createStagingFile], in order. */ + val requestedNames = mutableListOf() + + /** Set to refuse every space check, the way `FakeFailures.FullDisk` does. */ + var refuseSpace = false + + override fun hasSpaceFor(bytes: Long): Boolean = !refuseSpace + + override fun createStagingFile(name: String): File { + requestedNames += name + return super.createStagingFile(name) + } +} + /** * An engine that writes the output file and nothing else. *