Compare commits
4
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a507736d3d | ||
|
|
70337b2c12 | ||
|
|
ddfb1dd78e | ||
|
|
348eaa2f22 |
@@ -6,6 +6,7 @@ import android.util.Log
|
|||||||
import androidx.lifecycle.AndroidViewModel
|
import androidx.lifecycle.AndroidViewModel
|
||||||
import androidx.lifecycle.viewModelScope
|
import androidx.lifecycle.viewModelScope
|
||||||
import androidx.media3.common.util.UnstableApi
|
import androidx.media3.common.util.UnstableApi
|
||||||
|
import androidx.work.Data
|
||||||
import androidx.work.WorkInfo
|
import androidx.work.WorkInfo
|
||||||
import androidx.work.WorkManager
|
import androidx.work.WorkManager
|
||||||
import kotlinx.coroutines.CoroutineDispatcher
|
import kotlinx.coroutines.CoroutineDispatcher
|
||||||
@@ -71,6 +72,119 @@ data class InputFile(
|
|||||||
val probe: InputProbe? = null,
|
val probe: InputProbe? = null,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
/**
|
||||||
|
* One update about a running conversion, as WorkManager last reported it.
|
||||||
|
*
|
||||||
|
* Only the fields [conversionStateFrom] reads — the same shape, and for the same reason, as
|
||||||
|
* `JobSnapshot` beside `Reattachment.choose`: the rule stays testable on the JVM because nothing
|
||||||
|
* in it needs a `WorkInfo`, which a test cannot readily build.
|
||||||
|
*
|
||||||
|
* [outputData] stays a `Data` rather than being unpacked into five nullable strings. It is what a
|
||||||
|
* test already builds with `workDataOf` everywhere in this suite, so unpacking would move the same
|
||||||
|
* reads without making anything easier to drive.
|
||||||
|
*/
|
||||||
|
internal data class ConversionUpdate(
|
||||||
|
val state: WorkInfo.State,
|
||||||
|
val progressPercent: Int,
|
||||||
|
val runAttemptCount: Int,
|
||||||
|
val outputData: Data,
|
||||||
|
)
|
||||||
|
|
||||||
|
/**
|
||||||
|
* What the screen should show, given what WorkManager last said about the job.
|
||||||
|
*
|
||||||
|
* ## Why this is a function rather than the body of a `collect`
|
||||||
|
*
|
||||||
|
* It was the body of one. `workManager` is built in the constructor from `WorkManager.getInstance`,
|
||||||
|
* `observe` is private, and nothing could hand either a chosen `WorkInfo` — so every arm below ran
|
||||||
|
* only when a real worker happened to produce it. A real worker produces a terminal state with
|
||||||
|
* well-formed output, which meant six of these arms had never been chosen by any test: the progress
|
||||||
|
* read, both sides of the retry check, a success with no file, a failure with nothing to say, and
|
||||||
|
* the two that map to a state the user cannot otherwise reach.
|
||||||
|
*
|
||||||
|
* That is the argument #141 made for `MediaProbe`'s track walk, against `WorkManager` instead of a
|
||||||
|
* media fixture, and it takes the same answer: the branch matrix is a pure function, and what is
|
||||||
|
* left needing the framework — the flow, the null check, the ownership check — is the thin edge.
|
||||||
|
*
|
||||||
|
* ## What is deliberately *not* in here
|
||||||
|
*
|
||||||
|
* The ownership check stays at the call site. Its comment is explicit that it guards the file
|
||||||
|
* ownership the `SUCCEEDED` arm takes, not merely the assignment, so moving it inside would change
|
||||||
|
* what it protects. And this function takes no responsibility for the staged file: it returns the
|
||||||
|
* state, and the caller reads the file off it. A pure function that deletes files is not a seam.
|
||||||
|
*
|
||||||
|
* @param cancelled where a cancellation lands, which differs for a reattached job — see [observe].
|
||||||
|
* @param fallbackSpec the current settings, read only when finished work predates the worker
|
||||||
|
* reporting its own name and MIME type.
|
||||||
|
*/
|
||||||
|
@UnstableApi
|
||||||
|
internal fun conversionStateFrom(
|
||||||
|
update: ConversionUpdate,
|
||||||
|
input: InputFile,
|
||||||
|
cancelled: ConversionState,
|
||||||
|
fallbackSpec: OutputSpec,
|
||||||
|
): ConversionState = when (update.state) {
|
||||||
|
WorkInfo.State.RUNNING -> ConversionState.Converting(input, update.progressPercent)
|
||||||
|
|
||||||
|
// ENQUEUED after a run means a retry is pending. Either the six-hour foreground budget ran out
|
||||||
|
// mid-job, or the system refused to let the job start again while the app was in the background
|
||||||
|
// — the second being the likelier of the two, since it needs only a process restart. Nothing
|
||||||
|
// here can tell them apart, and nothing needs to: the answer is the same.
|
||||||
|
WorkInfo.State.ENQUEUED ->
|
||||||
|
if (update.runAttemptCount > 0) {
|
||||||
|
ConversionState.Waiting(input)
|
||||||
|
} else {
|
||||||
|
ConversionState.Converting(input, 0)
|
||||||
|
}
|
||||||
|
|
||||||
|
WorkInfo.State.SUCCEEDED -> convertedFrom(update.outputData, input, fallbackSpec)
|
||||||
|
|
||||||
|
// A worker that dies before it can report anything leaves no output data at all — a
|
||||||
|
// foreground-service start refused after a process restart is one way — and an exception's
|
||||||
|
// message can be an empty string. Both would read as a failure with nothing said, so blank
|
||||||
|
// falls back like missing does.
|
||||||
|
WorkInfo.State.FAILED -> ConversionState.Failed(
|
||||||
|
update.outputData.getString(ConversionWorker.KEY_ERROR)
|
||||||
|
?.takeIf { it.isNotBlank() }
|
||||||
|
?: ConversionWorker.GENERIC_FAILURE_MESSAGE,
|
||||||
|
)
|
||||||
|
|
||||||
|
WorkInfo.State.CANCELLED -> cancelled
|
||||||
|
WorkInfo.State.BLOCKED -> ConversionState.Converting(input, 0)
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The `SUCCEEDED` arm, which is the only one that reads more than one field.
|
||||||
|
*
|
||||||
|
* Split out so [conversionStateFrom] stays a table of one line per state. A success with no output
|
||||||
|
* path is a failure: the job said it finished and named nothing, and there is no file to offer.
|
||||||
|
*/
|
||||||
|
@UnstableApi
|
||||||
|
private fun convertedFrom(outputData: Data, input: InputFile, fallbackSpec: OutputSpec): ConversionState {
|
||||||
|
val path = outputData.getString(ConversionWorker.KEY_OUTPUT_PATH)
|
||||||
|
?: return ConversionState.Failed(SUCCEEDED_WITHOUT_A_FILE_MESSAGE)
|
||||||
|
return ConversionState.Converted(
|
||||||
|
input = input,
|
||||||
|
staged = File(path),
|
||||||
|
engineUsed = outputData.getString(ConversionWorker.KEY_ENGINE_USED).orEmpty(),
|
||||||
|
routeReason = outputData.getString(ConversionWorker.KEY_ROUTE_REASON).orEmpty(),
|
||||||
|
// Work enqueued before the worker reported this carries nothing, and WorkManager keeps
|
||||||
|
// finished work for about a week -- so this branch is ordinary for a few days rather than a
|
||||||
|
// corner. It is the old derivation, kept because it is the same guess the app already made
|
||||||
|
// and there is genuinely nothing better available for such a job. New work never reaches it.
|
||||||
|
suggestedName = outputData.getString(ConversionWorker.KEY_SUGGESTED_NAME)
|
||||||
|
?.takeIf { it.isNotBlank() }
|
||||||
|
?: ConversionWorker.outputNameFor(input.displayName, fallbackSpec),
|
||||||
|
mimeType = outputData.getString(ConversionWorker.KEY_MIME_TYPE)
|
||||||
|
?.takeIf { it.isNotBlank() }
|
||||||
|
?: fallbackSpec.mimeType,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/** A job that reported success and named no file. There is nothing to offer the user to save. */
|
||||||
|
internal const val SUCCEEDED_WITHOUT_A_FILE_MESSAGE: String =
|
||||||
|
"Conversion reported success but produced no file."
|
||||||
|
|
||||||
sealed interface ConversionState {
|
sealed interface ConversionState {
|
||||||
data object Idle : ConversionState
|
data object Idle : ConversionState
|
||||||
data class Ready(val input: InputFile) : ConversionState
|
data class Ready(val input: InputFile) : ConversionState
|
||||||
@@ -442,75 +556,24 @@ class ConversionViewModel @JvmOverloads constructor(
|
|||||||
// that either. The state and `pendingStaged` are meant to refer to the same file
|
// 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.
|
// or to no file, and this is where that stays true.
|
||||||
if (!ownership.stillHeldBy(token)) return@collect
|
if (!ownership.stillHeldBy(token)) return@collect
|
||||||
_state.value = when (info.state) {
|
val next = conversionStateFrom(
|
||||||
WorkInfo.State.RUNNING -> ConversionState.Converting(
|
ConversionUpdate(
|
||||||
input,
|
state = info.state,
|
||||||
info.progress.getInt(ConversionWorker.KEY_PROGRESS, 0),
|
progressPercent = info.progress.getInt(ConversionWorker.KEY_PROGRESS, 0),
|
||||||
)
|
runAttemptCount = info.runAttemptCount,
|
||||||
|
outputData = info.outputData,
|
||||||
// ENQUEUED after a run means a retry is pending. Either the six-hour
|
|
||||||
// foreground budget ran out mid-job, or the system refused to let the job
|
|
||||||
// start again while the app was in the background — the second being the
|
|
||||||
// likelier of the two, since it needs only a process restart. Nothing here
|
|
||||||
// can tell them apart, and nothing needs to: the answer is the same.
|
|
||||||
WorkInfo.State.ENQUEUED ->
|
|
||||||
if (info.runAttemptCount > 0) {
|
|
||||||
ConversionState.Waiting(input)
|
|
||||||
} else {
|
|
||||||
ConversionState.Converting(input, 0)
|
|
||||||
}
|
|
||||||
|
|
||||||
WorkInfo.State.SUCCEEDED -> {
|
|
||||||
val path = info.outputData.getString(ConversionWorker.KEY_OUTPUT_PATH)
|
|
||||||
if (path == null) {
|
|
||||||
ConversionState.Failed("Conversion reported success but produced no file.")
|
|
||||||
} else {
|
|
||||||
val staged = File(path)
|
|
||||||
// Take responsibility for the file at the same moment the state
|
|
||||||
// starts referring to it, so the two cannot disagree.
|
|
||||||
pendingStaged = staged
|
|
||||||
ConversionState.Converted(
|
|
||||||
input = input,
|
|
||||||
staged = staged,
|
|
||||||
engineUsed = info.outputData
|
|
||||||
.getString(ConversionWorker.KEY_ENGINE_USED).orEmpty(),
|
|
||||||
routeReason = info.outputData
|
|
||||||
.getString(ConversionWorker.KEY_ROUTE_REASON).orEmpty(),
|
|
||||||
suggestedName = info.outputData
|
|
||||||
.getString(ConversionWorker.KEY_SUGGESTED_NAME)
|
|
||||||
?.takeIf { it.isNotBlank() }
|
|
||||||
// Work enqueued before the worker reported this carries
|
|
||||||
// nothing, and WorkManager keeps finished work for about a
|
|
||||||
// week -- so this branch is ordinary for a few days rather
|
|
||||||
// than a corner. It is the old derivation, kept because it is
|
|
||||||
// the same guess the app already made and there is genuinely
|
|
||||||
// nothing better available for such a job. New work never
|
|
||||||
// reaches it.
|
|
||||||
?: ConversionWorker.outputNameFor(
|
|
||||||
input.displayName,
|
|
||||||
_settings.value.spec,
|
|
||||||
),
|
),
|
||||||
mimeType = info.outputData
|
input = input,
|
||||||
.getString(ConversionWorker.KEY_MIME_TYPE)
|
cancelled = cancelled,
|
||||||
?.takeIf { it.isNotBlank() }
|
fallbackSpec = _settings.value.spec,
|
||||||
?: _settings.value.spec.mimeType,
|
|
||||||
)
|
)
|
||||||
}
|
// Take responsibility for the file at the same moment the state starts referring
|
||||||
}
|
// to it, so the two cannot disagree. Read off the result rather than assigned
|
||||||
|
// inside the mapping: `Converted` is the only state that carries a staged file, so
|
||||||
// A worker that dies before it can report anything leaves no output data at
|
// "the state and `pendingStaged` refer to the same file or to no file" is now the
|
||||||
// all — a foreground-service start refused after a process restart is one
|
// shape of the code rather than a rule two branches have to keep.
|
||||||
// way — and an exception's message can be an empty string. Both would read
|
if (next is ConversionState.Converted) pendingStaged = next.staged
|
||||||
// as a failure with nothing said, so blank falls back like missing does.
|
_state.value = next
|
||||||
WorkInfo.State.FAILED -> ConversionState.Failed(
|
|
||||||
info.outputData.getString(ConversionWorker.KEY_ERROR)
|
|
||||||
?.takeIf { it.isNotBlank() }
|
|
||||||
?: ConversionWorker.GENERIC_FAILURE_MESSAGE,
|
|
||||||
)
|
|
||||||
|
|
||||||
WorkInfo.State.CANCELLED -> cancelled
|
|
||||||
WorkInfo.State.BLOCKED -> ConversionState.Converting(input, 0)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,238 @@
|
|||||||
|
package org.libremediaconverter.convert
|
||||||
|
|
||||||
|
import android.net.Uri
|
||||||
|
import androidx.media3.common.util.UnstableApi
|
||||||
|
import androidx.work.Data
|
||||||
|
import androidx.work.WorkInfo
|
||||||
|
import androidx.work.workDataOf
|
||||||
|
import org.junit.Assert.assertEquals
|
||||||
|
import org.junit.Test
|
||||||
|
import org.junit.runner.RunWith
|
||||||
|
import org.libremediaconverter.model.OutputFormat
|
||||||
|
import org.libremediaconverter.work.ConversionWorker
|
||||||
|
import org.robolectric.RobolectricTestRunner
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Every answer [conversionStateFrom] can give, chosen rather than stumbled into.
|
||||||
|
*
|
||||||
|
* ## What this revises
|
||||||
|
*
|
||||||
|
* The mapping is not cold code and never was: `ConversionViewModel$observe$1$1` reported 28 covered
|
||||||
|
* lines before this file existed, because every test that drives a real worker runs it. What no
|
||||||
|
* test did was **choose which arm it took**. A real worker reaches a terminal state with
|
||||||
|
* well-formed output, so `SUCCEEDED`-with-a-path and `FAILED`-with-a-message were the only arms any
|
||||||
|
* test had ever produced — the other six ran never.
|
||||||
|
*
|
||||||
|
* A `grep` for `WorkInfo.State.` across the JVM suite makes that look untrue: all six constants are
|
||||||
|
* there. They are in `ReattachmentTest`, driven into **`Reattachment.choose`** — a different
|
||||||
|
* function that encodes the same enqueued-means-retry rule. So that rule had a test in one of its
|
||||||
|
* two homes, and the copy the user's screen reads had none.
|
||||||
|
*
|
||||||
|
* ## Why the seam, and why these assertions
|
||||||
|
*
|
||||||
|
* `WorkManager.getInstance` is called in the ViewModel's constructor and `observe` is private, so
|
||||||
|
* nothing could hand this a chosen `WorkInfo`. Cutting the `when` out as a pure function over
|
||||||
|
* [ConversionUpdate] is the answer #141 took for `MediaProbe`, and `JobSnapshot` beside
|
||||||
|
* `Reattachment.choose` is the same shape again.
|
||||||
|
*
|
||||||
|
* The assertions are on the whole state, not on its type. `Converting(input, 40)` and
|
||||||
|
* `Converting(input, 0)` are both `Converting`, and a mapping that dropped the progress read would
|
||||||
|
* pass any test that only asked which class came back.
|
||||||
|
*/
|
||||||
|
@UnstableApi
|
||||||
|
@RunWith(RobolectricTestRunner::class)
|
||||||
|
class ConversionStateMappingTest {
|
||||||
|
|
||||||
|
// --- running ------------------------------------------------------------
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a running job reports the progress it published`() {
|
||||||
|
// The percent is read from `progress`, not from `outputData`, and not from the settings.
|
||||||
|
// A mapping that returned Converting(input, 0) for every RUNNING would leave the bar
|
||||||
|
// pinned at zero for the whole conversion.
|
||||||
|
val state = map(WorkInfo.State.RUNNING, progress = 40)
|
||||||
|
|
||||||
|
assertEquals(ConversionState.Converting(INPUT, 40), state)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a running job with no published progress reports zero rather than failing`() {
|
||||||
|
// getInt's default. A worker that has started but not yet called setProgress is ordinary,
|
||||||
|
// and must not read as an error.
|
||||||
|
val state = map(WorkInfo.State.RUNNING, progress = null)
|
||||||
|
|
||||||
|
assertEquals(ConversionState.Converting(INPUT, 0), state)
|
||||||
|
}
|
||||||
|
|
||||||
|
// --- enqueued: the rule that had a test only in its other home ----------
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `an enqueued job that has already run is waiting to retry`() {
|
||||||
|
val state = map(WorkInfo.State.ENQUEUED, runAttemptCount = 1)
|
||||||
|
|
||||||
|
assertEquals(
|
||||||
|
"an ENQUEUED after a run is a pending retry, which the user is told about",
|
||||||
|
ConversionState.Waiting(INPUT),
|
||||||
|
state,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `an enqueued job that has never run is simply starting`() {
|
||||||
|
// The other side, and the reason the test above is not enough on its own: a mapping that
|
||||||
|
// ignored runAttemptCount and always answered Waiting would pass that one and fail this.
|
||||||
|
val state = map(WorkInfo.State.ENQUEUED, runAttemptCount = 0)
|
||||||
|
|
||||||
|
assertEquals(ConversionState.Converting(INPUT, 0), state)
|
||||||
|
}
|
||||||
|
|
||||||
|
// --- succeeded ----------------------------------------------------------
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a success that named no file is a failure, not an empty success`() {
|
||||||
|
// The job said it finished and named nothing. There is no file to offer, so `Converted`
|
||||||
|
// would put a Save button over a path that does not exist.
|
||||||
|
val state = map(WorkInfo.State.SUCCEEDED, data = Data.EMPTY)
|
||||||
|
|
||||||
|
assertEquals(ConversionState.Failed(SUCCEEDED_WITHOUT_A_FILE_MESSAGE), state)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a success carries the worker's own name and type, not the current settings`() {
|
||||||
|
val state = map(
|
||||||
|
WorkInfo.State.SUCCEEDED,
|
||||||
|
data = workDataOf(
|
||||||
|
ConversionWorker.KEY_OUTPUT_PATH to "/cache/conversions/out.mkv",
|
||||||
|
ConversionWorker.KEY_SUGGESTED_NAME to "holiday.mkv",
|
||||||
|
ConversionWorker.KEY_MIME_TYPE to "video/x-matroska",
|
||||||
|
ConversionWorker.KEY_ENGINE_USED to "FFMPEG",
|
||||||
|
ConversionWorker.KEY_ROUTE_REASON to "container needs FFmpeg",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
val converted = state as ConversionState.Converted
|
||||||
|
assertEquals("holiday.mkv", converted.suggestedName)
|
||||||
|
assertEquals("video/x-matroska", converted.mimeType)
|
||||||
|
assertEquals("FFMPEG", converted.engineUsed)
|
||||||
|
assertEquals("container needs FFmpeg", converted.routeReason)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a success from older work falls back to the current settings for name and type`() {
|
||||||
|
// WorkManager keeps finished work about a week, so a job enqueued before the worker
|
||||||
|
// reported these is ordinary for a few days rather than a corner case.
|
||||||
|
val state = map(
|
||||||
|
WorkInfo.State.SUCCEEDED,
|
||||||
|
data = workDataOf(ConversionWorker.KEY_OUTPUT_PATH to "/cache/conversions/out.mp4"),
|
||||||
|
)
|
||||||
|
|
||||||
|
val converted = state as ConversionState.Converted
|
||||||
|
assertEquals(FALLBACK_SPEC.mimeType, converted.mimeType)
|
||||||
|
assertEquals(
|
||||||
|
ConversionWorker.outputNameFor(INPUT.displayName, FALLBACK_SPEC),
|
||||||
|
converted.suggestedName,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a blank name or type falls back the same way a missing one does`() {
|
||||||
|
// A blank string is not an answer. Without takeIf, the save dialog opens named "" and
|
||||||
|
// registered for a MIME type of "", which no provider will accept.
|
||||||
|
val state = map(
|
||||||
|
WorkInfo.State.SUCCEEDED,
|
||||||
|
data = workDataOf(
|
||||||
|
ConversionWorker.KEY_OUTPUT_PATH to "/cache/conversions/out.mp4",
|
||||||
|
ConversionWorker.KEY_SUGGESTED_NAME to "",
|
||||||
|
ConversionWorker.KEY_MIME_TYPE to " ",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
val converted = state as ConversionState.Converted
|
||||||
|
assertEquals(FALLBACK_SPEC.mimeType, converted.mimeType)
|
||||||
|
assertEquals(
|
||||||
|
ConversionWorker.outputNameFor(INPUT.displayName, FALLBACK_SPEC),
|
||||||
|
converted.suggestedName,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// --- failed -------------------------------------------------------------
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a failure carries the reason the worker gave`() {
|
||||||
|
val state = map(
|
||||||
|
WorkInfo.State.FAILED,
|
||||||
|
data = workDataOf(ConversionWorker.KEY_ERROR to "Not enough free space to convert."),
|
||||||
|
)
|
||||||
|
|
||||||
|
assertEquals(ConversionState.Failed("Not enough free space to convert."), state)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a failure with nothing said still says something`() {
|
||||||
|
// A worker killed before it could write output data leaves none at all -- a refused
|
||||||
|
// foreground start after a process restart is one way. Failed("") would render as a blank
|
||||||
|
// error card.
|
||||||
|
val state = map(WorkInfo.State.FAILED, data = Data.EMPTY)
|
||||||
|
|
||||||
|
assertEquals(ConversionState.Failed(ConversionWorker.GENERIC_FAILURE_MESSAGE), state)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a failure whose message is blank falls back like a missing one`() {
|
||||||
|
val state = map(
|
||||||
|
WorkInfo.State.FAILED,
|
||||||
|
data = workDataOf(ConversionWorker.KEY_ERROR to " "),
|
||||||
|
)
|
||||||
|
|
||||||
|
assertEquals(ConversionState.Failed(ConversionWorker.GENERIC_FAILURE_MESSAGE), state)
|
||||||
|
}
|
||||||
|
|
||||||
|
// --- cancelled and blocked ---------------------------------------------
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a cancellation lands wherever the caller said it should`() {
|
||||||
|
// Not a fixed state: a conversion started here goes back to Ready with the picked file,
|
||||||
|
// while one picked up by reattach goes to Idle, because the URI that job holds belongs to
|
||||||
|
// a process that no longer exists. `observe`'s KDoc is where that distinction is set.
|
||||||
|
val toReady = map(WorkInfo.State.CANCELLED, cancelled = ConversionState.Ready(INPUT))
|
||||||
|
val toIdle = map(WorkInfo.State.CANCELLED, cancelled = ConversionState.Idle)
|
||||||
|
|
||||||
|
assertEquals(ConversionState.Ready(INPUT), toReady)
|
||||||
|
assertEquals(ConversionState.Idle, toIdle)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `a blocked job looks like one that is starting`() {
|
||||||
|
// BLOCKED is a job waiting on a prerequisite. There is nothing useful to say about it that
|
||||||
|
// differs from "starting", and inventing a state for it would put a word on screen the
|
||||||
|
// user cannot act on.
|
||||||
|
val state = map(WorkInfo.State.BLOCKED)
|
||||||
|
|
||||||
|
assertEquals(ConversionState.Converting(INPUT, 0), state)
|
||||||
|
}
|
||||||
|
|
||||||
|
private fun map(
|
||||||
|
state: WorkInfo.State,
|
||||||
|
progress: Int? = null,
|
||||||
|
runAttemptCount: Int = 0,
|
||||||
|
data: Data = Data.EMPTY,
|
||||||
|
cancelled: ConversionState = ConversionState.Ready(INPUT),
|
||||||
|
): ConversionState = conversionStateFrom(
|
||||||
|
ConversionUpdate(
|
||||||
|
state = state,
|
||||||
|
// Modelled on the call site, which reads `getInt(KEY_PROGRESS, 0)` -- so "no progress
|
||||||
|
// published" is the default reaching the mapping, not a null it has to handle.
|
||||||
|
progressPercent = progress ?: 0,
|
||||||
|
runAttemptCount = runAttemptCount,
|
||||||
|
outputData = data,
|
||||||
|
),
|
||||||
|
input = INPUT,
|
||||||
|
cancelled = cancelled,
|
||||||
|
fallbackSpec = FALLBACK_SPEC,
|
||||||
|
)
|
||||||
|
|
||||||
|
private companion object {
|
||||||
|
val INPUT = InputFile(Uri.parse("content://test/holiday.mov"), "holiday.mov", 4096L)
|
||||||
|
val FALLBACK_SPEC = OutputFormat.MP4_H265.spec
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,197 @@
|
|||||||
|
package org.libremediaconverter.convert
|
||||||
|
|
||||||
|
import android.app.Application
|
||||||
|
import androidx.media3.common.util.UnstableApi
|
||||||
|
import androidx.work.Data
|
||||||
|
import org.junit.After
|
||||||
|
import org.junit.Assert.assertEquals
|
||||||
|
import org.junit.Assert.assertNotEquals
|
||||||
|
import org.junit.Assert.assertNull
|
||||||
|
import org.junit.Before
|
||||||
|
import org.junit.Test
|
||||||
|
import org.junit.runner.RunWith
|
||||||
|
import org.libremediaconverter.model.AudioCodec
|
||||||
|
import org.libremediaconverter.model.Container
|
||||||
|
import org.libremediaconverter.model.EnginePreference
|
||||||
|
import org.libremediaconverter.model.OutputFormat
|
||||||
|
import org.libremediaconverter.model.QualityTier
|
||||||
|
import org.libremediaconverter.model.VideoCodec
|
||||||
|
import org.robolectric.RobolectricTestRunner
|
||||||
|
import org.robolectric.RuntimeEnvironment
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The seven one-line edits the settings sheet makes, and what each one leaves alone.
|
||||||
|
*
|
||||||
|
* ## Why these needed a file of their own
|
||||||
|
*
|
||||||
|
* `setPreset` was covered. The six beside it — `setContainer`, `setVideoCodec`, `setAudioCodec`,
|
||||||
|
* `applySuggestion`, `setQuality`, `setEnginePreference` — and `cancel()` had **no coverage at
|
||||||
|
* all**, which is the tell: they are reachable from the JVM suite by exactly the route `setPreset`
|
||||||
|
* already takes, and nothing had asked.
|
||||||
|
*
|
||||||
|
* ## What is actually being asserted
|
||||||
|
*
|
||||||
|
* Not "the setter sets something". Each of these copies into a nested `OutputSpec`, so the failure
|
||||||
|
* worth catching is **a setter that writes the right value into the wrong field, or that rebuilds
|
||||||
|
* the spec and silently discards the other two**. So every test here asserts the field it changed
|
||||||
|
* *and* that the rest of the spec survived — a `setContainer` implemented as
|
||||||
|
* `it.copy(spec = OutputFormat.MP4_H265.spec.copy(container = container))` would pass a test that
|
||||||
|
* only checked the container.
|
||||||
|
*
|
||||||
|
* `ConverterScreenContentTest` cannot cover this: it builds `ConverterActions` itself and never
|
||||||
|
* touches the ViewModel. That the *screen* calls these is #156's, and neither implies the other.
|
||||||
|
*/
|
||||||
|
@UnstableApi
|
||||||
|
@RunWith(RobolectricTestRunner::class)
|
||||||
|
class SettingsEditsTest {
|
||||||
|
|
||||||
|
private lateinit var app: Application
|
||||||
|
private lateinit var viewModel: ConversionViewModel
|
||||||
|
|
||||||
|
@Before
|
||||||
|
fun setUp() {
|
||||||
|
app = RuntimeEnvironment.getApplication()
|
||||||
|
installTestWorkManager(app, Data.EMPTY)
|
||||||
|
viewModel = ConversionViewModel(app)
|
||||||
|
}
|
||||||
|
|
||||||
|
@After
|
||||||
|
fun tearDown() {
|
||||||
|
ConversionDependencies.reset()
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `choosing a preset replaces the whole spec`() {
|
||||||
|
viewModel.setPreset(OutputFormat.WEBM_VP9)
|
||||||
|
|
||||||
|
assertEquals(OutputFormat.WEBM_VP9.spec, viewModel.settings.value.spec)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `changing the container leaves both codecs alone`() {
|
||||||
|
// Moved off the default spec first, and that is load-bearing rather than tidiness. The
|
||||||
|
// default IS `OutputFormat.MP4_H265.spec`, so a `setContainer` that rebuilt the spec from
|
||||||
|
// that preset instead of from the current one produced an identical answer and the
|
||||||
|
// mutation went green. Editing the codecs away from the default first is what makes
|
||||||
|
// "the other two survived" an assertion rather than a coincidence.
|
||||||
|
viewModel.setPreset(OutputFormat.WEBM_VP9)
|
||||||
|
val before = viewModel.settings.value.spec
|
||||||
|
|
||||||
|
viewModel.setContainer(Container.MKV)
|
||||||
|
|
||||||
|
val after = viewModel.settings.value.spec
|
||||||
|
assertEquals(Container.MKV, after.container)
|
||||||
|
assertEquals("the video codec is not the container's to change", before.videoCodec, after.videoCodec)
|
||||||
|
assertEquals("the audio codec is not the container's to change", before.audioCodec, after.audioCodec)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `changing the video codec leaves the container and the audio codec alone`() {
|
||||||
|
// The transposition this guards against is real: setVideoCodec and setAudioCodec take
|
||||||
|
// different enum types, but a copy(...) naming the wrong field compiles wherever the types
|
||||||
|
// happen to line up, and the picker would silently set the other one.
|
||||||
|
val before = viewModel.settings.value.spec
|
||||||
|
|
||||||
|
viewModel.setVideoCodec(VideoCodec.VP9)
|
||||||
|
|
||||||
|
val after = viewModel.settings.value.spec
|
||||||
|
assertEquals(VideoCodec.VP9, after.videoCodec)
|
||||||
|
assertEquals(before.container, after.container)
|
||||||
|
assertEquals(before.audioCodec, after.audioCodec)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `changing the audio codec leaves the container and the video codec alone`() {
|
||||||
|
val before = viewModel.settings.value.spec
|
||||||
|
|
||||||
|
viewModel.setAudioCodec(AudioCodec.OPUS)
|
||||||
|
|
||||||
|
val after = viewModel.settings.value.spec
|
||||||
|
assertEquals(AudioCodec.OPUS, after.audioCodec)
|
||||||
|
assertEquals(before.container, after.container)
|
||||||
|
assertEquals(before.videoCodec, after.videoCodec)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `applying a suggestion replaces the spec without disturbing quality or engine`() {
|
||||||
|
// A suggestion comes from ContainerCapabilities when the current spec is invalid, so it is
|
||||||
|
// a whole spec by construction. What it must not do is reset the two settings beside it.
|
||||||
|
viewModel.setQuality(QualityTier.BEST)
|
||||||
|
viewModel.setEnginePreference(EnginePreference.FORCE_SOFTWARE)
|
||||||
|
|
||||||
|
viewModel.applySuggestion(OutputFormat.MKV_H264.spec)
|
||||||
|
|
||||||
|
val settings = viewModel.settings.value
|
||||||
|
assertEquals(OutputFormat.MKV_H264.spec, settings.spec)
|
||||||
|
assertEquals(QualityTier.BEST, settings.quality)
|
||||||
|
assertEquals(EnginePreference.FORCE_SOFTWARE, settings.enginePreference)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `changing the quality leaves the spec and the engine preference alone`() {
|
||||||
|
// Both neighbours are moved off their defaults first. Asserting against AUTO -- which is
|
||||||
|
// what `ConversionSettings` starts with -- let a `setQuality` that also reset the engine
|
||||||
|
// preference to AUTO pass, because the reset and the survival looked identical.
|
||||||
|
viewModel.setPreset(OutputFormat.WEBM_VP9)
|
||||||
|
viewModel.setEnginePreference(EnginePreference.FORCE_SOFTWARE)
|
||||||
|
val before = viewModel.settings.value.spec
|
||||||
|
|
||||||
|
viewModel.setQuality(QualityTier.BEST)
|
||||||
|
|
||||||
|
val settings = viewModel.settings.value
|
||||||
|
assertEquals(QualityTier.BEST, settings.quality)
|
||||||
|
assertEquals(before, settings.spec)
|
||||||
|
assertEquals(
|
||||||
|
"quality is not the engine preference's to change",
|
||||||
|
EnginePreference.FORCE_SOFTWARE,
|
||||||
|
settings.enginePreference,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `changing the engine preference leaves the spec and the quality alone`() {
|
||||||
|
// Off the defaults for the same reason as the test above: QualityTier.FAST is the starting
|
||||||
|
// value, so asserting it here would have been satisfied by a reset as readily as by a
|
||||||
|
// survival.
|
||||||
|
viewModel.setPreset(OutputFormat.WEBM_VP9)
|
||||||
|
viewModel.setQuality(QualityTier.BEST)
|
||||||
|
val before = viewModel.settings.value.spec
|
||||||
|
|
||||||
|
viewModel.setEnginePreference(EnginePreference.FORCE_SOFTWARE)
|
||||||
|
|
||||||
|
val settings = viewModel.settings.value
|
||||||
|
assertEquals(EnginePreference.FORCE_SOFTWARE, settings.enginePreference)
|
||||||
|
assertEquals(before, settings.spec)
|
||||||
|
assertEquals(
|
||||||
|
"the engine preference is not the quality's to change",
|
||||||
|
QualityTier.BEST,
|
||||||
|
settings.quality,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `editing past every preset leaves no matching preset`() {
|
||||||
|
// `matchingPreset` is what the settings sheet reads to decide whether to show a preset as
|
||||||
|
// selected or to say "Custom". Editing one field of a preset must drop it out of the list
|
||||||
|
// rather than leaving the old one highlighted.
|
||||||
|
viewModel.setPreset(OutputFormat.MP4_H265)
|
||||||
|
assertEquals(OutputFormat.MP4_H265, viewModel.settings.value.matchingPreset)
|
||||||
|
|
||||||
|
viewModel.setAudioCodec(AudioCodec.FLAC)
|
||||||
|
|
||||||
|
assertNull(
|
||||||
|
"an edited spec is no longer any preset, and the sheet says Custom",
|
||||||
|
viewModel.settings.value.matchingPreset,
|
||||||
|
)
|
||||||
|
assertNotEquals(OutputFormat.MP4_H265.spec, viewModel.settings.value.spec)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `cancelling with no active job does nothing rather than throwing`() {
|
||||||
|
// `activeWorkId?.let(...)` -- the null side. A user can reach Cancel through a state that
|
||||||
|
// has already finished, and taking the app down for it would be worse than doing nothing.
|
||||||
|
viewModel.cancel()
|
||||||
|
|
||||||
|
assertEquals(ConversionState.Idle, viewModel.state.value)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user