diff --git a/app/build.gradle.kts b/app/build.gradle.kts
index c5ab3ed..5db1707 100644
--- a/app/build.gradle.kts
+++ b/app/build.gradle.kts
@@ -102,6 +102,23 @@ android {
informational += "UsableSpace"
}
+ testOptions {
+ unitTests {
+ // Robolectric needs the merged manifest and the compiled resource table to build
+ // an Android runtime on the JVM. Without this, AGP hands the unit tests a stub
+ // android.jar with no resources and Robolectric cannot start.
+ isIncludeAndroidResources = true
+
+ all {
+ // Robolectric's native runtime calls System.load(), which Java 25 reports as
+ // a restricted method -- four lines of warning on every test run, and a hard
+ // failure in some later JDK. Granting it explicitly says the native access is
+ // known and wanted rather than leaving the JVM to guess.
+ it.jvmArgs("--enable-native-access=ALL-UNNAMED")
+ }
+ }
+ }
+
packaging {
jniLibs {
// Uncompressed .so, so the APK zip-aligns them on 16 KB boundaries.
@@ -263,6 +280,16 @@ dependencies {
debugImplementation(libs.compose.ui.tooling)
testImplementation(libs.junit)
+ // An Android runtime on the JVM. Everything else in src/test is a pure function; this is
+ // here for the one thing a pure function cannot assert -- that a staged file is really
+ // gone from a real cacheDir. Instrumented tests do not run on the development host, so
+ // without it that assertion could only be written where nobody can execute it.
+ testImplementation(libs.robolectric)
+ // Already in the catalog for androidTest, and already inside the prerelease guard via its
+ // androidx. group. WorkManagerTestInitHelper + SynchronousExecutor are what let a JVM test
+ // drive a ViewModel through a real WorkManager to SUCCEEDED, which is where the cleanup
+ // handle is set -- the wiring the leak actually lived in.
+ testImplementation(libs.androidx.work.testing)
androidTestImplementation(platform(libs.compose.bom))
androidTestImplementation(libs.androidx.junit)
diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
index 983c19d..ff07d03 100644
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@@ -20,7 +20,13 @@
off by default on new installs. -->
+
/conversions/` once per
+ * process.
+ *
+ * Every other cleanup path in the app depends on a ViewModel still being alive to run it.
+ * The cases that leak are exactly the ones where it is not — the process is reclaimed
+ * between a conversion finishing and the user saving it, a worker fails before its output
+ * ever becomes a `Converted` state, or a `reset()`'s delete is cancelled along with the
+ * Activity. Process start is the one moment those leftovers are reliably observable.
+ */
+class LibreMediaConverterApp : Application() {
+
+ /**
+ * Deliberately process-lifetime and never cancelled: the work it carries is a single
+ * short task that should outlive nothing in particular and be interrupted by nothing.
+ * A `SupervisorJob` so a failure here could never take a sibling down with it.
+ */
+ private val appScope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
+
+ override fun onCreate() {
+ super.onCreate()
+
+ // Off the main thread: this lists a directory and stats each entry, and it runs on
+ // the path that decides how long the launcher icon stays unresponsive.
+ //
+ // Why this cannot race a live job -- and note the argument is NOT about ordering.
+ // WorkManager initialises through androidx.startup's InitializationProvider, which
+ // is a ContentProvider, so it is already up before onCreate() is called and can be
+ // resuming a worker on its own executor while this runs. Workers run in this same
+ // process, so "nothing has started yet" would simply be false.
+ //
+ // The grace period is what makes it safe. StagingSweep only collects a file nothing
+ // has written to for a full day:
+ //
+ // - Conversion and join outputs are written continuously, so a running job keeps
+ // its own mtime fresh and never looks abandoned.
+ // - concat_list.txt is the one file written once and then only read, so it is the
+ // one that has to be reasoned about rather than observed. A WorkManager attempt
+ // is capped by the six-hour-per-day foreground-service budget and a retry starts
+ // doWork() again from the top, rewriting the list file -- so no single attempt
+ // can hold a file untouched for twenty-four hours.
+ // - A worker resuming right now writes its files at attempt start, which makes
+ // them zero seconds old, not a day.
+ //
+ // sweepStaging() also re-reads each timestamp immediately before deleting, which
+ // closes the window between listing the directory and acting on the listing.
+ appScope.launch { OutputPublisher(this@LibreMediaConverterApp).sweepStaging() }
+ }
+}
diff --git a/app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt b/app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt
index 4e2ed51..31edae5 100644
--- a/app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt
+++ b/app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt
@@ -8,6 +8,7 @@ import androidx.lifecycle.viewModelScope
import androidx.media3.common.util.UnstableApi
import androidx.work.WorkInfo
import androidx.work.WorkManager
+import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.MutableStateFlow
@@ -76,10 +77,29 @@ sealed interface ConversionState {
}
@UnstableApi
-class ConversionViewModel(app: Application) : AndroidViewModel(app) {
+class ConversionViewModel @JvmOverloads constructor(
+ app: Application,
+ /**
+ * Where [reset] runs its delete.
+ *
+ * A parameter so a test can make the cleanup run inline and assert on the result. It
+ * also makes the ordering an explicit choice rather than an accident: the state flips
+ * to `Idle` synchronously while the delete is dispatched, and naming the dispatcher is
+ * what says that was decided rather than inherited.
+ *
+ * `@JvmOverloads` keeps the single-argument constructor that `viewModel()`'s default
+ * `AndroidViewModelFactory` looks up reflectively; without it the app would crash on
+ * the first screen.
+ */
+ private val cleanupDispatcher: CoroutineDispatcher = Dispatchers.IO,
+) : AndroidViewModel(app) {
private val workManager = WorkManager.getInstance(app)
- private val publisher = OutputPublisher(app)
+
+ // Through ConversionDependencies, like the workers, rather than `OutputPublisher(app)`
+ // direct: the ViewModels were the only place bypassing the seam, which left the
+ // cleanup wiring impossible to substitute in a test.
+ private val publisher = ConversionDependencies.publisher(app)
private val _state = MutableStateFlow(ConversionState.Idle)
val state: StateFlow = _state.asStateFlow()
@@ -87,6 +107,16 @@ class ConversionViewModel(app: Application) : AndroidViewModel(app) {
private var observer: Job? = null
private var activeWorkId: UUID? = null
+ /**
+ * The staged output this ViewModel is responsible for deleting.
+ *
+ * A field rather than something read back out of [_state], because the state machine
+ * cannot answer the question on the path that needs it most: a failed [save] lands on
+ * [ConversionState.Failed], which carries a message and no file at all. By then the
+ * only remaining reference would have been lost.
+ */
+ private var pendingStaged: File? = null
+
/** Conversion settings, kept separate from the job state machine. */
private val _settings = MutableStateFlow(ConversionSettings())
val settings: StateFlow = _settings.asStateFlow()
@@ -124,7 +154,7 @@ class ConversionViewModel(app: Application) : AndroidViewModel(app) {
// would read as the app having ignored the tap.
_state.value = ConversionState.Ready(file)
- val probe = withContext(Dispatchers.IO) { MediaProbe.probe(getApplication(), uri) }
+ val probe = withContext(Dispatchers.IO) { ConversionDependencies.probe(getApplication(), uri) }
// Only fill in the probe if the user has not moved on in the meantime.
_state.update { current ->
if (current is ConversionState.Ready && current.input.uri == uri) {
@@ -186,9 +216,13 @@ class ConversionViewModel(app: Application) : AndroidViewModel(app) {
if (path == null) {
ConversionState.Failed("Conversion reported success but produced no file.")
} else {
+ val staged = File(path)
+ // Take responsibility for the file at the same moment the state
+ // starts referring to it, so the two cannot disagree.
+ pendingStaged = staged
ConversionState.Converted(
input = input,
- staged = File(path),
+ staged = staged,
engineUsed = info.outputData
.getString(ConversionWorker.KEY_ENGINE_USED).orEmpty(),
routeReason = info.outputData
@@ -222,6 +256,8 @@ class ConversionViewModel(app: Application) : AndroidViewModel(app) {
converted.staged.delete()
}
}.onSuccess {
+ // publish() already deleted it; nothing left to clean up.
+ pendingStaged = null
_state.value = ConversionState.Saved(
ConversionWorker.outputNameFor(
converted.input.displayName,
@@ -229,15 +265,35 @@ class ConversionViewModel(app: Application) : AndroidViewModel(app) {
),
)
}.onFailure { e ->
+ // Deliberately NOT cleared. A failed save may mean the staged file is the
+ // only copy of an hour of transcoding, and the user's destination did not
+ // receive it -- deleting here would destroy the work to tidy up a cache
+ // directory. It stays collectable: by a later reset(), or by the sweep once
+ // it is old enough to be certain nobody is coming back for it.
_state.value = ConversionState.Failed(e.message ?: "Could not save the file.")
}
}
}
+ /**
+ * Returns to [ConversionState.Idle], deleting anything staged on the way out.
+ *
+ * "Start over" on a finished conversion is an ordinary path through the UI, and it used
+ * to drop the only reference to a full-size file in cache. The delete runs on
+ * [Dispatchers.IO] because it touches the filesystem, and is fire-and-forget: it is
+ * cancelled with [viewModelScope] if the Activity finishes first, so it is a best
+ * effort rather than a guarantee. `OutputPublisher.sweepStaging` is the backstop for
+ * the times it does not run.
+ */
fun reset() {
observer?.cancel()
observer = null
activeWorkId = null
+ val staged = pendingStaged
+ pendingStaged = null
+ if (staged != null) {
+ viewModelScope.launch(cleanupDispatcher) { publisher.discardStaged(staged) }
+ }
_state.value = ConversionState.Idle
}
diff --git a/app/src/main/java/org/libremediaconverter/convert/OutputPublisher.kt b/app/src/main/java/org/libremediaconverter/convert/OutputPublisher.kt
index f0cbe30..59a847b 100644
--- a/app/src/main/java/org/libremediaconverter/convert/OutputPublisher.kt
+++ b/app/src/main/java/org/libremediaconverter/convert/OutputPublisher.kt
@@ -41,10 +41,61 @@ open class OutputPublisher(private val context: Context) {
} ?: error("Could not open destination for writing: $destination")
}
- fun clearStaging() {
- stagingDir.listFiles()?.forEach { it.delete() }
+ /**
+ * Deletes one staged file, if it really is one of ours.
+ *
+ * This is what a ViewModel's `reset()` calls when the user taps "Start over" on a
+ * finished-but-unsaved conversion, which is otherwise a full-size copy left in cache
+ * for the OS to reclaim whenever it feels like it.
+ *
+ * The guard is not decoration. The handle reaches the ViewModel as a path string in
+ * `WorkInfo.outputData` and is turned straight into a `File`, so this is the one place
+ * that checks where it points before deleting. Comparing the *canonical* parent rather
+ * than the path as written is what makes `conversions/../something` fail: the naive
+ * string comparison accepts it.
+ *
+ * @return true if a file was deleted. False covers both "not in staging" and "already
+ * gone", which the caller has no reason to tell apart — a `reset()` after a
+ * successful save is an ordinary second call.
+ */
+ open fun discardStaged(staged: File): Boolean {
+ val parent = staged.parentFile?.canonicalOrAbsolute() ?: return false
+ if (parent != stagingDir.canonicalOrAbsolute()) return false
+ return staged.delete()
}
+ /**
+ * Deletes staged files old enough to have been abandoned.
+ *
+ * The backstop for everything `discardStaged` cannot reach: a process killed between
+ * finishing a conversion and saving it, a worker that failed before its output ever
+ * became a `Converted` state, or a `reset()` whose delete was cancelled with the
+ * Activity. [StagingSweep] owns the rule and its reasoning.
+ *
+ * Deliberately not the `clearStaging()` this replaces. That deleted the directory's
+ * whole contents, and the convert tab, the join tab and `ConcatEngine`'s
+ * `concat_list.txt` all share this directory with no per-job namespacing, so a blanket
+ * delete could destroy a live job's file.
+ *
+ * [nowMs] is a parameter so the clock is the caller's, not a hidden global.
+ */
+ open fun sweepStaging(nowMs: Long = System.currentTimeMillis()) {
+ val dir = stagingDir
+ val listing = dir.listFiles() ?: return
+ val entries = listing.map { StagingSweep.Entry(it.name, it.lastModified()) }
+ StagingSweep.collectable(entries, nowMs).forEach { name ->
+ val file = File(dir, name)
+ // Re-read the timestamp rather than trusting the snapshot above. Between the
+ // listing and here, a worker resumed by WorkManager -- which runs in this same
+ // process -- could have started writing this very file, and unlinking an inode a
+ // running job still holds open would end with the job reporting success for a
+ // path that no longer exists.
+ if (StagingSweep.isCollectable(file.lastModified(), nowMs)) file.delete()
+ }
+ }
+
+ private fun File.canonicalOrAbsolute(): File = runCatching { canonicalFile }.getOrDefault(absoluteFile)
+
private companion object {
const val SPACE_HEADROOM_BYTES = 128L * 1024 * 1024
}
diff --git a/app/src/main/java/org/libremediaconverter/convert/StagingSweep.kt b/app/src/main/java/org/libremediaconverter/convert/StagingSweep.kt
new file mode 100644
index 0000000..63b5b4a
--- /dev/null
+++ b/app/src/main/java/org/libremediaconverter/convert/StagingSweep.kt
@@ -0,0 +1,55 @@
+package org.libremediaconverter.convert
+
+/**
+ * Decides which staging entries are old enough to collect.
+ *
+ * A pure function rather than a loop inside [OutputPublisher], for the reason
+ * [org.libremediaconverter.work.FailureOutcome] documents: the decision is worth verifying
+ * and the situation that provokes it is not reproducible. Here the untestable part is the
+ * clock — an orphan is only interesting a day after it was written, and a filesystem's
+ * mtime granularity is not something a test should be measuring. Timestamps therefore
+ * arrive as values.
+ *
+ * The rule replaces an unconditional `clearStaging()` that deleted the directory's whole
+ * contents. That was hazardous: `/conversions/` is shared by the convert tab, the
+ * join tab and [org.libremediaconverter.ffmpeg.ConcatEngine]'s `concat_list.txt`, and any
+ * two of them can be live at once, so a blanket delete could take a file out from under a
+ * running job. Age is the narrowing.
+ */
+object StagingSweep {
+
+ /** One directory entry, reduced to what the decision actually needs. */
+ data class Entry(val name: String, val lastModifiedMs: Long)
+
+ /**
+ * How stale a staging file has to be before it is assumed abandoned.
+ *
+ * Twenty-four hours, chosen against the longest a live file can plausibly go untouched
+ * rather than against how quickly cache should be reclaimed. Output files are written
+ * continuously, so a running job refreshes their mtime by itself. `concat_list.txt` is
+ * the exception — written once and then only read for the rest of the join — so the
+ * period has to exceed a whole join. WorkManager caps a single attempt at the
+ * six-hour-per-day foreground-service budget and then stops the worker, and a retry
+ * rewrites the list file, so no attempt can hold a file still for a day.
+ */
+ const val GRACE_PERIOD_MS: Long = 24L * 60 * 60 * 1000
+
+ /** The names in [entries] that may be deleted, in the order they were given. */
+ fun collectable(entries: List, nowMs: Long, gracePeriodMs: Long = GRACE_PERIOD_MS): List = entries
+ .filter { isCollectable(it.lastModifiedMs, nowMs, gracePeriodMs) }
+ .map { it.name }
+
+ /**
+ * True if a file last written at [lastModifiedMs] is collectable at [nowMs].
+ *
+ * Exposed separately so the caller can re-check a single entry immediately before
+ * deleting it, closing the window between listing a directory and acting on the list.
+ *
+ * A negative age — a file dated in the future, because the clock moved backwards — is
+ * deliberately not collectable. It carries no information about whether the file is in
+ * use, and keeping a file costs cache while deleting one can cost the user an hour of
+ * transcoding.
+ */
+ fun isCollectable(lastModifiedMs: Long, nowMs: Long, gracePeriodMs: Long = GRACE_PERIOD_MS): Boolean =
+ nowMs - lastModifiedMs >= gracePeriodMs
+}
diff --git a/app/src/main/java/org/libremediaconverter/convert/Transcoders.kt b/app/src/main/java/org/libremediaconverter/convert/Transcoders.kt
index b70b9ae..acdd654 100644
--- a/app/src/main/java/org/libremediaconverter/convert/Transcoders.kt
+++ b/app/src/main/java/org/libremediaconverter/convert/Transcoders.kt
@@ -7,6 +7,7 @@ import org.libremediaconverter.codec.AndroidDeviceCodecs
import org.libremediaconverter.ffmpeg.FFmpegEngine
import org.libremediaconverter.model.ConversionRequest
import org.libremediaconverter.model.DeviceCodecs
+import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.OutputFormat
import java.io.File
@@ -74,10 +75,34 @@ object ConversionDependencies {
@Volatile
var deviceCodecs: () -> DeviceCodecs = { AndroidDeviceCodecs.get() }
+ /**
+ * Reading an input's codecs, container and duration.
+ *
+ * Here for a reason the others are not, and the reason is worth recording rather than
+ * just working around. [MediaProbe] spawns FFprobe, and when FFmpegKit's native library
+ * cannot load its initialiser throws a bare `java.lang.Error` — which
+ * `probeWithFFprobe`'s own `catch (e: Exception)` does not catch, and which
+ * `ConversionViewModel.onInputPicked` does not catch either. Picking a file would then
+ * fail with an uncaught error rather than the "could not read this file" the code was
+ * written to give.
+ *
+ * **That is a latent production hazard, found here and deliberately not fixed here.**
+ * It cannot fire on a device that ships the `.so` files, which is every real install,
+ * so making `MediaProbe` catch `Throwable` would be a behaviour change to the pick path
+ * on the strength of a condition no user meets — its own commit, with its own test.
+ * What this seam does is narrower: it keeps the JVM out of that path, which is what
+ * makes the ViewModel reachable from a unit test at all.
+ *
+ * Instrumented tests and the app itself get the real probe, exactly as before.
+ */
+ @Volatile
+ var probe: (Context, Uri) -> InputProbe = { context, uri -> MediaProbe.probe(context, uri) }
+
fun reset() {
hardware = { Media3Engine(it) }
software = { FFmpegEngine() }
publisher = { OutputPublisher(it) }
deviceCodecs = { AndroidDeviceCodecs.get() }
+ probe = { context, uri -> MediaProbe.probe(context, uri) }
}
}
diff --git a/app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt b/app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt
index 2d09e3f..0f42ab2 100644
--- a/app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt
+++ b/app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt
@@ -8,6 +8,7 @@ import androidx.lifecycle.viewModelScope
import androidx.media3.common.util.UnstableApi
import androidx.work.WorkInfo
import androidx.work.WorkManager
+import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.MutableStateFlow
@@ -15,8 +16,8 @@ import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
+import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.InputFile
-import org.libremediaconverter.convert.OutputPublisher
import org.libremediaconverter.model.ConcatStrategy
import org.libremediaconverter.work.ConcatWorker
import java.io.File
@@ -33,10 +34,17 @@ sealed interface JoinState {
}
@UnstableApi
-class JoinViewModel(app: Application) : AndroidViewModel(app) {
+class JoinViewModel @JvmOverloads constructor(
+ app: Application,
+ /** Where [reset] runs its delete. See the same parameter on `ConversionViewModel`. */
+ private val cleanupDispatcher: CoroutineDispatcher = Dispatchers.IO,
+) : AndroidViewModel(app) {
private val workManager = WorkManager.getInstance(app)
- private val publisher = OutputPublisher(app)
+
+ // Through ConversionDependencies, like the workers, rather than `OutputPublisher(app)`
+ // direct -- see the same line in ConversionViewModel.
+ private val publisher = ConversionDependencies.publisher(app)
private val _state = MutableStateFlow(JoinState.Idle)
val state: StateFlow = _state.asStateFlow()
@@ -44,6 +52,16 @@ class JoinViewModel(app: Application) : AndroidViewModel(app) {
private var observer: Job? = null
private var activeWorkId: UUID? = null
+ /**
+ * The staged output this ViewModel is responsible for deleting.
+ *
+ * Held here rather than read back out of [_state] for the same reason as in
+ * `ConversionViewModel`: a failed [save] lands on [JoinState.Failed], which carries a
+ * message and no file, so the state machine cannot answer this on the one path that
+ * most needs it.
+ */
+ private var pendingStaged: File? = null
+
fun onInputsPicked(uris: List) {
if (uris.size < 2) {
_state.value = JoinState.Failed("Pick at least two files to join.")
@@ -85,7 +103,11 @@ class JoinViewModel(app: Application) : AndroidViewModel(app) {
if (path == null) {
JoinState.Failed("Joining reported success but produced no file.")
} else {
- JoinState.Joined(File(path), strategy)
+ val staged = File(path)
+ // Take responsibility for the file at the same moment the state
+ // starts referring to it, so the two cannot disagree.
+ pendingStaged = staged
+ JoinState.Joined(staged, strategy)
}
}
@@ -112,17 +134,34 @@ class JoinViewModel(app: Application) : AndroidViewModel(app) {
joined.staged.delete()
}
}.onSuccess {
+ // publish() already deleted it; nothing left to clean up.
+ pendingStaged = null
_state.value = JoinState.Saved("joined.mp4")
}.onFailure { e ->
+ // Deliberately NOT cleared -- see the same branch in ConversionViewModel.
+ // A failed save can leave the staged file as the only copy of the work, so
+ // it is left for a later reset() or for the sweep to collect once its age
+ // makes it certain nobody is coming back for it.
_state.value = JoinState.Failed(e.message ?: "Could not save the file.")
}
}
}
+ /**
+ * Returns to [JoinState.Idle], deleting anything staged on the way out.
+ *
+ * Best effort, not a guarantee: the delete is cancelled with [viewModelScope] if the
+ * Activity finishes first. `OutputPublisher.sweepStaging` is the backstop.
+ */
fun reset() {
observer?.cancel()
observer = null
activeWorkId = null
+ val staged = pendingStaged
+ pendingStaged = null
+ if (staged != null) {
+ viewModelScope.launch(cleanupDispatcher) { publisher.discardStaged(staged) }
+ }
_state.value = JoinState.Idle
}
diff --git a/app/src/test/java/org/libremediaconverter/convert/ConversionViewModelCleanupTest.kt b/app/src/test/java/org/libremediaconverter/convert/ConversionViewModelCleanupTest.kt
new file mode 100644
index 0000000..8b5a8a7
--- /dev/null
+++ b/app/src/test/java/org/libremediaconverter/convert/ConversionViewModelCleanupTest.kt
@@ -0,0 +1,118 @@
+package org.libremediaconverter.convert
+
+import android.app.Application
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.work.workDataOf
+import kotlinx.coroutines.Dispatchers
+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.model.InputProbe
+import org.libremediaconverter.work.ConversionWorker
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import java.io.File
+
+/**
+ * That `reset()` actually deletes — the wiring, not the tool.
+ *
+ * D2 was never that `OutputPublisher` could not delete a file. It was that "Start over"
+ * dropped the reference without calling anything. So this drives the real ViewModel through
+ * a real `WorkManager` to `Converted` and then asserts on the filesystem.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class ConversionViewModelCleanupTest {
+
+ private lateinit var app: Application
+ private lateinit var publisher: RecordingPublisher
+ private lateinit var staged: File
+
+ @Before
+ fun setUp() {
+ app = RuntimeEnvironment.getApplication()
+ publisher = RecordingPublisher(app)
+ ConversionDependencies.publisher = { publisher }
+ // MediaProbe spawns FFprobe, whose loader throws a bare java.lang.Error with no
+ // native library present. Without this the pick dies before the test starts.
+ ConversionDependencies.probe = { _, _ -> InputProbe() }
+
+ staged = publisher.createStagingFile("holiday.mp4").apply { writeBytes(ByteArray(4096)) }
+ installTestWorkManager(app, workDataOf(ConversionWorker.KEY_OUTPUT_PATH to staged.absolutePath))
+ }
+
+ @After
+ fun tearDown() {
+ ConversionDependencies.reset()
+ }
+
+ @Test
+ fun `start over on a finished conversion deletes the staged file`() {
+ val viewModel = convertedViewModel()
+ assertTrue("the conversion should have produced a staged file", staged.exists())
+
+ viewModel.reset()
+
+ assertEquals(ConversionState.Idle, viewModel.state.value)
+ assertEquals("reset() should have discarded exactly the staged file", listOf(staged), publisher.discarded)
+ assertFalse("Start over must not leave a full-size copy in cache", staged.exists())
+ }
+
+ @Test
+ fun `reset after a successful save does not try to delete again`() {
+ val viewModel = convertedViewModel()
+ viewModel.save(DESTINATION)
+ awaitState(viewModel.state, "Saved") { it is ConversionState.Saved }
+
+ // save() deletes the staged file itself, through File.delete() rather than through
+ // the publisher, so assert the disappearance as well as the absent second discard.
+ assertFalse("a successful save should have removed the staged file", staged.exists())
+
+ viewModel.reset()
+
+ // Discarding again would be a delete aimed at a path this ViewModel no longer owns.
+ assertEquals(emptyList(), publisher.discarded)
+ assertFalse(staged.exists())
+ }
+
+ @Test
+ fun `a failed save keeps the staged file, and a later reset collects it`() {
+ val viewModel = convertedViewModel()
+ publisher.publishFailure = IllegalStateException("destination volume full")
+
+ viewModel.save(DESTINATION)
+ awaitState(viewModel.state, "Failed") { it is ConversionState.Failed }
+
+ // The deliberate decision, pinned: the staged file may be the only copy of an hour
+ // of transcoding, and the destination did not receive it.
+ assertTrue("a failed save must not destroy the only copy", staged.exists())
+ assertEquals(emptyList(), publisher.discarded)
+
+ // Failed carries no file reference at all, so this only works because the handle is
+ // a ViewModel field rather than something read back out of the state machine.
+ viewModel.reset()
+
+ assertEquals(listOf(staged), publisher.discarded)
+ assertFalse(staged.exists())
+ }
+
+ /** A ViewModel driven all the way to [ConversionState.Converted]. */
+ private fun convertedViewModel(): ConversionViewModel {
+ // Unconfined so reset()'s delete runs inline instead of on a real IO thread.
+ val viewModel = ConversionViewModel(app, Dispatchers.Unconfined)
+ viewModel.onInputPicked(Uri.parse("content://test/holiday.mp4"))
+ awaitState(viewModel.state, "Ready") { it is ConversionState.Ready }
+ viewModel.convert()
+ awaitState(viewModel.state, "Converted") { it is ConversionState.Converted }
+ return viewModel
+ }
+
+ private companion object {
+ val DESTINATION: Uri = Uri.parse("content://test/destination.mp4")
+ }
+}
diff --git a/app/src/test/java/org/libremediaconverter/convert/OutputPublisherStagingTest.kt b/app/src/test/java/org/libremediaconverter/convert/OutputPublisherStagingTest.kt
new file mode 100644
index 0000000..28d8d8b
--- /dev/null
+++ b/app/src/test/java/org/libremediaconverter/convert/OutputPublisherStagingTest.kt
@@ -0,0 +1,93 @@
+package org.libremediaconverter.convert
+
+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.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import java.io.File
+
+/**
+ * What a pure function cannot say: the file is really gone.
+ *
+ * [StagingSweepTest] pins the rule; this pins the effect. Robolectric gives each test a
+ * real, empty `cacheDir` on a temp path, so this drives the actual [OutputPublisher] over
+ * the actual filesystem — the same calls `reset()` makes, without needing a ViewModel (both
+ * of those construct a `WorkManager`, which is not initialised on the JVM classpath).
+ *
+ * The instrumented suite cannot run on the development host, so this is the only place the
+ * "Start over leaks a full-size copy" defect can be caught before CI.
+ */
+@RunWith(RobolectricTestRunner::class)
+class OutputPublisherStagingTest {
+
+ private lateinit var cacheDir: File
+ private lateinit var publisher: OutputPublisher
+
+ @Before
+ fun setUp() {
+ val context = RuntimeEnvironment.getApplication()
+ cacheDir = context.cacheDir
+ publisher = OutputPublisher(context)
+ }
+
+ @Test
+ fun `discarding a staged output actually removes it`() {
+ // This is the leak in D2: convert, decline to save, tap "Start over".
+ val staged = publisher.createStagingFile("holiday.mp4").apply { writeBytes(ByteArray(4096)) }
+ assertTrue("the staged file should exist to begin with", staged.exists())
+
+ assertTrue("discard should report that it deleted the file", publisher.discardStaged(staged))
+ assertFalse("the staged file should be gone after the reset path runs", staged.exists())
+ }
+
+ @Test
+ fun `discarding a file that is already gone is not an error`() {
+ // reset() after a successful save, or two resets in a row. Neither should throw.
+ val staged = publisher.createStagingFile("already_published.mp4")
+ assertFalse(publisher.discardStaged(staged))
+ }
+
+ @Test
+ fun `a file outside the staging directory is refused`() {
+ // The handle can originate in WorkInfo.outputData, which is a string the ViewModel
+ // turns straight into a File. Nothing else checks where it points.
+ val outsider = File(cacheDir, "someone_elses.bin").apply { writeBytes(ByteArray(16)) }
+
+ assertFalse(publisher.discardStaged(outsider))
+ assertTrue("a file outside staging must survive", outsider.exists())
+ }
+
+ @Test
+ fun `a path that climbs out of the staging directory is refused`() {
+ // The naive parent check -- comparing path strings -- passes this one.
+ val outsider = File(cacheDir, "climbed_to.bin").apply { writeBytes(ByteArray(16)) }
+ val escaping = File(cacheDir, "conversions/../climbed_to.bin")
+
+ assertFalse(publisher.discardStaged(escaping))
+ assertTrue("a traversal must not delete outside staging", outsider.exists())
+ }
+
+ @Test
+ fun `the sweep collects an orphan and leaves a live job alone`() {
+ val orphan = publisher.createStagingFile("orphan.mp4").apply { writeBytes(ByteArray(4096)) }
+ val liveOutput = publisher.createStagingFile("joined.mp4").apply { writeBytes(ByteArray(4096)) }
+ val liveList = publisher.createStagingFile("concat_list.txt").apply { writeText("file 'a.mp4'\n") }
+ assertTrue(orphan.setLastModified(System.currentTimeMillis() - StagingSweep.GRACE_PERIOD_MS - 60_000))
+
+ publisher.sweepStaging()
+
+ assertFalse("an abandoned output should be collected", orphan.exists())
+ assertTrue("a live job's output must survive", liveOutput.exists())
+ assertTrue("a live join's list file must survive", liveList.exists())
+ }
+
+ @Test
+ fun `the sweep tolerates a staging directory that does not exist yet`() {
+ File(cacheDir, "conversions").deleteRecursively()
+
+ publisher.sweepStaging()
+ }
+}
diff --git a/app/src/test/java/org/libremediaconverter/convert/StagingCleanupSupport.kt b/app/src/test/java/org/libremediaconverter/convert/StagingCleanupSupport.kt
new file mode 100644
index 0000000..50a33c6
--- /dev/null
+++ b/app/src/test/java/org/libremediaconverter/convert/StagingCleanupSupport.kt
@@ -0,0 +1,106 @@
+package org.libremediaconverter.convert
+
+import android.content.Context
+import android.net.Uri
+import android.os.Looper
+import android.util.Log
+import androidx.work.Configuration
+import androidx.work.Data
+import androidx.work.ListenableWorker
+import androidx.work.Worker
+import androidx.work.WorkerFactory
+import androidx.work.WorkerParameters
+import androidx.work.testing.SynchronousExecutor
+import androidx.work.testing.WorkManagerTestInitHelper
+import kotlinx.coroutines.flow.StateFlow
+import org.robolectric.Shadows.shadowOf
+import java.io.File
+import java.util.concurrent.TimeUnit
+
+/**
+ * Shared scaffolding for the two ViewModel cleanup tests.
+ *
+ * These tests exist because [StagingSweepTest] and [OutputPublisherStagingTest] both prove
+ * the *tool* works while saying nothing about whether anything calls it — and the wiring is
+ * where D2 actually lived. Deleting the `discardStaged` line from either `reset()` left all
+ * of the earlier tests green.
+ */
+
+/**
+ * A real [OutputPublisher] that records what it was asked to discard.
+ *
+ * It still really deletes, so the assertions are about the filesystem rather than about a
+ * mock's memory. [publish] is stubbed because the SAF destination is not what these tests
+ * are about, and because making it throw is the only way to reach the failed-save branch
+ * deterministically.
+ */
+open class RecordingPublisher(context: Context) : OutputPublisher(context) {
+
+ val discarded = mutableListOf()
+
+ /** When set, [publish] throws it — the failed-save path. */
+ var publishFailure: Throwable? = null
+
+ override fun publish(staged: File, destination: Uri) {
+ publishFailure?.let { throw it }
+ }
+
+ override fun discardStaged(staged: File): Boolean {
+ discarded += staged
+ return super.discardStaged(staged)
+ }
+}
+
+/**
+ * Stands in for whichever worker is enqueued and succeeds immediately with [outputData].
+ *
+ * The real workers cannot run here: both drive FFmpeg or Media3 through native libraries
+ * that do not exist on the JVM. What the ViewModel actually needs from them is one
+ * `SUCCEEDED` `WorkInfo` carrying an output path, and that is exactly what this produces —
+ * through a real `WorkManager`, so the ViewModel's own observer, its `SUCCEEDED` branch and
+ * its cleanup handle are all the production ones.
+ */
+class SucceedingWorkerFactory(private val outputData: Data) : WorkerFactory() {
+ override fun createWorker(
+ appContext: Context,
+ workerClassName: String,
+ workerParameters: WorkerParameters,
+ ): ListenableWorker = object : Worker(appContext, workerParameters) {
+ override fun doWork(): Result = Result.success(outputData)
+ }
+}
+
+/** Installs a synchronous test WorkManager whose workers succeed with [outputData]. */
+fun installTestWorkManager(context: Context, outputData: Data) {
+ WorkManagerTestInitHelper.initializeTestWorkManager(
+ context,
+ Configuration.Builder()
+ .setMinimumLoggingLevel(Log.ASSERT)
+ .setExecutor(SynchronousExecutor())
+ .setTaskExecutor(SynchronousExecutor())
+ .setWorkerFactory(SucceedingWorkerFactory(outputData))
+ .build(),
+ )
+}
+
+/**
+ * Waits for [predicate] to hold, pumping the main looper as it goes.
+ *
+ * Both ViewModels hop to a real `Dispatchers.IO` for file metadata and resume on the main
+ * looper, which Robolectric leaves paused. So neither a bare read of `state.value` nor a
+ * single `idle()` is enough, and the timeout is generous because it only has to be longer
+ * than a few file stats — in practice this converges in milliseconds.
+ */
+fun awaitState(state: StateFlow, description: String, predicate: (T) -> Boolean): T {
+ val deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(AWAIT_TIMEOUT_SECONDS)
+ while (System.nanoTime() < deadline) {
+ shadowOf(Looper.getMainLooper()).idle()
+ val current = state.value
+ if (predicate(current)) return current
+ Thread.sleep(POLL_INTERVAL_MS)
+ }
+ throw AssertionError("Timed out waiting for $description; state was ${state.value}")
+}
+
+private const val AWAIT_TIMEOUT_SECONDS = 10L
+private const val POLL_INTERVAL_MS = 5L
diff --git a/app/src/test/java/org/libremediaconverter/convert/StagingSweepTest.kt b/app/src/test/java/org/libremediaconverter/convert/StagingSweepTest.kt
new file mode 100644
index 0000000..f63f9dc
--- /dev/null
+++ b/app/src/test/java/org/libremediaconverter/convert/StagingSweepTest.kt
@@ -0,0 +1,77 @@
+package org.libremediaconverter.convert
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+/**
+ * The orphan-collection rule.
+ *
+ * Timestamps are passed in as values rather than read off real files on purpose. The
+ * interesting part of this decision is clock arithmetic — the boundary, and a clock that
+ * has moved backwards — and a test that created real files would be measuring the
+ * filesystem's mtime granularity instead of the rule.
+ */
+class StagingSweepTest {
+
+ private val now = 1_700_000_000_000L
+ private val grace = StagingSweep.GRACE_PERIOD_MS
+
+ @Test
+ fun `an orphan older than the grace period is collectable`() {
+ // Left behind by a process that died, or by a "Start over" whose delete never ran.
+ val entries = listOf(StagingSweep.Entry("orphan.mp4", now - grace - 1))
+ assertEquals(listOf("orphan.mp4"), StagingSweep.collectable(entries, now))
+ }
+
+ @Test
+ fun `a file written moments ago is left alone`() {
+ // The in-flight guard. A live job's output has its mtime refreshed by every write,
+ // so a running conversion always looks young; deleting it would destroy the job.
+ val entries = listOf(StagingSweep.Entry("in_progress.mp4", now - 1_000))
+ assertEquals(emptyList(), StagingSweep.collectable(entries, now))
+ }
+
+ @Test
+ fun `the grace boundary itself collects`() {
+ // Pins the comparison: age >= grace collects, age one millisecond short does not.
+ assertTrue(StagingSweep.isCollectable(lastModifiedMs = now - grace, nowMs = now))
+ assertFalse(StagingSweep.isCollectable(lastModifiedMs = now - grace + 1, nowMs = now))
+ }
+
+ @Test
+ fun `a file dated in the future is left alone`() {
+ // The clock moved backwards — an RTC correction, or the user setting the date. The
+ // age is negative, which says nothing about whether the file is still in use, so
+ // the safe answer is to keep it and let a later sweep decide.
+ val entries = listOf(StagingSweep.Entry("tomorrow.mp4", now + grace))
+ assertEquals(emptyList(), StagingSweep.collectable(entries, now))
+ }
+
+ @Test
+ fun `an empty directory yields nothing`() {
+ assertEquals(emptyList(), StagingSweep.collectable(emptyList(), now))
+ }
+
+ @Test
+ fun `a mixed directory names only the orphans`() {
+ // The whole point of narrowing the old clearStaging(): a sweep that runs while a
+ // join is live must not take the list file out from under it.
+ val entries = listOf(
+ StagingSweep.Entry("orphan.mp4", now - grace - 1),
+ StagingSweep.Entry("concat_list.txt", now - 5_000),
+ StagingSweep.Entry("joined.mp4", now - 5_000),
+ StagingSweep.Entry("older_orphan.webm", now - grace * 7),
+ )
+ assertEquals(listOf("orphan.mp4", "older_orphan.webm"), StagingSweep.collectable(entries, now))
+ }
+
+ @Test
+ fun `a shorter grace period can be asked for explicitly`() {
+ // The caller owns the period; the constant is only a default.
+ val entries = listOf(StagingSweep.Entry("recent.mp4", now - 60_000))
+ assertEquals(emptyList(), StagingSweep.collectable(entries, now))
+ assertEquals(listOf("recent.mp4"), StagingSweep.collectable(entries, now, gracePeriodMs = 30_000))
+ }
+}
diff --git a/app/src/test/java/org/libremediaconverter/join/JoinViewModelCleanupTest.kt b/app/src/test/java/org/libremediaconverter/join/JoinViewModelCleanupTest.kt
new file mode 100644
index 0000000..91b7c05
--- /dev/null
+++ b/app/src/test/java/org/libremediaconverter/join/JoinViewModelCleanupTest.kt
@@ -0,0 +1,113 @@
+package org.libremediaconverter.join
+
+import android.app.Application
+import android.net.Uri
+import androidx.media3.common.util.UnstableApi
+import androidx.work.workDataOf
+import kotlinx.coroutines.Dispatchers
+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.RecordingPublisher
+import org.libremediaconverter.convert.awaitState
+import org.libremediaconverter.convert.installTestWorkManager
+import org.libremediaconverter.work.ConcatWorker
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import java.io.File
+
+/**
+ * The join tab's half of the same defect.
+ *
+ * Carried separately rather than parameterised with the convert tab, because the two are
+ * independent ViewModels that can each hold a staged file at the same time — the reason
+ * `clearStaging()` could not simply be wired up.
+ */
+@UnstableApi
+@RunWith(RobolectricTestRunner::class)
+class JoinViewModelCleanupTest {
+
+ private lateinit var app: Application
+ private lateinit var publisher: RecordingPublisher
+ private lateinit var staged: File
+
+ @Before
+ fun setUp() {
+ app = RuntimeEnvironment.getApplication()
+ publisher = RecordingPublisher(app)
+ ConversionDependencies.publisher = { publisher }
+
+ staged = publisher.createStagingFile("joined.mp4").apply { writeBytes(ByteArray(4096)) }
+ installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to staged.absolutePath))
+ }
+
+ @After
+ fun tearDown() {
+ ConversionDependencies.reset()
+ }
+
+ @Test
+ fun `start over on a finished join deletes the staged file`() {
+ val viewModel = joinedViewModel()
+ assertTrue("the join should have produced a staged file", staged.exists())
+
+ viewModel.reset()
+
+ assertEquals(JoinState.Idle, viewModel.state.value)
+ assertEquals(listOf(staged), publisher.discarded)
+ assertFalse("Start over must not leave a full-size copy in cache", staged.exists())
+ }
+
+ @Test
+ fun `reset after a successful save does not try to delete again`() {
+ val viewModel = joinedViewModel()
+ viewModel.save(DESTINATION)
+ awaitState(viewModel.state, "Saved") { it is JoinState.Saved }
+
+ // save() deletes the staged file itself, through File.delete() rather than through
+ // the publisher, so assert the disappearance as well as the absent second discard.
+ assertFalse("a successful save should have removed the staged file", staged.exists())
+
+ viewModel.reset()
+
+ assertEquals(emptyList(), publisher.discarded)
+ assertFalse(staged.exists())
+ }
+
+ @Test
+ fun `a failed save keeps the staged file, and a later reset collects it`() {
+ val viewModel = joinedViewModel()
+ publisher.publishFailure = IllegalStateException("destination volume full")
+
+ viewModel.save(DESTINATION)
+ awaitState(viewModel.state, "Failed") { it is JoinState.Failed }
+
+ assertTrue("a failed save must not destroy the only copy", staged.exists())
+ assertEquals(emptyList(), publisher.discarded)
+
+ viewModel.reset()
+
+ assertEquals(listOf(staged), publisher.discarded)
+ assertFalse(staged.exists())
+ }
+
+ /** A ViewModel driven all the way to [JoinState.Joined]. */
+ private fun joinedViewModel(): JoinViewModel {
+ // Unconfined so reset()'s delete runs inline instead of on a real IO thread.
+ val viewModel = JoinViewModel(app, Dispatchers.Unconfined)
+ viewModel.onInputsPicked(listOf(Uri.parse("content://test/a.mp4"), Uri.parse("content://test/b.mp4")))
+ awaitState(viewModel.state, "Ready") { it is JoinState.Ready }
+ viewModel.join()
+ awaitState(viewModel.state, "Joined") { it is JoinState.Joined }
+ return viewModel
+ }
+
+ private companion object {
+ val DESTINATION: Uri = Uri.parse("content://test/destination.mp4")
+ }
+}
diff --git a/app/src/test/resources/robolectric.properties b/app/src/test/resources/robolectric.properties
new file mode 100644
index 0000000..131b785
--- /dev/null
+++ b/app/src/test/resources/robolectric.properties
@@ -0,0 +1,12 @@
+# Robolectric defaults to the manifest's targetSdk, which is 37 here, and there is no
+# android-all jar for 37 -- Robolectric 4.16.1 stops at 36 and fails the whole class with
+# "Package targetSdkVersion=37 > maxSdkVersion=36" before any test body runs.
+#
+# 36 is where CI's emulator matrix already stops, for the unrelated reason in
+# docs/api-37-emulator-crash.md, so this does not widen the gap between what is verified
+# automatically and what is not: API 37 was already a manual check on the Pixel 10 Pro XL
+# before each release, and still is.
+#
+# Set here rather than in a @Config on each class so a later Robolectric test does not have
+# to rediscover it. Remove it once Robolectric ships an android-all jar for 37.
+sdk=36
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
index ffde3ac..7b96107 100644
--- a/gradle/libs.versions.toml
+++ b/gradle/libs.versions.toml
@@ -67,6 +67,19 @@ detekt = "2.0.0-alpha.6"
# so the agent version that reads Kotlin 2.2.10 bytecode is stated, not implied.
jacoco = "0.8.15"
+# Robolectric. PINNED, and it belongs with ktlint/detekt/jacoco above rather than with
+# the floating libraries, for two reasons that both point the same way.
+#
+# First, the prerelease guard in app/build.gradle.kts only covers the groups this project
+# floats -- "androidx.", "junit", "com.arthenica" -- so org.robolectric is unguarded, and
+# a "4.+" here would resolve straight to 4.17-beta-3, which is the newest thing published.
+# Second, Robolectric is not a library the app ships: it is the JVM's Android runtime, and
+# a version bump changes which android-all jar the tests execute against. That is the same
+# "a tool moved under a diff that cannot explain it" failure the linters are pinned for.
+#
+# 4.16.1 is the newest RELEASED version; the 4.17 line is beta-only at the time of writing.
+robolectric = "4.16.1"
+
[libraries]
androidx-core-ktx = { group = "androidx.core", name = "core-ktx", version.ref = "coreKtx" }
androidx-activity-compose = { group = "androidx.activity", name = "activity-compose", version.ref = "activityCompose" }
@@ -115,6 +128,12 @@ junit = { group = "junit", name = "junit", version.ref = "junit" }
androidx-junit = { group = "androidx.test.ext", name = "junit", version.ref = "androidxJunit" }
androidx-espresso-core = { group = "androidx.test.espresso", name = "espresso-core", version.ref = "espressoCore" }
+# Robolectric — an Android runtime for the JVM test source set, so file-lifecycle behaviour
+# that needs a real Context can be verified without a device. The instrumented suite cannot
+# run on the development host at all (see CLAUDE.md), so an androidTest-only red test is not
+# a TDD loop anyone here can execute.
+robolectric = { group = "org.robolectric", name = "robolectric", version.ref = "robolectric" }
+
[plugins]
# com.android.application and org.jetbrains.kotlin.plugin.compose are deliberately absent.
# They come from the root buildscript classpath (see build.gradle.kts) so that a newer KGP