JMR-dev/LibreMediaConverter
1
0
Fork 0
Code Issues 16 Pull Requests Actions Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'origin/main' into m-115-tmp

Browse Source
This commit is contained in:
JMR-dev
2026-08-25 23:24:29 -05:00
parent 68f841e84f 93ebfa6b4a
commit bf78e06969
29 changed files with 2284 additions and 121 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/scripts/e2e-report-shape.sh
+36
View File
@@ -30,16 +30,31 @@
#
# Usage:
# e2e-report-shape.sh <label> <gradle-log> [<baseline-file>]
# E2E_WEDGED_AFTER=<seconds> the wrapper timeout killed gradle after that many seconds
#
# With a third argument the run is compared against the baseline in that file (advisory mode)
# and a `::notice::` is emitted per deviation. NEVER `::error::`: the advisory job is
# `continue-on-error: true` and stays that way, and an error annotation would be a new way for
# a diagnostic to change a conclusion.
#
# WHY THE WEDGE ARRIVES AS AN ENV VAR (#118) rather than being read out of the log like every
# other field: there is nothing in the log to read. A wedge is gradle never returning, so gradle
# never printed a verdict, never printed a truncation line, and never aborted instrumentation --
# the log of a wedged leg is the log of a run that simply stops. Measured on job 98035980326:
# `expected: 59`, `received: 59`, `completed cleanly: yes`, six seconds before the wedge warning,
# for a leg that the timeout had killed 22 minutes in. Only e2e-run.sh knows, because only it
# saw `timeout` exit 124, so it says so. Guessing it from a log that ends abruptly would call
# every cancelled run a wedge.
#
# It is read as a STRING and only ever interpolated into one. `[ -n ... ]`, never `-gt`: it
# crosses a process boundary from a shell that deliberately sets it EMPTY on every non-wedge
# path, and an arithmetic test on an empty string is the header's rule four paragraphs up.
set -uo pipefail
LABEL="${1:-unknown}"
LOG="${2:-}"
BASELINE_FILE="${3:-}"
WEDGED_AFTER="${E2E_WEDGED_AFTER:-}"
SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
REPO_ROOT="$(cd -- "$SCRIPT_DIR/../.." && pwd)"
@@ -115,6 +130,13 @@ elif [ -n "$abort_received" ]; then
elif [ -n "$expected" ] && [ -z "$abort_line" ]; then
received="$expected"
received_src="the run was not truncated, so every expected test reported"
# ... unless it was killed, in which case "not truncated" is only "gradle never got as far as
# saying so". This is the branch the wedged leg in #118 took -- with no XML written yet, the
# number is what the runner was TOLD to run, and the source line said the opposite in the same
# table that called the leg clean. The number is deliberately left alone: it is still the best
# available answer, and only the claim about where it came from was wrong.
[ -n "$WEDGED_AFTER" ] \
&& received_src="no test XML was written and gradle never printed a truncation line — but the leg was killed mid-run, so this is what it was told to run, not what reported"
fi
failed="unknown"
@@ -152,6 +174,12 @@ elif [ "$no_run" = "nothing" ]; then
# "cleanly" would be a lie about a run that left no evidence it happened.
completed="unknown"
completed_src="no runner output to read"
elif [ -n "$WEDGED_AFTER" ]; then
# The wedge is checked LAST of the four, so it only ever overrides the `yes`. The two "no"s
# above are already right and name the abort, which the wedge row does not; `unknown` is
# already right too. A wedge on top of an abort is both facts, and both get printed.
completed="**no**"
completed_src="the wrapper timeout killed gradle after ${WEDGED_AFTER}s — instrumentation itself was never aborted, which is why nothing in the log says so"
else
completed="yes"
completed_src="no truncation line and no \`INSTRUMENTATION_ABORTED\`"
@@ -217,6 +245,11 @@ echo "----- RUN SHAPE (api${LABEL}) -----"
echo " expected: $expected"
echo " received: $received"
echo " failed: $failed"
# Above `completed cleanly`, because it is the line that says what happened to the leg and the
# other one only qualifies it. A reader who stops after three rows still sees it.
if [ -n "$WEDGED_AFTER" ]; then
echo " wedged: yes -- gradle was killed after ${WEDGED_AFTER}s and never returned"
fi
echo " completed cleanly: ${completed//\*/}"
if [ -n "$abort_received" ]; then
echo " received before the abort: $abort_received"
@@ -249,6 +282,9 @@ if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then
echo "| expected | $expected | $expected_src |"
echo "| received | $received | $received_src |"
echo "| failed | $failed | $failed_src |"
if [ -n "$WEDGED_AFTER" ]; then
echo "| wedged | **yes** | \`timeout\` fired after ${WEDGED_AFTER}s and killed gradle (exit 124), which is what e2e-run.sh then captured the wedge diagnostics for |"
fi
echo "| completed cleanly | $completed | $completed_src |"
if [ -n "$abort_received" ]; then
echo "| received before the abort | $abort_received | the same line — the XML above counts the truncated test as a failure, this number does not |"
.github/scripts/e2e-run.sh
+17 -3
View File
@@ -239,20 +239,34 @@ timeout -k 30s "$WEDGE_TIMEOUT" \
${E2E_EXTRA_GRADLE_ARGS:-} 2>&1 | tee "$GRADLE_LOG" || status=$?
echo "::endgroup::"
# Whether the wrapper timeout fired, decided ONCE. 124 is `timeout` saying it killed the
# command, and two places downstream need that fact: capture_wedge below, and the report, which
# otherwise calls a killed leg `completed cleanly: yes` (#118). Deriving it twice is how those
# two would drift apart -- the report would keep printing after someone changed what a wedge
# means here. It stays a string: empty on every other path, so those legs pass an empty
# E2E_WEDGED_AFTER and the report behaves exactly as before.
wedged=""
[ "$status" -eq 124 ] && wedged="$WEDGE_TIMEOUT"
# The run-shape report: expected/received/failed and whether the run finished, every time,
# green or red. It never changes `status` -- it is a diagnostic, and the header's rule about
# diagnostics applies to it as much as to every probe below.
#
# E2E_WEDGED_AFTER is the wedge, told to the report rather than left for it to infer. It cannot
# be inferred: a wedge is gradle never returning, so gradle printed no verdict at all, and the
# log the report reads looks like a run that simply stopped. Only this script knows the
# difference, because only this script saw the exit status.
#
# The baseline argument, and only it, turns on the comparison, and only the advisory API 37 job
# passes E2E_ADVISORY=1. Comparing on the gating legs would announce a deviation on all five of
# them every run, since they run the whole suite rather than the marked three. They still get
# the report: a truncated run reporting fewer results than it ran is what #108 looks like, and
# `completed cleanly` is the field that shows it.
if [ "${E2E_ADVISORY:-}" = "1" ]; then
bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" \
E2E_WEDGED_AFTER="$wedged" bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" \
"$REPO_ROOT/app/src/androidTest/java/org/libremediaconverter/FailsOnEmulatorApi37.kt" || true
else
bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" || true
E2E_WEDGED_AFTER="$wedged" bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" || true
fi
if [ "$status" -eq 0 ]; then
@@ -260,7 +274,7 @@ if [ "$status" -eq 0 ]; then
exit 0
fi
if [ "$status" -eq 124 ]; then
if [ -n "$wedged" ]; then
capture_wedge "api${LABEL}"
else
echo "::error::E2E api${LABEL} failed (exit $status)"
CLAUDE.md
+9 -2
View File
@@ -76,11 +76,11 @@ days. Read it as the current answer, and see the git history if you need the old
`angle_indirect` and `swangle_indirect` all boot, while `auto`, `off`, `guest` and
`swiftshader_indirect` do not. `docs/local-emulator.md` has the evidence and the per-API renderer
table.
- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. **Three** of the 59 instrumented
- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. **Three** of the 60 instrumented
tests cannot pass on that image, for two unrelated reasons: two Media3 hardware transcodes fail
inside the emulator's own `c2.goldfish.h264.decoder`, and one SAF test takes the framework down
when it rotates the display. All three carry `@FailsOnEmulatorApi37` and run in a separate
`continue-on-error` job; the gating leg runs the other 56.
`continue-on-error` job; the gating leg runs the other 57.
That job is still called `E2E API 37 Media3 hardware transcode (advisory)`, which no longer
describes everything in it. The name is kept deliberately — it is not a required context and
@@ -98,6 +98,13 @@ days. Read it as the current answer, and see the git history if you need the old
is written anyway and says nothing about it — `.github/scripts/e2e-report-shape.sh` is where that
is measured and explained.
Every leg prints that table, advisory or not, and **on the wedge path it also carries a `wedged:`
row** (#118). `completed cleanly` only ever meant "instrumentation was not aborted", which stays
true of a leg the `WEDGE_TIMEOUT` killed 22 minutes in — so without that row the table read
`received: 59, completed cleanly: yes` for a leg that had just died. The wedge is passed to the
report as `E2E_WEDGED_AFTER` by `e2e-run.sh`, which is the only thing that can know it: a wedge
is gradle never returning, so the log it left says nothing about it.
Still true, and the reason the advisory job is not simply deleted: **API 37 needs a manual check on
the Pixel 10 Pro XL before each release.** Those three tests are the one thing CI cannot answer
for.
app/src/androidTest/java/org/libremediaconverter/convert/Media3EngineTest.kt
+75
View File
@@ -11,15 +11,26 @@ import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withTimeout
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.FailsOnEmulatorApi37
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.AudioPlan
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.ConversionRequest
import org.libremediaconverter.model.CopyPlanner
import org.libremediaconverter.model.InputKind
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.OutputFormat
import org.libremediaconverter.model.OutputSpec
import org.libremediaconverter.model.VideoCodec
import org.libremediaconverter.model.VideoPlan
import java.io.File
import java.util.concurrent.CancellationException
import java.util.concurrent.Executors
import java.util.concurrent.TimeUnit
@@ -170,6 +181,63 @@ class Media3EngineTest {
)
}
/**
* The builders that used to throw where nothing could catch them.
*
* `EditedMediaItem.Builder` rejects a composition with both tracks removed —
* checkState("Audio and video cannot both be removed") — and the engine builds it on its own
* HandlerThread. That build sat *between* two narrow `runCatching` blocks, one around
* `buildTransformer` and one around `start`, so the exception reached the thread's uncaught
* handler and took the process with it while the continuation was never resumed.
*
* `ContainerCapabilities.validate` now refuses the spec that gets here from the picker; this
* is the other half — the engine surviving a request that arrives without being validated.
* Deliberately not `@FailsOnEmulatorApi37`: nothing here decodes or encodes, so no emulator
* codec is involved. The builder refuses the input before any media is touched.
*/
@Test
fun aPlanThatRemovesBothTracksFailsInsteadOfKillingTheProcess() {
val request = ConversionRequest(
spec = OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE),
probe = InputProbe(
videoCodec = null,
audioCodec = "mp3",
hasVideo = false,
container = Container.MP3,
kind = InputKind.AUDIO_ONLY,
),
)
// Asserted rather than assumed: ConversionRequest's default probe says hasVideo = true,
// and with it this same spec plans to (Encode, Drop) and nothing throws at all — which
// would make the whole test vacuous without a word of warning.
val plan = CopyPlanner.plan(request.spec, request.probe)
assertEquals(VideoPlan.Drop, plan.video)
assertEquals(AudioPlan.Drop, plan.audio)
val failure = runCatching {
runBlocking {
withTimeout(BUILDER_TIMEOUT_MS) {
engine.transcode(Uri.fromFile(input), output, request) {}
}
}
}.exceptionOrNull()
// Two assertions, and the second is not pedantry. withTimeout raises
// TimeoutCancellationException, and `java.util.concurrent.CancellationException` *extends*
// IllegalStateException — so testing only the type below would call an unresumed
// continuation a pass. A hang is the other half of this defect and every bit as bad as the
// crash: the worker would sit holding a foreground service forever.
assertFalse(
"the continuation was never resumed — the failure escaped instead of being reported: " +
"$failure",
failure is CancellationException,
)
assertTrue(
"the builder's refusal must surface as a failed job, not a dead process; got $failure",
failure is IllegalStateException,
)
}
private fun durationMsOf(file: File): Long {
val extractor = MediaExtractor()
return try {
@@ -211,5 +279,12 @@ class Media3EngineTest {
private companion object {
const val TIMEOUT_SECONDS = 120L
/**
* Short on purpose. Nothing is decoded or encoded on this path — the builder refuses the
* input outright — so anything approaching this is a hang, which is what the test is
* looking for.
*/
const val BUILDER_TIMEOUT_MS = 30_000L
}
}
app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt
+146 -25
View File
@@ -101,7 +101,42 @@ sealed interface ConversionState {
val mimeType: String = "",
) : ConversionState
data class Saved(val displayName: String) : ConversionState
data class Failed(val message: String) : ConversionState
/**
* The job, or the save that followed it, could not be finished.
*
* [retry] is non-null for exactly one cause: a [ConversionViewModel.save] whose copy to the
* user's destination threw. That save deliberately keeps the staged file -- it can be the only
* copy of an hour of transcoding -- and this is what lets the screen offer it again. Every
* other failure leaves it null, because there is nothing staged to offer: a transcode that
* died produced no output, and a save that found the file gone has nothing left to save.
*
* Nullable rather than a `SaveFailed` state of its own. What the screen does with the message
* is identical either way, so a second variant would make every exhaustive `when` grow an arm
* that duplicates this one.
*
* A view of the file, not a second owner of it -- see [PendingSave].
*/
data class Failed(val message: String, val retry: PendingSave? = null) : ConversionState
}
/**
* The staged output a save would target from this state, or null when there is nothing to save.
*
* One function for two callers that have to agree. [ConversionViewModel.save] picks the file to
* copy with it, and `ConverterScreen` registers its `CreateDocument` contract with the MIME type
* it returns; when those two read the state separately, a retry offered after a failed save opened
* the dialog with the *picker's* current type instead of the finished job's -- wrong for any job
* whose spec has been edited since, and for every reattached job, whose spec was never in these
* settings at all.
*
* Top-level and `internal` rather than a member of the ViewModel, so the screen can call it
* without one -- which is also what makes the derivation testable on the JVM.
*/
internal fun ConversionState.pendingSave(): PendingSave? = when (this) {
is ConversionState.Converted -> PendingSave(staged, suggestedName, mimeType)
is ConversionState.Failed -> retry
else -> null
}
@UnstableApi
@@ -158,13 +193,34 @@ class ConversionViewModel @JvmOverloads constructor(
private var observer: Job? = null
private var activeWorkId: UUID? = null
/**
* Who is allowed to write to this screen — see [ScreenOwnership] for the rule and why
* cancelling the superseded coroutine is not one.
*
* Every write below that lands after a suspension point is guarded by it: the two in
* [onInputPicked] and the one in [observe].
*
* [save] is the one left out, deliberately — and not because it is safe in both directions.
* Nothing can overwrite what it writes: it is reachable only from [ConversionState.Converted]
* or a [ConversionState.Failed] carrying its file, so the only observation that could belongs
* to a job already in a terminal state, which will not emit again. What it can still do is
* land on top of a [reset] taken while its copy was in flight, putting `Saved` on a screen the
* user has just cleared. Guarding it would drop that write instead, reporting nothing for a
* file that may genuinely have reached the user's destination. Which of those two is right is
* a question about what the screen should offer during a save, not about this race, so it is
* filed as issue #123 rather than decided here in passing.
*/
private val ownership = ScreenOwnership()
/**
* 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.
* A field rather than something read back out of [_state], and still one now that
* [ConversionState.Failed] carries a [PendingSave] after a failed [save]. That handle is a
* view for the screen to offer a retry through; this one is the single reference [reset]
* deletes through, and keeping the two apart is what stops a second owner appearing. Reading
* the file back out of the state machine instead would mean trusting every state that has no
* file -- `Idle`, `Saved`, a transcode failure -- to say so.
*/
private var pendingStaged: File? = null
@@ -207,6 +263,10 @@ class ConversionViewModel @JvmOverloads constructor(
* `Data` — see [ConversionState.Converted].
*/
private fun reattach() {
// Read before the launch, and before the query it is about to suspend in. This is the
// claim the answer will belong to: anything the user does from here on supersedes it, and
// reading it on the far side of the query would read whatever superseded it instead.
val token = ownership.current
viewModelScope.launch {
val reattachment = Reattachment.choose(
workManager.jobSnapshots(
@@ -217,8 +277,17 @@ class ConversionViewModel @JvmOverloads constructor(
// The query suspends, so by now the user may have picked a file or started a
// conversion of their own. Either owns the screen; reattaching over it would throw
// away what they just did. Both this check and the assignment below run on the main
// dispatcher with no suspension point between them, so nothing can interleave.
// away what they just did.
//
// This catches a pick that has already *landed*, and only that. It used to claim that
// "both this check and the assignment below run on the main dispatcher with no
// suspension point between them, so nothing can interleave" — which was the exact
// opposite of what happens. There is no assignment below. There is observe(), which
// launches a *separate* coroutine that must suspend on `collect` before it can write
// anything, so the check happens at one moment and the write lands at another with a
// whole pick able to fit in between. That was issue #49, and believing this comment is
// why it read as flaky CI for two days. What actually holds the line is the token
// observe() carries: see [ScreenOwnership].
if (_state.value !is ConversionState.Idle || activeWorkId != null) return@launch
// Only a job that is the sole explanation for its staged file gets to name the input.
@@ -244,7 +313,7 @@ class ConversionViewModel @JvmOverloads constructor(
activeWorkId = reattachment.job.id
// No initial state of our own: the flow's first emission carries the job's real
// state, so observe() maps it exactly as it would for a conversion started here.
observe(reattachment.job.id, input, cancelled = ConversionState.Idle)
observe(reattachment.job.id, input, cancelled = ConversionState.Idle, token = token)
}
}
@@ -260,25 +329,34 @@ class ConversionViewModel @JvmOverloads constructor(
fun setQuality(quality: QualityTier) = _settings.update { it.copy(quality = quality) }
fun setEnginePreference(preference: EnginePreference) = _settings.update { it.copy(enginePreference = preference) }
/**
* The tap is the claim, which is why [ScreenOwnership.claim] is called here and not inside the
* `launch`. A claim made in the coroutine would only be immediate for as long as
* `Dispatchers.Main.immediate` happened to run it inline, and a deferred claim leaves the same
* gap this closes: it is the difference between the user owning the screen from the moment
* they tapped and owning it from whenever their coroutine got around to running.
*/
fun onInputPicked(uri: Uri) {
val token = ownership.claim()
viewModelScope.launch {
// Both the metadata query and the probe touch disk, and the probe spawns FFprobe.
// Neither belongs on the main thread.
val file = withContext(pickDispatcher) { InputQuery.describe(getApplication(), uri) }
// Every write below the hop above is guarded, this one included: two picks in quick
// succession suspend here together, and without this the slower one would land last
// and put the file the user did not choose on screen.
if (!ownership.stillHeldBy(token)) return@launch
// Show the file as soon as its name and size are known. Probing now runs FFprobe on
// every pick, which is a native process spawn, and making the whole screen wait on it
// would read as the app having ignored the tap.
_state.value = ConversionState.Ready(file)
val probe = withContext(pickDispatcher) { probeOrUnreadable(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) {
ConversionState.Ready(file.copy(probe = probe))
} else {
current
}
}
// Only fill in the probe if the user has not moved on in the meantime. The claim is
// what says whether they have -- it covers a second pick of the same URI, which a
// comparison of URIs cannot, and every state a later claim could have written.
if (!ownership.stillHeldBy(token)) return@launch
_state.value = ConversionState.Ready(file.copy(probe = probe))
}
}
@@ -329,10 +407,13 @@ class ConversionViewModel @JvmOverloads constructor(
quality = settings.quality,
enginePreference = settings.enginePreference,
)
// Tapping Convert claims the screen for this job, which is what supersedes the pick's
// still-in-flight probe and any reattachment that has not finished asking.
val token = ownership.claim()
activeWorkId = request.id
workManager.enqueue(request)
_state.value = ConversionState.Converting(input, 0)
observe(request.id, input)
observe(request.id, input, token = token)
}
/**
@@ -340,12 +421,27 @@ class ConversionViewModel @JvmOverloads constructor(
* picked file, ready to convert again. For one picked up by [reattach] there is no picked
* file — the URI that job holds belongs to a process that no longer exists — so it lands
* on Idle instead, rather than offering a Convert button over a file nothing can open.
* @param token the claim this observation belongs to. Nothing here can write until `collect`
* has resumed with a `WorkInfo`, which is some time after the caller decided to observe, so
* the claim is checked again at the last possible moment rather than trusted from then. This
* is issue #49's fix and the only thing standing between a superseded observation and the
* user's screen — see [ScreenOwnership].
*/
private fun observe(id: UUID, input: InputFile, cancelled: ConversionState = ConversionState.Ready(input)) {
private fun observe(
id: UUID,
input: InputFile,
cancelled: ConversionState = ConversionState.Ready(input),
token: Long,
) {
observer?.cancel()
observer = viewModelScope.launch {
workManager.getWorkInfoByIdFlow(id).collect { info ->
if (info == null) return@collect
// Ahead of the `when`, not merely ahead of the assignment: the SUCCEEDED branch
// takes ownership of the staged file, and a superseded observation must not do
// that either. The state and `pendingStaged` are meant to refer to the same file
// or to no file, and this is where that stays true.
if (!ownership.stillHeldBy(token)) return@collect
_state.value = when (info.state) {
WorkInfo.State.RUNNING -> ConversionState.Converting(
input,
@@ -426,35 +522,50 @@ class ConversionViewModel @JvmOverloads constructor(
/**
* Copies the staged result out to the destination the user picked.
*
* Reached from [ConversionState.Converted] and again from a [ConversionState.Failed] that an
* earlier save left carrying its file. [pendingSave] is what makes those one call rather than
* two, so a retry cannot drift from the first attempt in what it copies or what it calls it.
*
* The existence check is not redundant with the one reattachment already made. That one ran
* inside a tag query which, for a result offered on launch, can be hours older than the tap —
* and `cacheDir` is exactly the directory the OS empties when it wants space, which is also
* what the sweep does to anything a day old. Without it the file's absence arrived as
* `staged.inputStream()` throwing, and `e.message` put a raw ENOENT path on screen.
* `staged.inputStream()` throwing, and `e.message` put a raw ENOENT path on screen. A retry
* meets that same check a second time, which is the point of reusing it here.
*/
fun save(destination: Uri) {
val converted = _state.value as? ConversionState.Converted ?: return
if (!converted.staged.isFile) {
val pending = _state.value.pendingSave() ?: return
if (!pending.staged.isFile) {
// No retry handle: the file such a state would offer again is exactly the one that
// has gone, so carrying it would put a button on screen that cannot do anything.
_state.value = ConversionState.Failed(STAGED_FILE_GONE_MESSAGE)
return
}
viewModelScope.launch {
runCatching {
withContext(Dispatchers.IO) {
publisher.publish(converted.staged, destination)
converted.staged.delete()
publisher.publish(pending.staged, destination)
pending.staged.delete()
}
}.onSuccess {
// publish() already deleted it; nothing left to clean up.
pendingStaged = null
_state.value = ConversionState.Saved(converted.suggestedName)
_state.value = ConversionState.Saved(pending.suggestedName)
}.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.")
//
// `pending` rides on the state so the screen can offer that file again. It used
// to live only in `pendingStaged`, where nothing on screen could reach it -- so
// the single button this branch rendered was "Start over", which deletes the very
// file the paragraph above goes out of its way to keep. It is `pending` rather
// than a fresh handle for the second failure's sake: a retry that fails again
// lands back here still carrying the file, not on a bare Failed that would take
// the offer away.
_state.value = ConversionState.Failed(e.message ?: "Could not save the file.", pending)
}
}
}
@@ -468,8 +579,18 @@ class ConversionViewModel @JvmOverloads constructor(
* 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.
*
* **It still deletes from a [ConversionState.Failed] carrying a [PendingSave], and that is a
* decision rather than something inherited.** Deletion is acceptable there only because the
* alternative was offered first: the screen puts "Try saving again" directly above this
* button, so reaching it is the user saying the work is not worth keeping. Until that button
* existed, this delete was the only thing a failed save could lead to — which was the defect.
*/
fun reset() {
// Start over is a claim like any other. The cancel below is a request honoured at the next
// suspension point, so a collector already on its way to a write has nothing left to
// honour it at; the claim is what actually stops that write landing on top of Idle.
ownership.claim()
observer?.cancel()
observer = null
activeWorkId = null
app/src/main/java/org/libremediaconverter/convert/ConverterScreen.kt
+34 -8
View File
@@ -73,8 +73,11 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
// stands: some providers rewrite a document's extension to match it, so an MP3 offered as
// video/webm can arrive with the wrong one. Read straight off the collected state, so this
// recomposes because it depends on that rather than because an unrelated line happens to.
// Through pendingSave() rather than a cast to Converted, so a retry offered after a failed
// save opens the dialog with the type its first attempt used -- the cast answered null for a
// Failed, and the fallback below is the current picker, which a reattached job never set.
// Remembered against the type so the launcher re-registers only when it actually changes.
val destinationMime = (state as? ConversionState.Converted)?.mimeType ?: settings.spec.mimeType
val destinationMime = state.pendingSave()?.mimeType ?: settings.spec.mimeType
val chooseDestination = rememberLauncherForActivityResult(
remember(destinationMime) { ActivityResultContracts.CreateDocument(destinationMime) },
) { uri -> uri?.let(viewModel::save) }
@@ -325,13 +328,36 @@ internal fun ConverterScreenContent(
color = MaterialTheme.colorScheme.error,
style = MaterialTheme.typography.bodyMedium,
)
Button(
onClick = actions.onReset,
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight)
.testTag(TestTags.START_OVER),
) { Text("Start over") }
val retry = s.retry
if (retry == null) {
// Nothing was staged, so "Start over" is the whole of what is on
// offer and stays the primary button.
Button(
onClick = actions.onReset,
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight)
.testTag(TestTags.START_OVER),
) { Text("Start over") }
} else {
Button(
onClick = { actions.onSave(retry.suggestedName) },
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight)
.testTag(TestTags.RETRY_SAVE),
) { Text("Try saving again") }
// Start over still deletes the file this state is carrying, and that
// is deliberate: `reset()` is what stops a full-size output sitting in
// cache until the sweep. What makes the delete acceptable is the
// button above it. Deletion is the user's choice only once the
// alternative has been offered -- and until that button existed, this
// one was the only thing a failed save could lead to.
OutlinedButton(
onClick = actions.onReset,
modifier = Modifier.fillMaxWidth().testTag(TestTags.START_OVER),
) { Text("Start over") }
}
}
}
}
app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt
+55 -33
View File
@@ -68,42 +68,64 @@ class Media3Engine(private val context: Context) : HardwareTranscoder {
): Unit = suspendCancellableCoroutine { cont ->
val plan = CopyPlanner.plan(request.spec, request.probe)
handler.post {
val transformer = runCatching { buildTransformer(plan, cont) }
.getOrElse {
cont.resumeWithException(it)
return@post
}
// Dropping the tracks the target does not have is what stops an audio-only export
// from carrying a re-encoded video track. Without setRemoveVideo, asking for M4A
// produced an HEVC stream in a file named .m4a.
val item = EditedMediaItem.Builder(MediaItem.fromUri(input))
.setRemoveVideo(plan.video == VideoPlan.Drop)
.setRemoveAudio(plan.audio == AudioPlan.Drop)
.build()
// A Composition is the only way to ask for transmuxing; the plain
// start(EditedMediaItem, path) overload always re-encodes. This is the remux path.
val composition = Composition.Builder(EditedMediaItemSequence.Builder(item).build())
.setTransmuxVideo(plan.video == VideoPlan.Copy)
.setTransmuxAudio(plan.audio == AudioPlan.Copy)
.build()
cont.invokeOnCancellation {
// cancel() has the same single-thread requirement as start().
handler.post { runCatching { transformer.cancel() } }
}
runCatching { transformer.start(composition, output.absolutePath) }
.onFailure {
cont.resumeWithException(it)
return@post
}
pollProgress(transformer, cont, onProgress)
// One guard around the whole body, deliberately.
//
// This used to be two narrow ones — around `buildTransformer` and around
// `transformer.start` — with the two Media3 builders sitting unguarded between them.
// On this thread that is not a small gap: nothing here has a caller to throw back to,
// so an escaping exception reaches the HandlerThread's uncaught handler and takes the
// process down, while [cont] is never resumed either way. `EditedMediaItem.Builder`
// does exactly that for a plan that drops both tracks
// ("Audio and video cannot both be removed"), which a queued job can still carry.
// Widening the guard costs nothing on success and turns every such refusal into a
// failed job with a reason.
runCatching { startExport(input, output, plan, cont, onProgress) }
.onFailure { if (cont.isActive) cont.resumeWithException(it) }
}
}
/**
* Builds the export and hands it to Transformer. Runs on the HandlerThread; may throw.
*
* Everything Transformer's single-thread contract covers lives here, so that the caller has
* exactly one place to catch. Returning normally means the export is running and [cont] belongs
* to the listener; throwing means it never started and the caller owns resuming.
*/
private fun startExport(
input: Uri,
output: File,
plan: ConversionPlan,
cont: CancellableContinuation<Unit>,
onProgress: (Int) -> Unit,
) {
val transformer = buildTransformer(plan, cont)
// Dropping the tracks the target does not have is what stops an audio-only export
// from carrying a re-encoded video track. Without setRemoveVideo, asking for M4A
// produced an HEVC stream in a file named .m4a.
val item = EditedMediaItem.Builder(MediaItem.fromUri(input))
.setRemoveVideo(plan.video == VideoPlan.Drop)
.setRemoveAudio(plan.audio == AudioPlan.Drop)
.build()
// A Composition is the only way to ask for transmuxing; the plain
// start(EditedMediaItem, path) overload always re-encodes. This is the remux path.
val composition = Composition.Builder(EditedMediaItemSequence.Builder(item).build())
.setTransmuxVideo(plan.video == VideoPlan.Copy)
.setTransmuxAudio(plan.audio == AudioPlan.Copy)
.build()
// Registered before start(), so a cancellation racing the export always finds a
// transformer to cancel.
cont.invokeOnCancellation {
// cancel() has the same single-thread requirement as start().
handler.post { runCatching { transformer.cancel() } }
}
transformer.start(composition, output.absolutePath)
pollProgress(transformer, cont, onProgress)
}
/**
* @throws IllegalArgumentException if [plan] names a container Media3 cannot mux. That is a
* routing bug rather than a runtime condition — [org.libremediaconverter.model.ConversionRouter]
app/src/main/java/org/libremediaconverter/convert/OutputPublisher.kt
+18
View File
@@ -23,6 +23,24 @@ const val STAGED_FILE_GONE_MESSAGE: String =
"The finished file is no longer in the cache, so there is nothing left to save. " +
"Start over to make it again."
/**
* A staged file that is still there to be saved, and everything the save dialog needs to offer it.
*
* The three travel together because a save cannot be repeated without all of them: the file to
* copy, the name to suggest, and the MIME type `CreateDocument` has to be registered with. None of
* them can be rederived from the pickers once the job is over -- they come from the job's own
* output `Data`, and a reattached job's spec was never in the current settings at all.
*
* Kept next to [STAGED_FILE_GONE_MESSAGE] for the same reason it is: both ViewModels need it and
* staging is what it is about.
*
* **A view of the staged file, never an owner of it.** The delete still runs through each
* ViewModel's own `pendingStaged` field, so a state carrying one of these can be dropped without
* losing the only reference -- which is what keeps "a `Failed` that carries a file" from being a
* new way to leak one.
*/
data class PendingSave(val staged: File, val suggestedName: String, val mimeType: String)
/**
* Staging and publication of conversion output.
*
app/src/main/java/org/libremediaconverter/convert/ScreenOwnership.kt
+57
View File
@@ -0,0 +1,57 @@
package org.libremediaconverter.convert
/**
* Which of the things writing to a screen is still allowed to.
*
* Both ViewModels are a state machine written to from several coroutines that each suspend before
* they write: a pick hops to a dispatcher for the metadata query, a reattachment hops for the tag
* query, and an observation of a WorkManager job cannot write at all until its `collect` has
* resumed with a `WorkInfo`. Whoever resumes last wins, which is how issue #49 let a finished job
* from an earlier session take a screen the user had already picked a file on.
*
* The rule this makes enforceable is one line: **every write that lands after a suspension point
* checks the claim it was made under, and drops itself if that claim has been superseded.** The
* claim is taken synchronously, when the user acts; the check happens immediately before the
* write. Superseded work is *dropped*, not reordered — a dropped write cannot come back later.
*
* Cancelling the superseded coroutine is not a substitute and was never going to be. `Job.cancel`
* is a request, honoured at the next suspension point; a collector that has already resumed and is
* on its way to `_state.value = …` has no suspension point left to honour it at, so the write
* lands anyway. Cancellation also cannot help at all in the case #49 actually reported, where
* nothing supersedes the observation until after it has been launched. Both ViewModels still
* cancel their old observer, because leaving a collector running is a leak — but the guarantee
* does not rest on it.
*
* **Confined to the main dispatcher, and that confinement is the atomicity argument.** Every
* claim and every check runs there, with no suspension point between a check and the write it
* guards, so a claim can never land between the two. Nothing here is synchronized and nothing is
* `@Volatile`: making the field visible across threads would invite exactly the off-main use this
* cannot support, and would replace an argument that holds with one that only looks like it does.
*/
internal class ScreenOwnership {
private var claims = 0L
/**
* The claim in force now.
*
* Read by work that is about to suspend and will want to know, when it comes back, whether
* the screen it was reading is still the screen it is writing to. Read it *before* the
* suspension, not after — reading it afterwards would return whatever claim superseded it,
* which is the bug rather than the check for it.
*/
val current: Long get() = claims
/**
* Takes the screen, invalidating every write still in flight under an older claim.
*
* Called synchronously from the user's action rather than from inside the coroutine it
* starts. A claim made inside a `launch` is only immediate while the dispatcher happens to
* run it inline, and a deferred claim is no claim at all: it would leave the same gap this
* exists to close.
*/
fun claim(): Long = ++claims
/** Whether [token] is still the claim in force, and may therefore write. */
fun stillHeldBy(token: Long): Boolean = token == claims
}
app/src/main/java/org/libremediaconverter/join/JoinScreen.kt
+30 -10
View File
@@ -46,9 +46,10 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
// The contract's MIME type comes from the finished job rather than from a literal: some
// providers rewrite a document's extension to match it, so naming MP4 for a join that is not
// one can hand the user a file the extension lies about. Remembered against that type so the
// launcher re-registers only when it actually changes.
val destinationMime = (state as? JoinState.Joined)?.mimeType ?: ConcatWorker.DEFAULT_FORMAT.mimeType
// one can hand the user a file the extension lies about. Through pendingSave() rather than a
// cast to Joined, so a retry after a failed save opens with the type its first attempt used.
// Remembered against that type so the launcher re-registers only when it actually changes.
val destinationMime = state.pendingSave()?.mimeType ?: ConcatWorker.DEFAULT_FORMAT.mimeType
val chooseDestination = rememberLauncherForActivityResult(
remember(destinationMime) { ActivityResultContracts.CreateDocument(destinationMime) },
) { uri -> uri?.let(viewModel::save) }
@@ -226,13 +227,32 @@ internal fun JoinScreenContent(state: JoinState, actions: JoinActions, modifier:
color = MaterialTheme.colorScheme.error,
style = MaterialTheme.typography.bodyMedium,
)
Button(
onClick = actions.onReset,
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight)
.testTag(TestTags.START_OVER),
) { Text("Start over") }
val retry = s.retry
if (retry == null) {
// Nothing staged, so "Start over" is all there is and stays primary.
Button(
onClick = actions.onReset,
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight)
.testTag(TestTags.START_OVER),
) { Text("Start over") }
} else {
Button(
onClick = { actions.onSave(retry.suggestedName) },
modifier = Modifier
.fillMaxWidth()
.height(PrimaryButtonHeight)
.testTag(TestTags.RETRY_SAVE),
) { Text("Try saving again") }
// Start over still deletes the carried file, for the reason the
// converter screen writes out next to the same pair of buttons:
// the delete is a choice only once the alternative is on screen.
OutlinedButton(
onClick = actions.onReset,
modifier = Modifier.fillMaxWidth().testTag(TestTags.START_OVER),
) { Text("Start over") }
}
}
}
}
app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt
+119 -16
View File
@@ -18,7 +18,9 @@ import kotlinx.coroutines.withContext
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.InputFile
import org.libremediaconverter.convert.InputQuery
import org.libremediaconverter.convert.PendingSave
import org.libremediaconverter.convert.STAGED_FILE_GONE_MESSAGE
import org.libremediaconverter.convert.ScreenOwnership
import org.libremediaconverter.model.ConcatStrategy
import org.libremediaconverter.work.ConcatWorker
import org.libremediaconverter.work.JobTags
@@ -44,7 +46,30 @@ sealed interface JoinState {
val mimeType: String,
) : JoinState
data class Saved(val displayName: String) : JoinState
data class Failed(val message: String) : JoinState
/**
* The join, or the save that followed it, could not be finished.
*
* [retry] is non-null for exactly one cause, and for the same reason as on
* `ConversionState.Failed`: a [JoinViewModel.save] whose copy to the user's destination threw
* keeps the staged file, and this is what lets the screen offer it again. Every other failure
* leaves it null — a join that died produced no output, and a save that found the file gone
* has nothing left to save.
*/
data class Failed(val message: String, val retry: PendingSave? = null) : JoinState
}
/**
* The staged output a save would target from this state, or null when there is nothing to save.
*
* The join tab's half of `ConversionState.pendingSave`, and it exists for the same reason: `save`
* and `JoinScreen`'s `CreateDocument` registration both have to answer this question, and answering
* it twice is how a retry ends up opening the dialog with a type the finished job never chose.
*/
internal fun JoinState.pendingSave(): PendingSave? = when (this) {
is JoinState.Joined -> PendingSave(staged, suggestedName, mimeType)
is JoinState.Failed -> retry
else -> null
}
@UnstableApi
@@ -52,6 +77,15 @@ class JoinViewModel @JvmOverloads constructor(
app: Application,
/** Where [reset] runs its delete. See the same parameter on `ConversionViewModel`. */
private val cleanupDispatcher: CoroutineDispatcher = Dispatchers.IO,
/**
* Where the metadata query behind a pick runs. See the same parameter on `ConversionViewModel`.
*
* The join side had no such seam, and the gap was not cosmetic: the one write `onInputsPicked`
* makes lands *after* this hop, so a test that wants to ask what happens while a pick is still
* in flight had no way to hold one there. Issue #49's race is exactly that question, and it
* went unasked on this side for as long as the dispatcher was a literal.
*/
private val pickDispatcher: CoroutineDispatcher = Dispatchers.IO,
) : AndroidViewModel(app) {
private val workManager = WorkManager.getInstance(app)
@@ -66,13 +100,28 @@ class JoinViewModel @JvmOverloads constructor(
private var observer: Job? = null
private var activeWorkId: UUID? = null
/**
* Who is allowed to write to this screen -- see [ScreenOwnership], which carries the rule and
* the reason cancelling the superseded coroutine is not one.
*
* The convert side had issue #49 reported against it four times in two days; this side has the
* identical shape and was never reported, because nothing was watching. Every write below that
* lands after a suspension point is guarded: the one in [onInputsPicked] and the one in
* [observe]. [save] is the one left out, deliberately and with the same limit its counterpart
* in `ConversionViewModel` spells out: nothing can overwrite what it writes, but it can still
* land on top of a [reset] taken while its copy was in flight. Which way that should go is a
* question about the save screen rather than about this race -- issue #123.
*/
private val ownership = ScreenOwnership()
/**
* 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.
* `ConversionViewModel`, and still held here now that [JoinState.Failed] carries a
* [PendingSave] after a failed [save]: that handle is a view for the screen to offer a retry
* through, this one is the single reference [reset] deletes through, and keeping the two
* apart is what stops a second owner of the file appearing.
*/
private var pendingStaged: File? = null
@@ -91,6 +140,10 @@ class JoinViewModel @JvmOverloads constructor(
* the rules about which job and why.
*/
private fun reattach() {
// Read before the launch, and before the query it is about to suspend in: this is the
// claim the answer belongs to. Reading it on the far side of the query would read whatever
// superseded it, which is the bug rather than the check for it.
val token = ownership.current
viewModelScope.launch {
val reattachment = Reattachment.choose(
workManager.jobSnapshots(
@@ -100,8 +153,15 @@ class JoinViewModel @JvmOverloads constructor(
) ?: return@launch
// The query suspends, so the user may have picked files or started a join in the
// meantime. Theirs wins. No suspension point between this check and the assignment
// below, and both run on the main dispatcher, so nothing can interleave.
// meantime. Theirs wins.
//
// This catches a pick that has already *landed*, and only that. It used to claim there
// was "no suspension point between this check and the assignment below", which was the
// opposite of what happens: there is no assignment below, only observe(), which
// launches a separate coroutine that cannot write until its `collect` resumes. The
// check happens at one moment and the write lands at another, with a whole pick able
// to fit in between -- issue #49. The token observe() carries is what holds that line;
// see [ScreenOwnership].
if (_state.value !is JoinState.Idle || activeWorkId != null) return@launch
// Joins used to stage under one constant name, so two finished joins always reported
@@ -121,19 +181,29 @@ class JoinViewModel @JvmOverloads constructor(
InputFile(Uri.EMPTY, "", sizeBytes = null)
}
activeWorkId = reattachment.job.id
observe(reattachment.job.id, inputs, cancelled = JoinState.Idle)
observe(reattachment.job.id, inputs, cancelled = JoinState.Idle, token = token)
}
}
/**
* The tap is the claim, which is why it is taken here rather than inside the `launch` -- and
* above the early return, so the refusal below is covered by it too. A claim made in the
* coroutine is only immediate while `Dispatchers.Main.immediate` happens to run it inline, and
* a deferred claim leaves exactly the gap this closes.
*/
fun onInputsPicked(uris: List<Uri>) {
val token = ownership.claim()
if (uris.size < 2) {
_state.value = JoinState.Failed("Pick at least two files to join.")
return
}
viewModelScope.launch {
val files = withContext(Dispatchers.IO) {
val files = withContext(pickDispatcher) {
uris.map { InputQuery.describe(getApplication(), it) }
}
// Guarded like every other write that lands after a hop: two picks in quick succession
// suspend here together, and the slower one would otherwise land last.
if (!ownership.stillHeldBy(token)) return@launch
_state.value = JoinState.Ready(files)
}
}
@@ -147,10 +217,13 @@ class JoinViewModel @JvmOverloads constructor(
// did answer would hand the space check a lower bound it would read as a total.
totalBytes = InputQuery.total(inputs.map { it.sizeBytes }),
)
// Tapping Join claims the screen for this job, superseding any reattachment that has not
// finished asking.
val token = ownership.claim()
activeWorkId = request.id
workManager.enqueue(request)
_state.value = JoinState.Joining(inputs)
observe(request.id, inputs)
observe(request.id, inputs, token = token)
}
/**
@@ -158,12 +231,25 @@ class JoinViewModel @JvmOverloads constructor(
* files, ready to join again. For one picked up by [reattach] there are no picked files —
* what that job holds are URIs granted to a process that no longer exists — so it lands on
* Idle rather than offering to re-join files nothing can open.
* @param token the claim this observation belongs to. Nothing here can write until `collect`
* has resumed with a `WorkInfo`, which is some time after the caller decided to observe, so
* the claim is checked again at the last possible moment rather than trusted from then. See
* [ScreenOwnership], and issue #49.
*/
private fun observe(id: UUID, inputs: List<InputFile>, cancelled: JoinState = JoinState.Ready(inputs)) {
private fun observe(
id: UUID,
inputs: List<InputFile>,
cancelled: JoinState = JoinState.Ready(inputs),
token: Long,
) {
observer?.cancel()
observer = viewModelScope.launch {
workManager.getWorkInfoByIdFlow(id).collect { info ->
if (info == null) return@collect
// Ahead of the `when`, not merely ahead of the assignment: the SUCCEEDED branch
// takes ownership of the staged file, and a superseded observation must not do
// that either.
if (!ownership.stillHeldBy(token)) return@collect
_state.value = when (info.state) {
WorkInfo.State.RUNNING, WorkInfo.State.BLOCKED -> JoinState.Joining(inputs)
WorkInfo.State.ENQUEUED ->
@@ -229,29 +315,38 @@ class JoinViewModel @JvmOverloads constructor(
* join offered by reattachment was last seen during a tag query that may be hours old, and
* `cacheDir` is reclaimed by the OS and swept by this app. Without it the file's absence
* reached the screen as a raw ENOENT path.
*
* Reached from [JoinState.Joined] and again from a [JoinState.Failed] an earlier save left
* carrying its file; [pendingSave] is what makes those the same call.
*/
fun save(destination: Uri) {
val joined = _state.value as? JoinState.Joined ?: return
if (!joined.staged.isFile) {
val pending = _state.value.pendingSave() ?: return
if (!pending.staged.isFile) {
// No retry handle -- the file it would offer again is the one that has gone.
_state.value = JoinState.Failed(STAGED_FILE_GONE_MESSAGE)
return
}
viewModelScope.launch {
runCatching {
withContext(Dispatchers.IO) {
publisher.publish(joined.staged, destination)
joined.staged.delete()
publisher.publish(pending.staged, destination)
pending.staged.delete()
}
}.onSuccess {
// publish() already deleted it; nothing left to clean up.
pendingStaged = null
_state.value = JoinState.Saved(joined.suggestedName)
_state.value = JoinState.Saved(pending.suggestedName)
}.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.")
//
// `pending` travels on the state so the screen can offer the file again rather
// than leaving "Start over" -- which deletes it -- as the only thing on offer.
// Passing `pending` rather than rebuilding it is what keeps a retry that fails
// again on a carrying Failed instead of a bare one.
_state.value = JoinState.Failed(e.message ?: "Could not save the file.", pending)
}
}
}
@@ -261,8 +356,16 @@ class JoinViewModel @JvmOverloads constructor(
*
* Best effort, not a guarantee: the delete is cancelled with [viewModelScope] if the
* Activity finishes first. `OutputPublisher.sweepStaging` is the backstop.
*
* It deletes from a [JoinState.Failed] carrying a [PendingSave] too, deliberately and for the
* reason `ConversionViewModel.reset` writes out: the screen offers "Try saving again" above
* this button, so deletion is what the user chose rather than all this state could do.
*/
fun reset() {
// Start over is a claim like any other. The cancel below is a request honoured at the next
// suspension point, so a collector already on its way to a write has nothing left to
// honour it at; the claim is what stops that write landing on top of Idle.
ownership.claim()
observer?.cancel()
observer = null
activeWorkId = null
app/src/main/java/org/libremediaconverter/model/ContainerCapabilities.kt
+29 -4
View File
@@ -129,9 +129,25 @@ object ContainerCapabilities {
}
}
if (spec.videoCodec == VideoCodec.NONE && spec.audioCodec == AudioCodec.NONE) {
// Two faces of one rule: the output would carry no tracks at all.
//
// The first is visible in the spec alone — NONE on both axes. The second only emerges once
// the spec meets the probe, because [CopyPlanner] drops a video track the *input* does not
// have no matter which codec was named for it, so "H.265 + no audio" on an MP3 plans to
// (Drop, Drop) exactly as "None + None" does. Asking the spec alone answered the first and
// missed the second, and the miss was not cosmetic: `EditedMediaItem.Builder` refuses that
// composition with IllegalStateException("Audio and video cannot both be removed"), on
// Transformer's own thread, where the user would have seen a dead app rather than a reason.
if (spec.audioCodec == AudioCodec.NONE && (spec.videoCodec == VideoCodec.NONE || !probe.hasVideo)) {
return Validation.Invalid(
"This would produce an empty file — keep at least one track.",
if (spec.videoCodec == VideoCodec.NONE) {
"This would produce an empty file — keep at least one track."
} else {
// Names both halves. "No video track" alone reads as though the video setting
// were the only thing wrong, and the user would fix that and still be stuck.
"This file has no video track, so turning the audio off too would produce an " +
"empty file."
},
suggestions(
// Ask for both tracks back, then let repair settle what this container and
// this input can actually give.
@@ -162,7 +178,12 @@ object ContainerCapabilities {
if (!probe.hasVideo) {
return Validation.Invalid(
"This file has no video track to copy.",
listOf(spec.copy(videoCodec = VideoCodec.NONE)),
// Dropping the video is the right shape of answer, but it is only half of one:
// `spec.copy(videoCodec = NONE)` is valid exactly when the audio axis already
// happened to be fine, and refused otherwise — a Vorbis or PCM source into MP4,
// an MP3 into WebM. Handing it to the shared path repairs both axes and drops
// anything that still fails, so the chip cannot lead to a second error.
suggestions(spec.copy(videoCodec = VideoCodec.NONE), probe, exclude = spec),
)
}
val source = CodecNames.videoFromName(probe.videoCodec)
@@ -284,8 +305,12 @@ object ContainerCapabilities {
private fun repairVideo(spec: OutputSpec, probe: InputProbe): VideoCodec {
val container = spec.container
if (spec.videoCodec == VideoCodec.NONE || !container.canHoldVideo) return VideoCodec.NONE
// There is no video track to make one out of, so naming a codec would be a suggestion
// [CopyPlanner] drops on the floor. It also read as a non-sequitur: before this line, the
// repair offered for an MP3 was "H.264", the first codec MP4 happens to encode.
if (!probe.hasVideo) return VideoCodec.NONE
val source = CodecNames.videoFromName(probe.videoCodec).takeIf { probe.hasVideo }
val source = CodecNames.videoFromName(probe.videoCodec)
val copyable = source != null && accepts(container, source, CodecMode.COPY)
return when {
app/src/main/java/org/libremediaconverter/ui/TestTags.kt
+11
View File
@@ -45,6 +45,17 @@ object TestTags {
const val SAVE_FILE: String = "action.saveFile"
/**
* The retry a `Failed` offers after a save that threw, on both screens.
*
* Its own tag rather than [SAVE_FILE], because the two are different claims about the screen.
* [SAVE_FILE] is the first attempt from a finished job; this one may appear only where a staged
* file survived a failed save. Sharing a tag would collapse "a transcode failure offers nothing
* to save" and "a failed save offers the file again" into one query, and that first assertion
* is the one stopping a Save button from appearing where there is nothing to save.
*/
const val RETRY_SAVE: String = "action.retrySave"
/** `ConverterScreen`. */
object Converter {
const val CHOOSE_FILE: String = "converter.chooseFile"
app/src/main/java/org/libremediaconverter/ui/theme/Theme.kt
+12 -2
View File
@@ -25,8 +25,18 @@ private val LightColorScheme = lightColorScheme(
* Material 3 theme.
*
* Dynamic color (Material You) needs API 31+; minSdk is 33, so it is available
* unconditionally and no version guard is required. It stays switchable so users can
* opt back to the brand palette.
* unconditionally and no version guard is required.
*
* [dynamicColor] has no caller. `MainActivity` is the single call site and takes the
* default, so the parameter is always `true`, the two dynamic branches always win, and
* [DarkColorScheme] and [LightColorScheme] are dead: nothing in the app can opt back to the
* brand palette. `ThemeColorSchemeTest` reaches those two branches only by passing
* [dynamicColor] explicitly -- a test doing it, not a feature.
*
* That is known rather than an oversight. #68 holds the choice between adding a switch,
* deleting the dead branches together with the template palette, and replacing that palette
* first; it is undecided, so nothing here should be read as a promise that any of them
* happens.
*/
@Composable
fun LibreMediaConverterTheme(
app/src/test/java/org/libremediaconverter/convert/ConversionViewModelCleanupTest.kt
+6 -2
View File
@@ -93,8 +93,12 @@ class ConversionViewModelCleanupTest {
assertTrue("a failed save must not destroy the only copy", staged.exists())
assertEquals(emptyList<File>(), 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.
// The handle is a ViewModel field rather than something read back out of the state
// machine, and stays one now that a save-failed `Failed` also carries a `PendingSave`:
// that is a view for the screen to offer a retry through, never a second owner of the
// file. This delete goes through the field, which is what keeps a state that is dropped
// rather than read from taking the only reference with it. What the state carries, and
// what the screen then does with it, are `FailedSaveRetryTest`'s.
viewModel.reset()
assertEquals(listOf(staged), publisher.discarded)
app/src/test/java/org/libremediaconverter/convert/ConverterStateAffordancesTest.kt
+80 -4
View File
@@ -53,10 +53,11 @@ import java.io.File
* it -- so it is unobservable from a JVM test, the same limit `FileCardTest` records for
* `HorizontalDivider`. The message text itself is asserted; the colour would need a screenshot.
* - **The three `assertDoesNotExist` checks on [TestTags.Converter.FILE_CARD] are compile-guarded,
* not guarded by this file.** `Idle` is a `data object`, and `Saved` and `Failed` carry only a
* `displayName` and a `message`; none of the three has an `input`, so `FileCard(s.input)` does not
* compile in those arms. The lines stay because they state the intent cheaply, but they are not
* what stops a `FileCard` appearing there and this file does not claim they are.
* not guarded by this file.** `Idle` is a `data object`, `Saved` carries a `displayName`, and
* `Failed` carries a message and -- after a failed save only -- the staged file it left behind;
* none of the three has an `input`, so `FileCard(s.input)` does not compile in those arms. The
* lines stay because they state the intent cheaply, but they are not what stops a `FileCard`
* appearing there and this file does not claim they are.
* - **Which constant each chip hands back** belongs to `ConverterPickerSelectionTest`, and **what
* the file card says about an unknown size** to `FileCardTest`. This file asserts that `Ready`
* puts those leaves on screen at all, not what they then do.
@@ -326,6 +327,22 @@ class ConverterStateAffordancesTest {
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD).assertDoesNotExist()
}
/**
* **The assertion that bounds #30's whole change**, and the one worth breaking things to keep.
*
* A transcode that died staged nothing, so its `Failed` carries no [PendingSave] and there is
* nothing for a save dialog to be handed. Making the retry unconditional -- or making the
* `WorkInfo.State.FAILED` arm of `ConversionViewModel.observe` carry a handle it has no file
* for -- puts a button on screen that can only fail, and this is what notices.
*/
@Test
fun `a transcode failure offers no way to save`() {
setContent(ConversionState.Failed(message = "Ran out of space while writing the output."))
composeRule.onNodeWithTag(TestTags.RETRY_SAVE).assertDoesNotExist()
composeRule.onNodeWithTag(TestTags.SAVE_FILE).assertDoesNotExist()
}
@Test
fun `tapping start over after a failure resets and does nothing else`() {
setContent(ConversionState.Failed(message = "Ran out of space while writing the output."))
@@ -335,6 +352,50 @@ class ConverterStateAffordancesTest {
assertEquals(listOf("reset"), fired)
}
// ----------------------------------------------------- Failed, carrying a file
/**
* The defect in #30, stated as what the branch must render.
*
* `save()` keeps the staged file on a failure deliberately -- it can be the only copy of an
* hour of transcoding -- and before this the only control here was "Start over", wired to
* `reset()`, which deletes exactly that file. Both buttons, not one: the restart has to stay
* reachable, because leaving a full-size file in cache is the outcome it exists to avoid.
*/
@Test
fun `a failed save offers the file again as well as a restart`() {
setContent(failedSave())
composeRule.onNodeWithTag(TestTags.RETRY_SAVE).assertExists()
composeRule.onNodeWithTag(TestTags.START_OVER).assertExists()
}
/**
* The name, not just that something fired: it comes from the finished job, and a retry wired to
* a literal or to the picker's current guess would hand the dialog a name the job never chose.
*/
@Test
fun `tapping try saving again hands back the name the job chose`() {
setContent(failedSave())
composeRule.onNodeWithTag(TestTags.RETRY_SAVE).performScrollTo().performClick()
assertEquals(listOf("save:holiday.mp4"), fired)
}
/**
* Start over from here still resets, and resetting still deletes -- see `reset()`'s KDoc for
* why that is acceptable now and was not before. What it must not do is save on the way past.
*/
@Test
fun `tapping start over after a failed save resets and does not save`() {
setContent(failedSave())
composeRule.onNodeWithTag(TestTags.START_OVER).performScrollTo().performClick()
assertEquals(listOf("reset"), fired)
}
// ------------------------------------------------------------------ Harness
private fun input() = InputFile(
@@ -348,6 +409,21 @@ class ConverterStateAffordancesTest {
* missing file rather than throwing, so the size line reads `0 B` and no temporary folder is
* needed to render the arm.
*/
/**
* A `Failed` an earlier save left carrying its file, which is the only way [retry] is non-null.
*
* The same missing `staged` path as [converted], and for the same reason: this arm renders no
* size line at all, so nothing here ever touches the filesystem.
*/
private fun failedSave() = ConversionState.Failed(
message = "There was not enough room on the destination.",
retry = PendingSave(
staged = File("no-such-staged-output.mp4"),
suggestedName = "holiday.mp4",
mimeType = "video/mp4",
),
)
private fun converted(routeReason: String = "") = ConversionState.Converted(
input = input(),
staged = File("no-such-staged-output.mp4"),
app/src/test/java/org/libremediaconverter/convert/FailedSaveRetryTest.kt
+370
View File
@@ -0,0 +1,370 @@
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.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.join.JoinState
import org.libremediaconverter.join.JoinViewModel
import org.libremediaconverter.join.pendingSave
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.OutputFormat
import org.libremediaconverter.work.ConcatWorker
import org.libremediaconverter.work.ConversionWorker
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.io.File
/**
* A failed save has to leave the file *offerable*, not merely undeleted.
*
* The defect is #30, and both halves of it were already written down in `main`. `save()`'s
* `onFailure` kept the staged file on purpose -- "deleting here would destroy the work to tidy up
* a cache directory" -- and then handed the screen a `Failed` carrying a message and nothing else,
* so the single control that branch rendered was "Start over", wired to `reset()`, which deletes
* exactly that file. The intent and the affordance disagreed, and the affordance won.
*
* `ConversionViewModelCleanupTest` already pins the *keeping*: after a failed save the file is
* still on disk and nothing has been discarded. It stays green with the state carrying nothing,
* because it reads the filesystem rather than the state. This file asserts the other half -- that
* the handle reaches the state a screen can read -- and the negative that bounds it: a failure
* with nothing staged behind it must not sprout a save button.
*
* Both ViewModels in one class, following `MissingStagedFileTest`. They are separate state
* machines that can each hold a staged file at once, but this defect and its fix are the same
* shape in both, and splitting them would put the two halves of one invariant in two files.
*
* ### Not asserted here, so each is a decision rather than an omission
*
* - **That the destination received the bytes.** [RecordingPublisher.publish] is a stub, which is
* the only way to make a save fail deterministically -- and making it fail is what every case
* here needs. `OutputPublisherPublishTest` owns what a real publish writes.
* - **The screen's two buttons.** `ConverterStateAffordancesTest` and `JoinStateAffordancesTest`
* own what each state renders; this file owns what each state carries.
* - **`ConverterScreen`'s `destinationMime` line itself.** It lives in the entry point, above the
* `ScreenContent` seam, and reaching it needs a real ViewModel inside a composition. What it
* reads -- `pendingSave()?.mimeType` -- is asserted directly instead, which is why that
* derivation was moved out of the entry point in the first place.
* - **Picking a new input while a `Failed` carries a file.** `onInputPicked` overwrites the state
* without discarding, from `Converted` exactly as much as from a carrying `Failed`, and neither
* branch renders a picker. It is a pre-existing path this change neither opens nor widens: the
* carried handle is a view of `pendingStaged`, never a second owner of the file.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class FailedSaveRetryTest {
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 with no native library present.
ConversionDependencies.probe = { _, _ -> InputProbe() }
staged = publisher.createStagingFile("holiday.mp4").apply { writeBytes(ByteArray(4096)) }
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
// ------------------------------------------------------------------ Convert
/**
* The bite named in #30's fix. Emitting a plain `Failed` from `save()`'s `onFailure` -- which
* is what `main` did -- reddens this case on the null handle, and nothing else in the suite.
*/
@Test
fun `a failed save leaves the staged file offerable, not merely undeleted`() {
val viewModel = failedSaveViewModel()
val failed = viewModel.state.value as ConversionState.Failed
val retry = failed.retry
assertNotNull("a failed save must leave the staged file offerable, not just on disk", retry)
assertEquals("the retry must name the file the conversion actually produced", staged, retry?.staged)
// Both from the job's own output Data rather than from the pickers, so a retry opens the
// same dialog the first attempt did.
assertEquals(SUGGESTED_NAME, retry?.suggestedName)
assertEquals(JOB_MIME_TYPE, retry?.mimeType)
assertTrue("a failed save must not destroy the only copy", staged.exists())
}
/**
* The whole point of carrying the handle: the second attempt is a real save, not a new job.
*
* `publishFailure` is cleared between the two calls, so one `RecordingPublisher` plays both a
* full destination and an empty one -- which is exactly the user's situation.
*/
@Test
fun `retrying a failed save publishes the file and leaves nothing staged`() {
val viewModel = failedSaveViewModel()
publisher.publishFailure = null
viewModel.save(DESTINATION)
val saved = awaitState(viewModel.state, "Saved") { it is ConversionState.Saved }
assertEquals(SUGGESTED_NAME, (saved as ConversionState.Saved).displayName)
assertFalse("a successful retry should have removed the staged file", staged.exists())
// Nothing to collect afterwards: the retry published it, so reset() has no work left.
viewModel.reset()
assertEquals(emptyList<File>(), publisher.discarded)
}
/**
* The second failure must not eat the file the first one kept.
*
* A `Failed` built fresh from `e.message` alone would drop the handle here while every other
* assertion in this file stayed green -- the file is still on disk, and the first failure
* already proved the state can carry it.
*/
@Test
fun `a retry that fails again still carries the file rather than dropping it`() {
val viewModel = failedSaveViewModel()
publisher.publishFailure = IllegalStateException("destination volume still full")
viewModel.save(DESTINATION)
// Waited for by the *second* message rather than by `is Failed`: the state was already
// Failed when the retry started, so the type alone would be satisfied before it ran.
val failed = awaitState(viewModel.state, "the second failure") {
it is ConversionState.Failed && it.message == "destination volume still full"
} as ConversionState.Failed
assertEquals("the second failure must offer the same file the first one did", staged, failed.retry?.staged)
assertTrue(staged.exists())
assertEquals(emptyList<File>(), publisher.discarded)
}
/**
* "Start over" still deletes, and that is the decision `reset()`'s KDoc records: acceptable
* only because "Try saving again" is on screen beside it. Exactly once, through the publisher.
*/
@Test
fun `start over from a failed save discards the carried file exactly once`() {
val viewModel = failedSaveViewModel()
viewModel.reset()
assertEquals(ConversionState.Idle, viewModel.state.value)
assertEquals(listOf(staged), publisher.discarded)
assertFalse(staged.exists())
}
/**
* A retry meets the same existence check the first attempt did, so a file collected by the
* sweep or by the OS in between is reported as a sentence rather than as a raw ENOENT path.
* And the state that reports it carries nothing: there is no file left to offer.
*/
@Test
fun `a retry whose staged file has gone says so and offers nothing further`() {
val viewModel = failedSaveViewModel()
assertTrue("the fixture must start with a real staged file", staged.delete())
publisher.publishFailure = null
viewModel.save(DESTINATION)
val failed = viewModel.state.value as ConversionState.Failed
assertEquals(STAGED_FILE_GONE_MESSAGE, failed.message)
assertNull("a file that has gone cannot be offered again", failed.retry)
}
/**
* The negative that bounds the whole change, and the reason `retry` is nullable.
*
* A transcode that died staged nothing, so there is no file to hand back -- and a `Failed`
* that carried one anyway would put a save button on a screen with nothing to save. Driven
* through a worker that really fails rather than by constructing the state, because the line
* under test is the `WorkInfo.State.FAILED` arm of `observe`.
*/
@Test
fun `a transcode failure carries nothing to save`() {
installFailingTestWorkManager(app, workDataOf(ConversionWorker.KEY_ERROR to "The encoder gave up."))
val viewModel = ConversionViewModel(app, Dispatchers.Unconfined)
viewModel.onInputPicked(Uri.parse("content://test/holiday.mp4"))
awaitState(viewModel.state, "Ready") { it is ConversionState.Ready }
viewModel.convert()
val failed = awaitState(viewModel.state, "Failed") { it is ConversionState.Failed } as ConversionState.Failed
assertEquals("The encoder gave up.", failed.message)
assertNull("a transcode failure has nothing staged, so it must offer no save", failed.retry)
assertNull("and nothing for the save dialog to open with either", failed.pendingSave())
}
/**
* What the save dialog reopens with, which is the entry point's only reader of this state.
*
* The pickers are moved *after* the job finishes, which is what makes this bite: a retry that
* asked the current settings would offer `audio/mpeg` for a file the job wrote as MP4. The
* same gap is permanent for a reattached job, whose spec was never in these settings at all.
*/
@Test
fun `a retry offers the type the job chose, not the one the pickers now show`() {
val viewModel = failedSaveViewModel()
viewModel.setPreset(OutputFormat.MP3)
assertEquals(
"the fixture needs the pickers to disagree with the job",
"audio/mpeg",
viewModel.settings.value.spec.mimeType,
)
assertEquals(JOB_MIME_TYPE, viewModel.state.value.pendingSave()?.mimeType)
}
// --------------------------------------------------------------------- Join
@Test
fun `a failed join save leaves the staged file offerable, not merely undeleted`() {
val viewModel = failedJoinSaveViewModel()
val failed = viewModel.state.value as JoinState.Failed
val retry = failed.retry
assertNotNull("a failed save must leave the staged file offerable, not just on disk", retry)
assertEquals(staged, retry?.staged)
assertEquals(SUGGESTED_NAME, retry?.suggestedName)
assertEquals(JOB_MIME_TYPE, retry?.mimeType)
assertTrue(staged.exists())
}
@Test
fun `retrying a failed join save publishes the file and leaves nothing staged`() {
val viewModel = failedJoinSaveViewModel()
publisher.publishFailure = null
viewModel.save(DESTINATION)
val saved = awaitState(viewModel.state, "Saved") { it is JoinState.Saved }
assertEquals(SUGGESTED_NAME, (saved as JoinState.Saved).displayName)
assertFalse(staged.exists())
viewModel.reset()
assertEquals(emptyList<File>(), publisher.discarded)
}
@Test
fun `a join retry that fails again still carries the file rather than dropping it`() {
val viewModel = failedJoinSaveViewModel()
publisher.publishFailure = IllegalStateException("destination volume still full")
viewModel.save(DESTINATION)
// By the second message, not by `is Failed` -- see the converter case above.
val failed = awaitState(viewModel.state, "the second failure") {
it is JoinState.Failed && it.message == "destination volume still full"
} as JoinState.Failed
assertEquals(staged, failed.retry?.staged)
assertTrue(staged.exists())
assertEquals(emptyList<File>(), publisher.discarded)
}
@Test
fun `start over from a failed join save discards the carried file exactly once`() {
val viewModel = failedJoinSaveViewModel()
viewModel.reset()
assertEquals(JoinState.Idle, viewModel.state.value)
assertEquals(listOf(staged), publisher.discarded)
assertFalse(staged.exists())
}
@Test
fun `a join failure carries nothing to save`() {
installFailingTestWorkManager(app, workDataOf(ConcatWorker.KEY_ERROR to "The files could not be joined."))
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()
val failed = awaitState(viewModel.state, "Failed") { it is JoinState.Failed } as JoinState.Failed
assertEquals("The files could not be joined.", failed.message)
assertNull("a join failure has nothing staged, so it must offer no save", failed.retry)
assertNull(failed.pendingSave())
}
// ------------------------------------------------------------------ Harness
/**
* A ViewModel driven to `Converted` and then through a save that threw.
*
* The WorkManager is installed here rather than in `@Before`, because two cases in this class
* need one whose workers fail instead.
*/
private fun failedSaveViewModel(): ConversionViewModel {
installTestWorkManager(app, conversionOutput())
// 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.mkv"))
awaitState(viewModel.state, "Ready") { it is ConversionState.Ready }
viewModel.convert()
awaitState(viewModel.state, "Converted") { it is ConversionState.Converted }
publisher.publishFailure = IllegalStateException("destination volume full")
viewModel.save(DESTINATION)
awaitState(viewModel.state, "Failed") { it is ConversionState.Failed }
return viewModel
}
/** The join tab's equivalent, driven to `Joined` and then through a save that threw. */
private fun failedJoinSaveViewModel(): JoinViewModel {
installTestWorkManager(app, joinOutput())
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 }
publisher.publishFailure = IllegalStateException("destination volume full")
viewModel.save(DESTINATION)
awaitState(viewModel.state, "Failed") { it is JoinState.Failed }
return viewModel
}
/**
* The output `Data` a finished conversion reports.
*
* The name and type are set rather than left out, so the assertions above are about what the
* *job* chose. Both ViewModels fall back to a derivation when they are missing, and a fixture
* that omitted them would be asserting the fallback while looking like it asserted the job.
*
* Spelled out per worker rather than shared with [joinOutput], even though the two constants
* hold the same strings today. A test that leaned on that would be asserting a coincidence.
*/
private fun conversionOutput() = workDataOf(
ConversionWorker.KEY_OUTPUT_PATH to staged.absolutePath,
ConversionWorker.KEY_SUGGESTED_NAME to SUGGESTED_NAME,
ConversionWorker.KEY_MIME_TYPE to JOB_MIME_TYPE,
)
/** The output `Data` a finished join reports. See [conversionOutput]. */
private fun joinOutput() = workDataOf(
ConcatWorker.KEY_OUTPUT_PATH to staged.absolutePath,
ConcatWorker.KEY_SUGGESTED_NAME to SUGGESTED_NAME,
ConcatWorker.KEY_MIME_TYPE to JOB_MIME_TYPE,
)
private companion object {
val DESTINATION: Uri = Uri.parse("content://test/destination.mp4")
const val SUGGESTED_NAME = "holiday.mp4"
/** What the job wrote. [OutputFormat.MP3]'s `audio/mpeg` is what the pickers move to. */
const val JOB_MIME_TYPE = "video/mp4"
}
}
app/src/test/java/org/libremediaconverter/convert/Media3EngineEmptyCompositionTest.kt
+105
View File
@@ -0,0 +1,105 @@
package org.libremediaconverter.convert
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withTimeout
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.AudioPlan
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.ConversionRequest
import org.libremediaconverter.model.CopyPlanner
import org.libremediaconverter.model.InputKind
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.OutputSpec
import org.libremediaconverter.model.VideoCodec
import org.libremediaconverter.model.VideoPlan
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.io.File
import java.util.concurrent.CancellationException
/**
* What happens when Media3 refuses the export before it starts.
*
* `EditedMediaItem.Builder` rejects a composition with both tracks removed —
* checkState("Audio and video cannot both be removed") — and [Media3Engine] builds it on its own
* HandlerThread. That build used to sit *between* two narrow `runCatching` blocks, one around
* `buildTransformer` and one around `start`, so the exception escaped `handler.post`'s body: it
* reached the thread's uncaught handler, which on Android takes the process down, and the
* continuation was left unresumed either way.
*
* Robolectric runs the real [android.os.HandlerThread] and the real Media3 builders, so the whole
* sequence happens here — the engine really posts, really builds, and really throws. What it cannot
* reproduce is the *consequence* of an escaped throw: a JVM background thread dying is not process
* death. So the assertion is on the half that is observable everywhere and is the half that
* matters to the user — the suspension is resolved, with the reason, rather than left hanging.
* `Media3EngineTest.aPlanThatRemovesBothTracksFailsInsteadOfKillingTheProcess` is the same case on
* a device.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class Media3EngineEmptyCompositionTest {
@Test
fun `a plan that removes both tracks fails the job instead of escaping the handler thread`() {
val context = RuntimeEnvironment.getApplication()
val engine = Media3Engine(context)
val request = ConversionRequest(
spec = OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE),
probe = InputProbe(
videoCodec = null,
audioCodec = "mp3",
hasVideo = false,
container = Container.MP3,
kind = InputKind.AUDIO_ONLY,
),
)
// Asserted rather than assumed: ConversionRequest's default probe says hasVideo = true, and
// with it this same spec plans to (Encode, Drop), nothing throws, and the test would pass
// over a code path it never entered.
val plan = CopyPlanner.plan(request.spec, request.probe)
assertEquals(VideoPlan.Drop, plan.video)
assertEquals(AudioPlan.Drop, plan.audio)
val failure = try {
runCatching {
runBlocking {
withTimeout(TIMEOUT_MS) {
engine.transcode(Uri.parse("file:///dev/null"), File(context.cacheDir, "empty.mp4"), request) {}
}
}
}.exceptionOrNull()
} finally {
engine.close()
}
// Both halves are load-bearing, and the second is not pedantry: withTimeout raises
// TimeoutCancellationException, and `java.util.concurrent.CancellationException` *extends*
// IllegalStateException — so testing only the first would call an unresumed continuation a
// pass. This assertion was written that way, and the mutation is what found it.
assertFalse(
"the continuation was never resumed — the failure escaped instead of being reported: $failure",
failure is CancellationException,
)
assertTrue(
"the builder's refusal must surface as a failed job; got $failure",
failure is IllegalStateException,
)
}
private companion object {
/**
* Short on purpose. Nothing is decoded, encoded or muxed on this path — the builder refuses
* the input outright — so anything approaching this is a hang, which is the failure mode
* this test is looking for.
*/
const val TIMEOUT_MS = 10_000L
}
}
app/src/test/java/org/libremediaconverter/convert/ParkedPickDispatcher.kt
+63
View File
@@ -0,0 +1,63 @@
package org.libremediaconverter.convert
import kotlinx.coroutines.CoroutineDispatcher
import java.util.concurrent.ConcurrentLinkedQueue
import kotlin.coroutines.CoroutineContext
/**
* A dispatcher that holds a pick in flight until the test lets it finish.
*
* Issue #49 is about what a ViewModel does *while* a pick is between the tap and the write it
* eventually makes. Both ViewModels put a blocking hop there — the metadata query, and on the
* convert side the probe as well — and both hops go through an injectable dispatcher. Handing
* them this one turns "the pick has been made but has not landed yet" from a window a test has
* to race into a state it can simply sit in.
*
* Nothing here is a fake pick. The real `InputQuery.describe` still runs, on this thread,
* whenever [runAll] is called; the only thing under the test's control is *when*.
*
* Confined to the thread that drives the test. Both ViewModels reach `withContext(pickDispatcher)`
* from a coroutine on the main dispatcher, so [dispatch] is only ever called from there — the
* queue is concurrent anyway, because a dispatcher that quietly dropped a block from another
* thread would fail as a hang rather than as an assertion.
*/
class ParkedPickDispatcher : CoroutineDispatcher() {
private val parked = ConcurrentLinkedQueue<Runnable>()
/**
* How many blocks are waiting.
*
* Asserted on before the interesting part of a test, because "the pick was in flight" is a
* premise rather than a detail: a zero here means the pick had already landed and whatever
* the test went on to prove was proved about a different situation.
*/
val parkedCount: Int get() = parked.size
override fun dispatch(context: CoroutineContext, block: Runnable) {
parked += block
}
/**
* Removes everything parked, oldest first, and hands it to the caller to run.
*
* What [runAll] cannot express: two picks are two hops through this dispatcher, and the defect
* they can produce is the *first* one finishing last. Running them in the order they arrived
* is the one order in which nothing goes wrong, so a test has to be able to choose.
*/
fun takeParked(): List<Runnable> = generateSequence { parked.poll() }.toList()
/**
* Runs everything parked, and everything that parks as a result.
*
* The loop is not defensive: `ConversionViewModel.onInputPicked` makes two hops through this
* dispatcher — the metadata query, then the probe — and the second is only enqueued once the
* first has run. Draining once would leave the probe parked for the rest of the process.
*/
fun runAll() {
while (true) {
val next = parked.poll() ?: return
next.run()
}
}
}
app/src/test/java/org/libremediaconverter/convert/PickOwnershipTest.kt
+125
View File
@@ -0,0 +1,125 @@
package org.libremediaconverter.convert
import android.app.Application
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import androidx.work.workDataOf
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
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
/**
* The half of issue #49 that is not about reattachment at all.
*
* `onInputPicked` makes two writes and both of them land after a hop off the main thread, so both
* belong to whichever pick was in flight rather than to whichever pick the user last made. Nothing
* was enforcing that. Two taps in quick succession — an easy thing to do while a `content://`
* metadata query is slow — put the loser's file on screen if its query happened to come back
* second, which is the same defect the ticket reported against reattachment with a different
* coroutine on the losing side.
*
* Both cases below were measured rather than assumed: deleting either guard turns the matching
* test red, and deleting the probe one turns nine other tests red with it. Neither was ever
* reported, because a pick that loses to another pick still shows *a* file the user chose -- which
* is what made it worth closing alongside #49 rather than leaving as a second thing to find.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class PickOwnershipTest {
private lateinit var app: Application
private lateinit var parkedPick: ParkedPickDispatcher
private lateinit var viewModel: ConversionViewModel
@Before
fun setUp() {
app = RuntimeEnvironment.getApplication()
ConversionDependencies.publisher = { RecordingPublisher(app) }
ConversionDependencies.probe = { _, _ -> PROBE }
installTestWorkManager(app, workDataOf(ConversionWorker.KEY_OUTPUT_PATH to "/dev/null"))
parkedPick = ParkedPickDispatcher()
viewModel = ConversionViewModel(app, pickDispatcher = parkedPick)
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
/**
* Two taps, and the first one's metadata query is the slow one.
*
* The order is chosen rather than raced: both queries are parked, and this runs the second
* before the first. Without an ownership check the straggler writes last and the screen ends
* up showing a file the user moved off two taps ago.
*/
@Test
fun `the slower of two picks does not land on top of the faster one`() {
viewModel.onInputPicked(FIRST)
viewModel.onInputPicked(SECOND)
val queries = parkedPick.takeParked()
assertEquals("both picks should be in flight", 2, queries.size)
// The second pick's query comes back first; the first pick's is the straggler.
queries[1].run()
queries[0].run()
val current = viewModel.state.value
assertEquals(
"a pick the user has already replaced took the screen: $current",
SECOND,
(current as ConversionState.Ready).input.uri,
)
}
/**
* The second of `onInputPicked`'s two writes, which lands a whole probe later.
*
* The probe hop is a native process spawn, so it is the longest gap in a pick and the easiest
* one to pick again during. This used to be guarded by comparing URIs against the state, which
* answers a narrower question than the one that matters — it cannot tell a second pick of the
* same file from the first, and it reads a state that a later claim may not have written yet,
* which is exactly this case: the newer pick has claimed the screen but its own query has not
* come back, so the state still names the older file and the comparison waves it through.
*/
@Test
fun `a probe from a pick the user has moved off does not fill the card in`() {
viewModel.onInputPicked(FIRST)
parkedPick.takeParked().single().run()
assertEquals(FIRST, (viewModel.state.value as ConversionState.Ready).input.uri)
// The user picks again while the first pick is still probing.
viewModel.onInputPicked(SECOND)
val pending = parkedPick.takeParked()
assertEquals("the first probe and the second query should both be waiting", 2, pending.size)
pending[0].run()
assertNull(
"a probe belonging to a pick the user replaced must not reach the card",
(viewModel.state.value as ConversionState.Ready).input.probe,
)
// And the pick that did win still fills its own card in, probe included.
pending[1].run()
parkedPick.runAll()
val settled = viewModel.state.value as ConversionState.Ready
assertEquals(SECOND, settled.input.uri)
assertNotNull("the winning pick's own probe still has to land", settled.input.probe)
}
private companion object {
val FIRST: Uri = Uri.fromFile(File("/tmp/first.mp4"))
val SECOND: Uri = Uri.fromFile(File("/tmp/second.mp4"))
val PROBE = InputProbe(videoCodec = "h264")
}
}
app/src/test/java/org/libremediaconverter/convert/ReattachmentOwnershipTest.kt
+156
View File
@@ -0,0 +1,156 @@
package org.libremediaconverter.convert
import android.app.Application
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import androidx.work.WorkManager
import androidx.work.workDataOf
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.model.InputProbe
import org.libremediaconverter.work.ConversionWorker
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.io.File
/**
* Issue #49, on the JVM and without the race.
*
* `ReattachOnLaunchTest.doesNotOverwriteAPickTheUserHasAlreadyMade` has been catching this on
* devices since 2026-08-24 — four occurrences, spread across API 33, 35 and 36, which is what
* ruled out an emulator-image quirk. Every one was on attempt 1 and every one passed on re-run,
* which is why it was read as flaky infrastructure for two days. It is not. The assertion it fails
* on is `expected null, but was:<Converted>`: a finished job from an earlier session taking a
* screen the user had already picked a file on.
*
* The defect is a check-then-act whose act is deferred into another coroutine. `reattach()` reads
* `_state.value` and then calls `observe()`, which *launches* a collector that has to suspend on
* `getWorkInfoByIdFlow(...).collect` before it can write anything. So the check happens at one
* moment and the write lands at another:
*
* 1. `init` starts the tag query and suspends in it.
* 2. The user picks a file; `onInputPicked` suspends in its metadata query.
* 3. The query comes back. `_state.value` is still `Idle` — step 2 has not written yet — so the
* guard passes and an observation of the old job is launched.
* 4. The pick lands. `Ready(picked)`. The user owns the screen.
* 5. The observation's first `WorkInfo` arrives and writes `Converted(yesterday)` over it.
*
* The comment above that guard claimed "no suspension point between this check and the assignment
* below, so nothing can interleave". There is no assignment below, and the check and the write
* are in different coroutines.
*
* [ReattachGuardsTest] covers the case where the pick has already *landed*, which the plain guard
* does catch. This covers the one where it is still in flight, which it does not.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class ReattachmentOwnershipTest {
private lateinit var app: Application
private lateinit var publisher: RecordingPublisher
private lateinit var workManager: WorkManager
private lateinit var staged: File
@Before
fun setUp() {
app = RuntimeEnvironment.getApplication()
publisher = RecordingPublisher(app)
ConversionDependencies.publisher = { publisher }
ConversionDependencies.probe = { _, _ -> InputProbe() }
staged = publisher.createStagingFile("holiday_converted.mp4").apply { writeBytes(ByteArray(4096)) }
installTestWorkManager(app, workDataOf(ConversionWorker.KEY_OUTPUT_PATH to staged.absolutePath))
workManager = WorkManager.getInstance(app)
// The situation reattachment exists for: a conversion that finished in a process that is
// gone, with its output still in the cache and nothing in the UI holding its id.
workManager.enqueue(
ConversionWorker.request(
inputUri = Uri.parse("content://test/holiday.mp4"),
displayName = "holiday.mp4",
sizeBytes = 4_096L,
),
).result.get()
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
/**
* The race, made into a state the test can sit in rather than one it has to catch.
*
* The pick is parked on a dispatcher this test owns, so it stays in flight — issued, not yet
* written — for as long as the assertions need it to be. Everything else is production: a
* real `WorkManager` holding a real finished job, the real `reattach`, the real `observe`.
*
* Determinism comes from where Robolectric leaves the main looper. `reattach`'s tag query hops
* to a real [kotlinx.coroutines.Dispatchers.IO] thread, so its continuation can only come back
* as a message posted to the main looper — and that looper is paused, so it cannot run until
* something pumps it. `onInputPicked` is an ordinary synchronous call from this thread. The
* pick is therefore *always* issued before the guard runs; none of it is left to timing, which
* is the whole point of writing it here rather than relying on the rare device sighting.
*/
@Test
fun `a conversion found while the user was picking never reaches the screen`() {
val picked = Uri.fromFile(File(app.cacheDir, "beach.mp4").apply { writeBytes(ByteArray(2048)) })
val parkedPick = ParkedPickDispatcher()
val viewModel = ConversionViewModel(app, pickDispatcher = parkedPick)
viewModel.onInputPicked(picked)
assertEquals(
"the pick must still be in flight, or this proves something about a different situation",
1,
parkedPick.parkedCount,
)
// The control, and the reason this test does not rest on a settle window being long
// enough. A second ViewModel with nothing to supersede it reattaches to the same job
// through the same code; when it has arrived, the whole query-guard-observe-write path has
// demonstrably run to completion. `viewModel` started its own reattachment first, so it
// has had at least as long. Waiting on this rather than on a sleep is what makes the
// assertion below "it did not happen" rather than "it had not happened yet".
reattachmentHasRunToCompletion()
val current = viewModel.state.value
assertTrue("reattachment took the screen from the user: $current", current is ConversionState.Idle)
// And the pick, when it lands, is what stays there.
parkedPick.runAll()
val ready = awaitState(viewModel.state, "Ready") { it is ConversionState.Ready }
assertEquals(picked, (ready as ConversionState.Ready).input.uri)
reattachmentHasRunToCompletion()
assertEquals("the user's pick must survive a late reattachment", ready, viewModel.state.value)
}
/**
* The other half of the contract: a reattachment nobody has superseded still takes the screen.
*
* Without this, dropping every reattachment on the floor would pass the test above. Same job,
* same WorkManager, same production path — only the pick is missing.
*/
@Test
fun `a conversion nobody has superseded still reaches the screen`() {
val converted = awaitState(ConversionViewModel(app).state, "Converted") {
it is ConversionState.Converted
}
assertEquals(staged.absolutePath, (converted as ConversionState.Converted).staged.absolutePath)
assertEquals("holiday.mp4", converted.input.displayName)
}
/**
* Drives a throwaway ViewModel through a whole reattachment, and returns once it has landed.
*
* [awaitState] pumps the main looper, which is what runs every reattachment continuation
* waiting on it — this one's, and the one belonging to the ViewModel under test, which was
* posted earlier and therefore runs first.
*/
private fun reattachmentHasRunToCompletion() {
awaitState(ConversionViewModel(app).state, "Converted") { it is ConversionState.Converted }
}
}
app/src/test/java/org/libremediaconverter/convert/StagingCleanupSupport.kt
+32 -1
View File
@@ -82,6 +82,28 @@ class SucceedingWorkerFactory(private val outputData: Data) : WorkerFactory() {
}
}
/**
* Stands in for a worker that died, reporting [outputData] on the way out.
*
* The counterpart to [SucceedingWorkerFactory], and needed for the same reason: the real workers
* cannot run on the JVM, so the only way to ask what a ViewModel does with a `FAILED` `WorkInfo` is
* to produce one. A `Result.failure` carrying `KEY_ERROR` is exactly what both real workers report
* when their engine gives up, and it is the one path where nothing has ever been staged.
*
* `runAttemptCount` is irrelevant here: `Result.failure` is terminal, so WorkManager does not retry
* it and the state goes straight to `Failed` rather than through `Waiting`.
*/
class FailingWorkerFactory(private val outputData: Data) : WorkerFactory() {
override fun createWorker(
appContext: Context,
workerClassName: String,
workerParameters: WorkerParameters,
): ListenableWorker = object : Worker(appContext, workerParameters) {
override fun doWork(): Result = Result.failure(outputData)
}
}
/**
* Installs a synchronous test WorkManager whose workers succeed with [outputData].
*
@@ -89,6 +111,16 @@ class SucceedingWorkerFactory(private val outputData: Data) : WorkerFactory() {
*/
fun installTestWorkManager(context: Context, outputData: Data): SucceedingWorkerFactory {
val factory = SucceedingWorkerFactory(outputData)
installWorkManager(context, factory)
return factory
}
/** Installs a synchronous test WorkManager whose workers fail, reporting [outputData]. */
fun installFailingTestWorkManager(context: Context, outputData: Data) {
installWorkManager(context, FailingWorkerFactory(outputData))
}
private fun installWorkManager(context: Context, factory: WorkerFactory) {
WorkManagerTestInitHelper.initializeTestWorkManager(
context,
Configuration.Builder()
@@ -98,7 +130,6 @@ fun installTestWorkManager(context: Context, outputData: Data): SucceedingWorker
.setWorkerFactory(factory)
.build(),
)
return factory
}
/**
app/src/test/java/org/libremediaconverter/join/JoinPickOwnershipTest.kt
+87
View File
@@ -0,0 +1,87 @@
package org.libremediaconverter.join
import android.app.Application
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import androidx.work.workDataOf
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.ParkedPickDispatcher
import org.libremediaconverter.convert.RecordingPublisher
import org.libremediaconverter.convert.installTestWorkManager
import org.libremediaconverter.work.ConcatWorker
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
/**
* `PickOwnershipTest`'s case on the join side.
*
* `onInputsPicked` makes one write and it lands after a hop off the main thread, so it belongs to
* whichever pick was in flight rather than to whichever set of files the user last chose. Two
* selections in quick succession — likelier here than on the convert side, since a join picks
* several files at a time and the metadata query is per file — put the loser's files on screen if
* its query came back second.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class JoinPickOwnershipTest {
private lateinit var app: Application
private lateinit var parkedPick: ParkedPickDispatcher
private lateinit var viewModel: JoinViewModel
@Before
fun setUp() {
app = RuntimeEnvironment.getApplication()
ConversionDependencies.publisher = { RecordingPublisher(app) }
installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
parkedPick = ParkedPickDispatcher()
viewModel = JoinViewModel(app, pickDispatcher = parkedPick)
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
/**
* Two selections, with the first one's metadata query the slow one.
*
* The order is chosen rather than raced: both queries are parked, and this runs the second
* before the first.
*/
@Test
fun `the slower of two selections does not land on top of the faster one`() {
viewModel.onInputsPicked(FIRST)
viewModel.onInputsPicked(SECOND)
val queries = parkedPick.takeParked()
assertEquals("both selections should be in flight", 2, queries.size)
// The second selection's query comes back first; the first one's is the straggler.
queries[1].run()
queries[0].run()
val current = viewModel.state.value
assertEquals(
"a selection the user has already replaced took the screen: $current",
SECOND,
(current as JoinState.Ready).inputs.map { it.uri },
)
}
private companion object {
val FIRST = listOf(
Uri.parse("content://test/first-a.mp4"),
Uri.parse("content://test/first-b.mp4"),
)
val SECOND = listOf(
Uri.parse("content://test/second-a.mp4"),
Uri.parse("content://test/second-b.mp4"),
)
}
}
app/src/test/java/org/libremediaconverter/join/JoinReattachmentOwnershipTest.kt
+166
View File
@@ -0,0 +1,166 @@
package org.libremediaconverter.join
import android.app.Application
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import androidx.work.WorkManager
import androidx.work.workDataOf
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.ParkedPickDispatcher
import org.libremediaconverter.convert.RecordingPublisher
import org.libremediaconverter.convert.awaitState
import org.libremediaconverter.convert.installTestWorkManager
import org.libremediaconverter.model.ConcatStrategy
import org.libremediaconverter.work.ConcatWorker
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.io.File
/**
* Issue #49 on the join side, where nothing was watching for it.
*
* `reattach()` checks that the screen is still free, and then hands the answer to `observe()`,
* which writes from a *different* coroutine that has to suspend on `collect` before it can write
* anything at all. So the check happens at one moment and the write lands at another, with a
* whole pick able to fit in between:
*
* 1. `init` starts the tag query and suspends in it.
* 2. The user picks files; `onInputsPicked` suspends in its metadata query.
* 3. The query comes back. The screen is still `Idle` — step 2 has not written yet — so the
* guard passes and an observation of the old job is launched.
* 4. The pick lands. `Ready(picked)`. The user owns the screen.
* 5. The observation's first `WorkInfo` arrives and writes `Joined(yesterday's file)` over it.
*
* The comment above that guard used to say "no suspension point between this check and the
* assignment below, so nothing can interleave". There is no assignment below, and the two lines
* are in different coroutines.
*
* The convert side has been failing this on CI for two days — four occurrences across three API
* levels, each read as flaky infrastructure. `JoinViewModel` has the identical shape and no test
* at all, which is why this one was written before the fix rather than after it.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class JoinReattachmentOwnershipTest {
private lateinit var app: Application
private lateinit var publisher: RecordingPublisher
private lateinit var workManager: WorkManager
private lateinit var staged: File
@Before
fun setUp() {
app = RuntimeEnvironment.getApplication()
publisher = RecordingPublisher(app)
ConversionDependencies.publisher = { publisher }
staged = publisher.createStagingFile("joined-yesterday.mp4").apply { writeBytes(ByteArray(4096)) }
installTestWorkManager(
app,
workDataOf(
ConcatWorker.KEY_OUTPUT_PATH to staged.absolutePath,
ConcatWorker.KEY_STRATEGY to ConcatStrategy.STREAM_COPY.name,
),
)
workManager = WorkManager.getInstance(app)
// The situation reattachment exists for: a join that finished in a process that is gone,
// with its output still in the cache and nothing in the UI holding its id.
workManager.enqueue(
ConcatWorker.request(
inputs = listOf(
Uri.parse("content://test/yesterday-a.mp4"),
Uri.parse("content://test/yesterday-b.mp4"),
),
totalBytes = 8_192L,
),
).result.get()
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
/**
* The race, made into a state the test can sit in rather than one it has to catch.
*
* The pick is parked on a dispatcher this test owns, so it is in flight — issued, not yet
* written — for as long as the assertions need it to be. Everything else is real: a real
* `WorkManager` holding a real finished job, the production `reattach`, the production
* `observe`.
*
* Determinism comes from where Robolectric leaves the main looper. `reattach`'s tag query
* hops to a real [kotlinx.coroutines.Dispatchers.IO] thread, so its continuation can only
* come back as a message posted to the main looper — and that looper is paused, so it cannot
* run until something pumps it. `onInputsPicked` is an ordinary synchronous call from this
* thread. The pick is therefore always issued before the guard runs, with nothing left to
* timing.
*/
@Test
fun `a join found while the user was picking never reaches the screen`() {
val parkedPick = ParkedPickDispatcher()
val viewModel = JoinViewModel(app, pickDispatcher = parkedPick)
viewModel.onInputsPicked(PICKED)
assertEquals(
"the pick must still be in flight, or this proves something about a different situation",
1,
parkedPick.parkedCount,
)
// The control, and the reason this test does not rest on a settle window being long
// enough. A second ViewModel with nothing to supersede it reattaches to the same job
// through the same code; when it has arrived, the whole query-guard-observe-write path
// has demonstrably run to completion. `viewModel` started its own reattachment first, so
// it has had at least as long. Waiting on this rather than on a sleep is what makes the
// assertion below "it did not happen" instead of "it had not happened yet".
reattachmentHasRunToCompletion()
val current = viewModel.state.value
assertTrue("reattachment took the screen from the user: $current", current is JoinState.Idle)
// And the pick, when it lands, is what stays there.
parkedPick.runAll()
val ready = awaitState(viewModel.state, "Ready") { it is JoinState.Ready }
assertEquals(PICKED, (ready as JoinState.Ready).inputs.map { it.uri })
reattachmentHasRunToCompletion()
assertEquals("the user's pick must survive a late reattachment", ready, viewModel.state.value)
}
/**
* The other half of the contract: a reattachment nobody has superseded still takes the screen.
*
* Without this, dropping every reattachment on the floor would pass the test above. It is the
* same job, the same WorkManager and the same production path — only the pick is missing.
*/
@Test
fun `a join nobody has superseded still reaches the screen`() {
val joined = awaitState(JoinViewModel(app).state, "Joined") { it is JoinState.Joined }
assertEquals(staged.absolutePath, (joined as JoinState.Joined).staged.absolutePath)
}
/**
* Drives a throwaway ViewModel through a whole reattachment, and returns once it has landed.
*
* [awaitState] pumps the main looper, which is what runs every reattachment continuation
* waiting on it — this one's and the one belonging to the ViewModel under test, which was
* posted earlier and therefore runs first.
*/
private fun reattachmentHasRunToCompletion() {
awaitState(JoinViewModel(app).state, "Joined") { it is JoinState.Joined }
}
private companion object {
val PICKED = listOf(
Uri.parse("content://test/clip-one.mp4"),
Uri.parse("content://test/clip-two.mp4"),
)
}
}
app/src/test/java/org/libremediaconverter/join/JoinStateAffordancesTest.kt
+59
View File
@@ -18,6 +18,7 @@ import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.InputFile
import org.libremediaconverter.convert.PendingSave
import org.libremediaconverter.model.ConcatStrategy
import org.libremediaconverter.ui.TestTags
import org.robolectric.RobolectricTestRunner
@@ -219,6 +220,51 @@ class JoinStateAffordancesTest {
assertEquals(listOf("reset"), events)
}
/**
* The negative that bounds #30 on this screen. A join that died staged nothing, so its `Failed`
* carries no [PendingSave] and there is nothing a save dialog could be handed. A retry button
* rendered unconditionally here could only fail, and this is what notices.
*/
@Test
fun `a failed join offers no way to save`() {
setContent(JoinState.Failed(message = "The second file has no audio track, so joining stopped."))
composeRule.onNodeWithTag(TestTags.RETRY_SAVE).assertDoesNotExist()
composeRule.onNodeWithTag(TestTags.SAVE_FILE).assertDoesNotExist()
}
/**
* #30 on this screen: `save()` keeps the staged file when the copy out throws, and until this
* branch grew a second button the only control it rendered was "Start over" -- `reset()`, which
* deletes exactly that file. Both, not one: the restart still has to be reachable.
*/
@Test
fun `a failed join save offers the file again as well as a restart`() {
setContent(failedSave())
composeRule.onNodeWithTag(TestTags.RETRY_SAVE).assertExists()
composeRule.onNodeWithTag(TestTags.START_OVER).assertExists()
}
/** The name comes from the job, so a retry wired to a literal would hand back the wrong one. */
@Test
fun `tapping try saving again hands back the name the join chose`() {
setContent(failedSave())
composeRule.onNodeWithTag(TestTags.RETRY_SAVE).performScrollTo().performClick()
assertEquals(listOf("save:joined.mp4"), events)
}
@Test
fun `tapping start over after a failed join save resets and does not save`() {
setContent(failedSave())
composeRule.onNodeWithTag(TestTags.START_OVER).performScrollTo().performClick()
assertEquals(listOf("reset"), events)
}
/** Anything `FileRow` tagged, whichever file it is showing. The prefix comes from the table. */
private val isFileRow = SemanticsMatcher("is a join file row") { node ->
node.config.getOrNull(SemanticsProperties.TestTag)?.startsWith(TestTags.Join.fileRow("")) == true
@@ -230,6 +276,19 @@ class JoinStateAffordancesTest {
sizeBytes = 4_000_000L,
)
/**
* A `Failed` an earlier save left carrying its file, which is the only way `retry` is non-null.
* `staged` names a missing file for the same reason [joined] does -- this arm reads no length.
*/
private fun failedSave() = JoinState.Failed(
message = "There was not enough room on the destination.",
retry = PendingSave(
staged = File("no-such-staged-output.mp4"),
suggestedName = "joined.mp4",
mimeType = "video/mp4",
),
)
/** `staged` names a missing file deliberately -- see the same helper in `JoinScreenContentTest`. */
private fun joined(strategy: ConcatStrategy) = JoinState.Joined(
staged = File("no-such-staged-output.mp4"),
app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt
+201 -11
View File
@@ -11,6 +11,11 @@ import org.junit.Test
* `OutputFormat` used to be twelve hand-picked triples, and its KDoc defended that on the grounds
* that a closed set was what made routing decidable. Opening it up moves that burden here, so this
* is where decidability now has to be proven.
*
* That includes what a refusal offers instead. `Validation.Invalid` promises every suggestion is
* itself valid and names this class as the proof, so a branch that assembles its own suggestion
* list rather than going through `suggestions()` is only checked here if some row happens to reach
* it — which is how a dead-end chip survived two widenings of that table.
*/
class ContainerCapabilitiesTest {
@@ -20,6 +25,44 @@ class ContainerCapabilitiesTest {
container = Container.MP4,
)
/**
* An MP3, and the reason several rules below need a second probe.
*
* `hasVideo = false` is the load-bearing field. Every rule that reads only the spec answers the
* same for this input as for a video file, which is exactly how a spec naming a video codec was
* called valid for a file with no video track to put in it.
*/
private val mp3Source = InputProbe(
videoCodec = null,
audioCodec = "mp3",
hasVideo = false,
kind = InputKind.AUDIO_ONLY,
container = Container.MP3,
)
/**
* An audio-only input carrying a codec MP4 has no place for at all.
*
* Vorbis lives in Ogg and Matroska; MP4 carries AAC, MP3, Opus and FLAC. That gap is what turns
* a suggestion which merely drops the video track into a second refusal.
*/
private val vorbisSource = InputProbe(
videoCodec = null,
audioCodec = "vorbis",
hasVideo = false,
kind = InputKind.AUDIO_ONLY,
container = Container.OGG,
)
/** The same shape, for the other codec MP4 refuses. One case is a coincidence; two is the rule. */
private val pcmSource = InputProbe(
videoCodec = null,
audioCodec = "pcm_s16le",
hasVideo = false,
kind = InputKind.AUDIO_ONLY,
container = Container.WAV,
)
// --- copy and encode are different questions ----------------------------
/**
@@ -82,20 +125,56 @@ class ContainerCapabilitiesTest {
}
}
/** A suggestion that is itself invalid is worse than no suggestion. */
/**
* A suggestion that is itself invalid is worse than no suggestion.
*
* Only a branch that assembles its own suggestion list can break that promise: [suggestions]
* ends by filtering on `validate(...).isValid`, so everything routed through it is valid by
* construction. Those branches are what this table has to cover — the image output, and copy
* the video from a file that has none, which built its list by hand and came back refused for
* a Vorbis or PCM source into MP4 and an MP3 into WebM. The Advanced picker showed a one-tap
* fix that led straight to a second error, through two widenings of this table that never
* reached the branch.
*/
@Test
fun `every suggestion is itself valid`() {
val broken = OutputSpec(Container.WEBM, VideoCodec.H264, AudioCodec.AAC)
val result = ContainerCapabilities.validate(broken, h264Source)
val cases = listOf(
OutputSpec(Container.WEBM, VideoCodec.H264, AudioCodec.AAC) to h264Source,
// The audio-only input. Every rejection it can reach used to hand back `None + None`
// — a spec validation refuses in the next breath — because these branches built their
// suggestion by hand instead of going through the repair-and-filter path.
OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE) to mp3Source,
OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.NONE) to mp3Source,
OutputSpec(Container.MP4, VideoCodec.NONE, AudioCodec.NONE) to mp3Source,
OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.AAC) to mp3Source,
// Copy-the-video-from-a-file-with-no-video, the last branch that built its offer by
// hand. It escaped the five rows above because `spec.copy(videoCodec = NONE)` is valid
// exactly when the audio axis happens to be fine — true for the AAC and MP3 sources
// used there, false for any audio the target container cannot carry.
OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.COPY) to vorbisSource,
OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.COPY) to pcmSource,
OutputSpec(Container.WEBM, VideoCodec.COPY, AudioCodec.COPY) to mp3Source,
// The same branch with audio the container *can* hold, which is the half that already
// worked and must keep working: the repair here is a copy, so the offer is the very
// spec the caller handed to `suggestions`. It survives only because the exclusion is
// against what the user asked for rather than against the repair.
OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.COPY) to mp3Source,
// The one branch that still builds its list by hand, so that it is asserted rather
// than merely reasoned about: an image container takes `None + None` and nothing else,
// which makes its single offer valid by construction.
OutputSpec(Container.GIF, VideoCodec.H264, AudioCodec.AAC) to h264Source,
)
val invalid = result as? Validation.Invalid
?: throw AssertionError("expected H.264 in WebM to be rejected")
assertTrue("no alternatives offered", invalid.suggestions.isNotEmpty())
invalid.suggestions.forEach { suggestion ->
assertTrue(
"suggested $suggestion is itself invalid",
ContainerCapabilities.validate(suggestion, h264Source).isValid,
)
cases.forEach { (spec, probe) ->
val invalid = ContainerCapabilities.validate(spec, probe) as? Validation.Invalid
?: throw AssertionError("expected $spec to be rejected")
assertTrue("no alternatives offered for $spec on $probe", invalid.suggestions.isNotEmpty())
invalid.suggestions.forEach { suggestion ->
assertTrue(
"suggested $suggestion for $spec on $probe is itself invalid",
ContainerCapabilities.validate(suggestion, probe).isValid,
)
}
}
}
@@ -127,6 +206,117 @@ class ContainerCapabilitiesTest {
assertTrue((result as Validation.Invalid).suggestions.isNotEmpty())
}
/**
* The same rule, seen only against the probe.
*
* A video codec named for a file with no video track is dropped, not encoded — so
* MP4/H.265/None on an MP3 empties the output exactly as None/None does. Reading the spec
* alone answered "valid" because the spec names a video codec, and the job went to Media3,
* where `EditedMediaItem.Builder` refuses a composition with both tracks removed by throwing
* on Transformer's own HandlerThread.
*/
@Test
fun `a video codec named for a file with no video track and no audio is refused`() {
ContainerCapabilities.encodableVideo(Container.MP4).forEach { codec ->
val spec = OutputSpec(Container.MP4, codec, AudioCodec.NONE)
val result = ContainerCapabilities.validate(spec, mp3Source)
assertFalse(
"MP4/${codec.label}/None on an audio-only input plans to (Drop, Drop) and would " +
"produce an empty file; it must be refused. Got $result",
result.isValid,
)
}
}
/**
* The refusal is only worth having if it leads somewhere.
*
* The COPY form of this was already refused, but its one hand-built suggestion was
* `None + None` — which validation refuses in the next breath, so the Advanced picker offered
* a one-tap fix that fixed nothing. Every face of the rule now goes through the shared
* suggestion path, so the offer keeps the one track the input actually has.
*/
@Test
fun `refusing an empty output still offers a way to keep the audio`() {
listOf(VideoCodec.H265, VideoCodec.H264, VideoCodec.COPY, VideoCodec.NONE).forEach { codec ->
val spec = OutputSpec(Container.MP4, codec, AudioCodec.NONE)
val invalid = ContainerCapabilities.validate(spec, mp3Source) as? Validation.Invalid
?: throw AssertionError("expected MP4/${codec.label}/None to be rejected")
assertTrue(
"a refusal with no way out is a dead end in the Advanced picker",
invalid.suggestions.isNotEmpty(),
)
assertTrue(
"every suggestion must keep a track, got ${invalid.suggestions}",
invalid.suggestions.all { it.audioCodec != AudioCodec.NONE },
)
}
}
/**
* A repair must not name a track the input does not have.
*
* `repairVideo` used to fall through to "the first codec this container can encode" whenever
* nothing else fitted, and for an MP3 that produced the non-sequitur `MP4 · H.264 · Copy`.
* It validated, so nothing caught it — but [CopyPlanner] drops that video track anyway, which
* makes the codec in the offer a fiction.
*/
@Test
fun `a repair for a file with no video track never names a video codec`() {
listOf(
OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE),
OutputSpec(Container.MP4, VideoCodec.NONE, AudioCodec.NONE),
OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.NONE),
).forEach { spec ->
val invalid = ContainerCapabilities.validate(spec, mp3Source) as Validation.Invalid
invalid.suggestions.forEach {
assertEquals(
"offering ${it.videoCodec.label} for a file with no video track is a fiction; " +
"CopyPlanner drops it. Suggested $it for $spec",
VideoCodec.NONE,
it.videoCodec,
)
}
}
}
/**
* The rule stated as the property it is, over the whole matrix.
*
* A plan of (Drop, Drop) is precisely the composition `EditedMediaItem.Builder` refuses to
* build, so no non-image spec that reaches it may be called valid. Sweeping every container ×
* codec × codec against both probes is what stops the next container or codec from
* reintroducing the gap on an axis nobody thought to write a case for.
*
* Image outputs are exempt and deliberately so: GIF and PNG frames carry no codecs at all, and
* `None + None` is the only spec they accept — but they never reach Media3, because the router
* sends every image output to FFmpeg.
*/
@Test
fun `no valid non-image spec plans to remove both tracks`() {
val specs = Container.entries
.filterNot { it == Container.GIF || it == Container.IMAGE_SEQUENCE }
.flatMap { container -> VideoCodec.entries.map { container to it } }
.flatMap { (container, video) -> AudioCodec.entries.map { OutputSpec(container, video, it) } }
val cases = specs.flatMap { spec -> listOf(h264Source, mp3Source).map { spec to it } }
val empties = cases.filter { (spec, probe) ->
val plan = CopyPlanner.plan(spec, probe)
plan.video == VideoPlan.Drop && plan.audio == AudioPlan.Drop
}
assertTrue("the sweep found nothing to check — the filter has gone wrong", empties.isNotEmpty())
empties.forEach { (spec, probe) ->
assertFalse(
"$spec on $probe plans to (Drop, Drop) — an empty file, and the composition " +
"Media3 cannot build — so it must not validate",
ContainerCapabilities.validate(spec, probe).isValid,
)
}
}
@Test
fun `copying is offered as the fix when the codec is right but unencodable`() {
val av1Source = InputProbe(videoCodec = "av1", audioCodec = "aac", container = Container.MKV)
app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt
+28
View File
@@ -350,6 +350,34 @@ class ConversionRouterTest {
}
}
/**
* Why `Media3Engine` still needs a guard of its own.
*
* `ContainerCapabilities.validate` now refuses "a video codec with the audio off" for an input
* with no video track, so neither the picker nor `ConversionWorker` will start one. Routing is
* a separate question and still answers MEDIA3 — nothing about a dropped track makes the job
* un-hardware-able — so a request that skips validation, from a direct
* `ConversionWorker.request(...)` or a job queued before the settings changed, arrives at the
* engine with a plan Media3 cannot build. That has to fail the job, not the process.
*/
@Test
fun `a plan that drops both tracks still routes to media3`() {
val audioOnly = InputProbe(
videoCodec = null,
audioCodec = "mp3",
hasVideo = false,
container = Container.MP3,
kind = InputKind.AUDIO_ONLY,
)
val spec = OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE)
val plan = CopyPlanner.plan(spec, audioOnly)
assertEquals(VideoPlan.Drop, plan.video)
assertEquals(AudioPlan.Drop, plan.audio)
assertEquals(Engine.MEDIA3, route(spec, probe = audioOnly).engine)
}
@Test
fun `audio-only formats are flagged as such`() {
assertEquals(true, OutputFormat.MP3.isAudioOnly)
app/src/test/java/org/libremediaconverter/model/CopyPlannerTest.kt
+27
View File
@@ -146,6 +146,33 @@ class CopyPlannerTest {
assertTrue("copying the only track is still a remux", plan.isPureRemux)
}
/**
* The one plan `Media3Engine` cannot be handed.
*
* `EditedMediaItem.Builder` refuses a composition with both tracks removed —
* checkState("Audio and video cannot both be removed") — and this is how an ordinary-looking
* spec reaches it: a video codec named for a file that has no video, with the audio switched
* off. Neither half is unusual on its own, which is why validation could read the spec, see a
* video codec, and call it fine.
*/
@Test
fun `an audio-only source with the audio dropped removes both tracks`() {
val audioOnly = InputProbe(
videoCodec = null,
audioCodec = "mp3",
hasVideo = false,
container = Container.MP3,
kind = InputKind.AUDIO_ONLY,
)
val plan = CopyPlanner.plan(
OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE),
audioOnly,
)
assertEquals(VideoPlan.Drop, plan.video)
assertEquals(AudioPlan.Drop, plan.audio)
assertTrue("an empty plan is not a remux", !plan.isPureRemux)
}
@Test
fun `copying one track and encoding the other is not a pure remux`() {
val plan = CopyPlanner.plan(
app/src/test/java/org/libremediaconverter/ui/theme/ThemeColorSchemeTest.kt
+131
View File
@@ -0,0 +1,131 @@
package org.libremediaconverter.ui.theme
import androidx.compose.material3.ColorScheme
import androidx.compose.material3.MaterialTheme
import androidx.compose.ui.graphics.luminance
import androidx.compose.ui.test.junit4.v2.createComposeRule
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotEquals
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
/**
* The theme has to resolve the scheme its arguments name, and `ThemeKt` had no test at all --
* 23 lines, none of them covered, which is how #68 was found.
*
* Only two of the four branches in [LibreMediaConverterTheme]'s `when` are reachable from the
* app. `MainActivity` is the single call site and passes no arguments, so `dynamicColor` is
* always `true` and the live choice is between the dynamic dark and dynamic light schemes.
* Those two are what ships, and asserting on them survives whichever way #68 is decided.
*
* **The other two branches have no caller.** `dynamicColor = false` is passed below by this
* test and by nothing else in `app/src`, so the coverage it produces is not evidence that a
* switch exists -- misreading it that way is the whole reason #68 was filed. #68 is the open
* decision about whether one ever will exist.
*
* What the assertions distinguish the branches on was measured under Robolectric `sdk=36`
* rather than assumed. The dynamic palette resolves to the platform's own default there --
* dark background `#121318` against light `#FAF8FF`, dark primary `#B0C6FF` -- and that is a
* different hue from the brand palette's [Purple80] / [Purple40]. A dynamic scheme reads the
* device, so those exact values belong to the Robolectric stub and to no particular phone,
* which is why the live-branch tests compare the two resolved schemes against each other
* instead of hard-coding either one.
*/
@RunWith(RobolectricTestRunner::class)
class ThemeColorSchemeTest {
// The **v2** rule (`androidx.compose.ui.test.junit4.v2`), as everywhere else in this
// source set.
@get:Rule
val composeRule = createComposeRule()
/**
* Every scheme the `when` can produce, read out of [MaterialTheme] inside the content
* lambda -- the only place that shows what the theme actually chose, rather than what the
* caller hoped for.
*
* All four are resolved in one composition because `setContent` may be called once per
* test, and a comparison needs at least two of them.
*/
private fun resolveAll(): Schemes {
lateinit var dynamicDark: ColorScheme
lateinit var dynamicLight: ColorScheme
lateinit var brandDark: ColorScheme
lateinit var brandLight: ColorScheme
composeRule.setContent {
LibreMediaConverterTheme(darkTheme = true) { dynamicDark = MaterialTheme.colorScheme }
LibreMediaConverterTheme(darkTheme = false) { dynamicLight = MaterialTheme.colorScheme }
LibreMediaConverterTheme(darkTheme = true, dynamicColor = false) {
brandDark = MaterialTheme.colorScheme
}
LibreMediaConverterTheme(darkTheme = false, dynamicColor = false) {
brandLight = MaterialTheme.colorScheme
}
}
composeRule.waitForIdle()
return Schemes(dynamicDark, dynamicLight, brandDark, brandLight)
}
private class Schemes(
val dynamicDark: ColorScheme,
val dynamicLight: ColorScheme,
val brandDark: ColorScheme,
val brandLight: ColorScheme,
)
/**
* The live branches, and the one assertion that catches them being swapped: both dynamic
* schemes come from the same device palette, so they are similar enough that identity or a
* bare inequality would prove nothing. Background luminance is not similar -- it is the
* thing dark mode is for.
*/
@Test
fun `dark mode resolves a darker scheme than light mode`() {
val schemes = resolveAll()
val dark = schemes.dynamicDark.background.luminance()
val light = schemes.dynamicLight.background.luminance()
assertTrue(
"darkTheme = true should resolve the dynamic dark scheme, whose background " +
"luminance ($dark) is below the light scheme's ($light)",
dark < light,
)
}
/**
* Which of the two dark branches ran, and which of the two light ones: the scheme a live
* call resolves is the dynamic one, not the brand palette sitting next to it.
*/
@Test
fun `the live branches take the dynamic palette rather than the brand one`() {
val schemes = resolveAll()
assertNotEquals(
"the default dynamicColor = true should not resolve the brand dark palette",
Purple80,
schemes.dynamicDark.primary,
)
assertNotEquals(
"the default dynamicColor = true should not resolve the brand light palette",
Purple40,
schemes.dynamicLight.primary,
)
}
/**
* The two dead branches. Nothing in `app/src` passes `dynamicColor = false`; this test
* does it directly, because the parameter is public, and that is the only way either
* branch runs. Covered so a later decision on #68 starts from a tested `when` -- not
* because the brand palette is reachable in the app.
*/
@Test
fun `the brand palette branches run only when dynamicColor is passed explicitly`() {
val schemes = resolveAll()
assertEquals(Purple80, schemes.brandDark.primary)
assertEquals(Purple40, schemes.brandLight.primary)
}
}