Compare commits

...
Author SHA1 Message Date
JMR-devandClaude Opus 5 a507736d3d W4 (#157): the seven settings edits, and three tests that did not bite until they did
`setPreset` was covered; the six beside it and `cancel()` had no coverage at all.
That asymmetry is the tell -- they are reachable from the JVM suite by exactly
the route `setPreset` already takes, and nothing had asked.

WHAT IS 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 quietly discards
the other two. Every test asserts the field it changed AND that the rest survived.

THREE OF THEM DID NOT BITE, AND THE REASON IS WORTH KEEPING. The first run of the
mutations came back with two green:

  setContainer rebuilding from OutputFormat.MP4_H265.spec   -> GREEN
  setQuality also resetting enginePreference to AUTO        -> GREEN

Both for one mistake of mine: I asserted "the rest survived" against values that
were still at their defaults. `ConversionSettings` starts at `MP4_H265.spec`,
`QualityTier.FAST` and `EnginePreference.AUTO` -- so a mutation that RESET a
neighbouring field to its default was indistinguishable from one that left it
alone. The tests were checking a value, not a behaviour.

Fixed by moving each neighbour off its default before the call under test. A
third test had the same latent hazard -- it asserted `quality == FAST` -- and was
corrected with the others rather than left to fail later.

That is precisely the shape CLAUDE.md warns about ("five of them passing the
whole suite over a completely unguarded code path"), and it is the second time in
this wave the mutation pass has earned its place: green was not evidence.

Eight mutations, all red after the fix:

  setContainer rebuilds from a preset            | 1 test
  setQuality resets the engine preference        | 1 test
  setEnginePreference resets the quality         | 2 tests
  applySuggestion resets quality                 | 1 test
  setVideoCodec writes nothing                   | 1 test
  setAudioCodec writes nothing                   | 2 tests
  setEnginePreference writes nothing             | 2 tests
  cancel() dereferences a null activeWorkId      | 1 test

516 -> 525 tests, 87.7% -> 88.0% line, 70.4% -> 70.5% branch. Gate green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 10:46:48 -05:00
Jason Ross 70337b2c12 Merge pull request #162 from JMR-dev/test/conversion-state-mapping
W1: cut the conversion state mapping into a seam, and choose all six arms
2026-08-29 10:38:52 -05:00
JMR-devandClaude Opus 5 ddfb1dd78e W1 (#154): cut the conversion state mapping into a seam, and choose all six arms
`ConversionViewModel.observe` maps a `WorkInfo` onto a `ConversionState`. That is
the app's main UI state machine, and no test had ever chosen which arm it took.

NOT COLD CODE, WHICH IS THE POINT. `ConversionViewModel$observe$1$1` already
reported 28 covered lines and 24 covered branches: every test that drives a real
worker runs this. But a real worker only ever 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 produced. The other six ran never -- the progress
read, both sides of the retry check, a success naming no file, a failure with
nothing to say, `CANCELLED`, and `BLOCKED`.

A grep makes that look untrue: all six `WorkInfo.State` constants appear in the
JVM suite. They are in `ReattachmentTest`, driven into `Reattachment.choose` --
a *different* function encoding the same enqueued-means-retry rule. So the rule
had a test in one of its two homes, and the copy the user's screen reads had
none.

THE SEAM. `workManager` comes from `WorkManager.getInstance` in the constructor
and `observe` is private, so nothing could hand this a chosen `WorkInfo`. The
`when` is now `conversionStateFrom`, a pure function over a `ConversionUpdate`
carrying only the fields it reads -- the same shape as `JobSnapshot` beside
`Reattachment.choose`, and its KDoc gives the same reason. `outputData` stays a
`Data`, which this suite already builds with `workDataOf` everywhere; unpacking
it into five nullable strings would move the same reads without helping.

TWO THINGS DELIBERATELY LEFT OUTSIDE IT:

  - The ownership check stays at the call site. Its comment says it guards the
    file ownership the SUCCEEDED arm takes, not merely the assignment, so moving
    it inside would change what it protects.
  - The mapping takes no responsibility for the staged file. It returns the
    state; the caller reads the file off the result. That is strictly better
    than the original, where `pendingStaged = staged` happened inside one arm:
    "the state and `pendingStaged` refer to the same file or to no file" is now
    the shape of the code rather than a rule two branches have to keep.

Mutations, each killing exactly the test it should:

  | mutation                                    | red test                    |
  |---------------------------------------------|-----------------------------|
  | progress read ignored                       | reports the progress        |
  | runAttemptCount ignored -> always Waiting   | never run is simply starting|
  | runAttemptCount ignored -> never Waiting    | already run is waiting      |
  | success with no path -> empty Converted     | named no file is a failure  |
  | blank name/type no longer falls back        | blank falls back like missing|
  | blank error no longer falls back            | blank message falls back    |
  | cancellation ignores the caller's state     | lands where caller said     |
  | BLOCKED remapped                            | blocked looks like starting |

`ENQUEUED` needs both mutations and both tests: either one alone passes against a
mapping that ignores `runAttemptCount` entirely.

The extracted functions carry `@UnstableApi` rather than swallowing the marker
with `@OptIn`, per CLAUDE.md -- lint's UnsafeOptInUsageError caught their absence.
An early `@Suppress("ReturnCount")` turned out to be unnecessary and was removed
rather than left: detekt is clean without it, and the file now carries none.

502 -> 516 tests, 87.1% -> 87.7% line, 69.1% -> 70.4% branch. Gate green:
assembleDebug, testDebugUnitTest, compileDebugAndroidTestKotlin, ktlintCheck,
detekt, lintDebug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 10:23:52 -05:00
Jason Ross 348eaa2f22 Merge pull request #161 from JMR-dev/test/dedupe-user-messages
W5: one sentence per user-facing condition, not two
2026-08-29 10:14:08 -05:00
3 changed files with 567 additions and 69 deletions
@@ -6,6 +6,7 @@ import android.util.Log
import androidx.lifecycle.AndroidViewModel
import androidx.lifecycle.viewModelScope
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import androidx.work.WorkInfo
import androidx.work.WorkManager
import kotlinx.coroutines.CoroutineDispatcher
@@ -71,6 +72,119 @@ data class InputFile(
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 {
data object Idle : 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
// 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,
info.progress.getInt(ConversionWorker.KEY_PROGRESS, 0),
)
// 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
.getString(ConversionWorker.KEY_MIME_TYPE)
?.takeIf { it.isNotBlank() }
?: _settings.value.spec.mimeType,
)
}
}
// 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(
info.outputData.getString(ConversionWorker.KEY_ERROR)
?.takeIf { it.isNotBlank() }
?: ConversionWorker.GENERIC_FAILURE_MESSAGE,
)
WorkInfo.State.CANCELLED -> cancelled
WorkInfo.State.BLOCKED -> ConversionState.Converting(input, 0)
}
val next = conversionStateFrom(
ConversionUpdate(
state = info.state,
progressPercent = info.progress.getInt(ConversionWorker.KEY_PROGRESS, 0),
runAttemptCount = info.runAttemptCount,
outputData = info.outputData,
),
input = input,
cancelled = cancelled,
fallbackSpec = _settings.value.spec,
)
// 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
// "the state and `pendingStaged` refer to the same file or to no file" is now the
// shape of the code rather than a rule two branches have to keep.
if (next is ConversionState.Converted) pendingStaged = next.staged
_state.value = next
}
}
}
@@ -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)
}
}