Compare commits
6
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
dbedfb4708 | ||
|
|
c6e9a480e1 | ||
|
|
d8110d7a62 | ||
|
|
6f3966cc69 | ||
|
|
7595177e81 | ||
|
|
a507736d3d |
@@ -130,9 +130,9 @@ install for code that can never run — and on API 37 the full APK does not fit
|
|||||||
- The `model` package is excluded from `ReturnCount` and `CyclomaticComplexMethod` only. It is the
|
- The `model` package is excluded from `ReturnCount` and `CyclomaticComplexMethod` only. It is the
|
||||||
decision layer, where one branch is one documented user-visible outcome and the metric counts
|
decision layer, where one branch is one documented user-visible outcome and the metric counts
|
||||||
answers rather than complexity. Every other rule still applies there.
|
answers rather than complexity. Every other rule still applies there.
|
||||||
- **Coverage is reported, not gated** — **87.1% of lines (2025/2324), 69.1% of branches
|
- **Coverage is reported, not gated** — **88.9% of lines (2087/2348), 75.4% of branches
|
||||||
(974/1410)**, measured 2026-08-27 with `./gradlew :app:jacocoTestReport`, against 502 JVM tests
|
(1011/1340)**, measured 2026-08-29 with `./gradlew :app:jacocoTestReport`, against 546 JVM tests
|
||||||
in 71 classes.
|
in 76 classes.
|
||||||
|
|
||||||
**Every figure this file carried before 2026-08-24 was an artifact, roughly half the real one.**
|
**Every figure this file carried before 2026-08-24 was an artifact, roughly half the real one.**
|
||||||
Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo
|
Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo
|
||||||
@@ -159,6 +159,21 @@ install for code that can never run — and on API 37 the full APK does not fit
|
|||||||
chooses a branch the suite had never taken. Line coverage barely notices; branch coverage is the
|
chooses a branch the suite had never taken. Line coverage barely notices; branch coverage is the
|
||||||
whole point.
|
whole point.
|
||||||
|
|
||||||
|
Then #153's five children on 2026-08-29 — 87.1% -> 88.9% line, 69.1% -> **75.4%** branch, 502 ->
|
||||||
|
546 tests.
|
||||||
|
|
||||||
|
**That last branch figure moved for two reasons, and only one of them is new tests.** The
|
||||||
|
numerator rose 974 -> 1011; the denominator *fell* 1410 -> 1340. Both are the seam work. Pulling
|
||||||
|
a `when` out of a lambda inside a `collect` deletes the coroutine state machine's synthesized
|
||||||
|
branches around it, and what is left is a plain function whose branches a test can choose:
|
||||||
|
`ConversionViewModel$observe$1$1` went from carrying the whole mapping to 6 branches, while the
|
||||||
|
extracted `ConversionViewModelKt` covers 41 of 42 and `JoinViewModelKt` 38 of 39. So a seam is
|
||||||
|
worth more than the tests it enables — it also stops the measurement counting scaffolding.
|
||||||
|
|
||||||
|
Be careful quoting a branch move on its own for that reason. A percentage that rises because the
|
||||||
|
denominator shrank is not the same claim as one that rises because more branches are tested, and
|
||||||
|
this entry has a history of explaining its own numbers wrongly.
|
||||||
|
|
||||||
And **re-measure before quoting**: this entry was once written quoting 81.4%, measured four hours
|
And **re-measure before quoting**: this entry was once written quoting 81.4%, measured four hours
|
||||||
earlier, and was already three points stale by the time it was ready to merge.
|
earlier, and was already three points stale by the time it was ready to merge.
|
||||||
- **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit
|
- **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit
|
||||||
@@ -182,6 +197,32 @@ install for code that can never run — and on API 37 the full APK does not fit
|
|||||||
|
|
||||||
- `kotlin.code.style=official`. Gradle stays Kotlin DSL.
|
- `kotlin.code.style=official`. Gradle stays Kotlin DSL.
|
||||||
|
|
||||||
|
- **A stacked PR does not merge with `gh pr merge`, and `MERGED` is not proof it reached `main`.**
|
||||||
|
Two separate traps, both measured on 2026-08-27 while landing #144-#151.
|
||||||
|
|
||||||
|
`gh pr merge` uses the GraphQL mutation, which refuses a stacked PR outright: *"This pull request
|
||||||
|
is part of a stack and must be merged using the asynchronous merge REST API."* So does
|
||||||
|
`PUT .../pulls/{n}/merge`. The one that works is
|
||||||
|
`gh api -X PUT repos/OWNER/REPO/pulls/N/merge-async -f merge_method=merge`, which returns a uuid
|
||||||
|
to poll at `.../merge-async/{uuid}` until `status` is `merged` or `failed`.
|
||||||
|
|
||||||
|
**The second trap is worse, because nothing looks wrong.** GitHub retargets a stacked PR's base
|
||||||
|
to `main` when the PR below it merges, but *asynchronously*. Merge a stack faster than that
|
||||||
|
settles — five PRs about thirty seconds apart, in the case that found this — and each one merges
|
||||||
|
into its own base branch, which has itself already been merged and left behind. Every call
|
||||||
|
returns `status: merged` and every one is true. `gh pr list --state open` comes back empty, every
|
||||||
|
PR shows `MERGED`, and **none of the content is on `main`**.
|
||||||
|
|
||||||
|
What caught it was a coverage re-measure reading two points lower than the same tree had measured
|
||||||
|
an hour earlier; a fresh `git pull` changed nothing, which is what turned it into a question.
|
||||||
|
`git merge-base --is-ancestor <merge-sha> origin/main` answers it in one line. Do that after
|
||||||
|
merging a stack, or merge one at a time and re-read `baseRefName` between. #160 is what the
|
||||||
|
recovery cost.
|
||||||
|
|
||||||
|
The auto-retarget belongs to the stacking feature specifically. A PR opened with a plain
|
||||||
|
`gh pr create --base some-branch` does **not** retarget when that branch merges — it is left
|
||||||
|
pointing at a dead base and has to be moved by hand.
|
||||||
|
|
||||||
- **File one-off issues with `tools/github/file-issue.sh`, not `gh issue create`.** `gh issue
|
- **File one-off issues with `tools/github/file-issue.sh`, not `gh issue create`.** `gh issue
|
||||||
create` does not touch the project board, so the issue exists, carries its labels, and is
|
create` does not touch the project board, so the issue exists, carries its labels, and is
|
||||||
invisible in the Kanban — indistinguishable from never having been filed. Measured 2026-08-24:
|
invisible in the Kanban — indistinguishable from never having been filed. Measured 2026-08-24:
|
||||||
|
|||||||
@@ -94,24 +94,61 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
|||||||
state = state,
|
state = state,
|
||||||
settings = settings,
|
settings = settings,
|
||||||
validation = validation,
|
validation = validation,
|
||||||
actions = ConverterActions(
|
actions = converterActions(
|
||||||
|
viewModel = viewModel,
|
||||||
onPickInput = { pickInput.launch(arrayOf("*/*")) },
|
onPickInput = { pickInput.launch(arrayOf("*/*")) },
|
||||||
onPreset = viewModel::setPreset,
|
|
||||||
onContainer = viewModel::setContainer,
|
|
||||||
onVideoCodec = viewModel::setVideoCodec,
|
|
||||||
onAudioCodec = viewModel::setAudioCodec,
|
|
||||||
onSuggestion = viewModel::applySuggestion,
|
|
||||||
onQuality = viewModel::setQuality,
|
|
||||||
onEnginePreference = viewModel::setEnginePreference,
|
|
||||||
onConvert = { requestNotifications.launch(Manifest.permission.POST_NOTIFICATIONS) },
|
onConvert = { requestNotifications.launch(Manifest.permission.POST_NOTIFICATIONS) },
|
||||||
onCancel = viewModel::cancel,
|
|
||||||
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
|
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
|
||||||
onReset = viewModel::reset,
|
|
||||||
),
|
),
|
||||||
modifier = modifier,
|
modifier = modifier,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Which of the ViewModel's methods each affordance on the screen calls.
|
||||||
|
*
|
||||||
|
* ## Why this is a function rather than an argument list
|
||||||
|
*
|
||||||
|
* It was an argument list, inside [ConverterScreen], which no test reached: `ConverterScreenContent`
|
||||||
|
* builds its own [ConverterActions], so every test in the suite drove the stateless inner and none
|
||||||
|
* of them ever saw the wiring.
|
||||||
|
*
|
||||||
|
* Most of the list is safe without a test, and saying so is more useful than pretending otherwise:
|
||||||
|
* `onContainer`, `onVideoCodec`, `onAudioCodec`, `onPreset`, `onSuggestion`, `onQuality` and
|
||||||
|
* `onEnginePreference` each take a distinct type, so binding one to another's setter does not
|
||||||
|
* compile. Verified rather than assumed — swapping `onVideoCodec` and `onAudioCodec` fails with
|
||||||
|
* *"Inapplicable candidate(s): fun setAudioCodec(codec: AudioCodec)"*.
|
||||||
|
*
|
||||||
|
* **[ConverterActions.onCancel] and [ConverterActions.onReset] are the exception.** Both are
|
||||||
|
* `() -> Unit`, so swapping them compiles silently — also verified — and ships a Cancel button that
|
||||||
|
* throws the conversion away and a Start-over button that leaves it on screen. That pair is what
|
||||||
|
* `ConverterWiringTest` exists for.
|
||||||
|
*
|
||||||
|
* The three launcher-backed actions stay parameters: they need an `ActivityResultLauncher`, which
|
||||||
|
* is the part that genuinely needs the composition, and keeping them out means the rest can be
|
||||||
|
* checked without one.
|
||||||
|
*/
|
||||||
|
@UnstableApi
|
||||||
|
internal fun converterActions(
|
||||||
|
viewModel: ConversionViewModel,
|
||||||
|
onPickInput: () -> Unit,
|
||||||
|
onConvert: () -> Unit,
|
||||||
|
onSave: (suggestedName: String) -> Unit,
|
||||||
|
): ConverterActions = ConverterActions(
|
||||||
|
onPickInput = onPickInput,
|
||||||
|
onPreset = viewModel::setPreset,
|
||||||
|
onContainer = viewModel::setContainer,
|
||||||
|
onVideoCodec = viewModel::setVideoCodec,
|
||||||
|
onAudioCodec = viewModel::setAudioCodec,
|
||||||
|
onSuggestion = viewModel::applySuggestion,
|
||||||
|
onQuality = viewModel::setQuality,
|
||||||
|
onEnginePreference = viewModel::setEnginePreference,
|
||||||
|
onConvert = onConvert,
|
||||||
|
onCancel = viewModel::cancel,
|
||||||
|
onSave = onSave,
|
||||||
|
onReset = viewModel::reset,
|
||||||
|
)
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Everything [ConverterScreenContent] can ask for, in one value.
|
* Everything [ConverterScreenContent] can ask for, in one value.
|
||||||
*
|
*
|
||||||
|
|||||||
@@ -56,17 +56,38 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
|||||||
|
|
||||||
JoinScreenContent(
|
JoinScreenContent(
|
||||||
state = state,
|
state = state,
|
||||||
actions = JoinActions(
|
actions = joinActions(
|
||||||
|
viewModel = viewModel,
|
||||||
onPickInputs = { pickInputs.launch(arrayOf("video/*")) },
|
onPickInputs = { pickInputs.launch(arrayOf("video/*")) },
|
||||||
onJoin = viewModel::join,
|
|
||||||
onCancel = viewModel::cancel,
|
|
||||||
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
|
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
|
||||||
onReset = viewModel::reset,
|
|
||||||
),
|
),
|
||||||
modifier = modifier,
|
modifier = modifier,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Which of the ViewModel's methods each affordance on the join screen calls.
|
||||||
|
*
|
||||||
|
* The join-side twin of `converterActions`, and the transposition risk here is worse: **three**
|
||||||
|
* `() -> Unit` bindings rather than two. `onJoin`, `onCancel` and `onReset` are mutually
|
||||||
|
* interchangeable as far as the compiler is concerned, so a Join button that cancels, or a Cancel
|
||||||
|
* button that starts the job, is a swap nothing but a test would catch.
|
||||||
|
*
|
||||||
|
* See `converterActions` for why the launcher-backed actions stay parameters.
|
||||||
|
*/
|
||||||
|
@UnstableApi
|
||||||
|
internal fun joinActions(
|
||||||
|
viewModel: JoinViewModel,
|
||||||
|
onPickInputs: () -> Unit,
|
||||||
|
onSave: (suggestedName: String) -> Unit,
|
||||||
|
): JoinActions = JoinActions(
|
||||||
|
onPickInputs = onPickInputs,
|
||||||
|
onJoin = viewModel::join,
|
||||||
|
onCancel = viewModel::cancel,
|
||||||
|
onSave = onSave,
|
||||||
|
onReset = viewModel::reset,
|
||||||
|
)
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Everything [JoinScreenContent] can ask for, in one value.
|
* Everything [JoinScreenContent] can ask for, in one value.
|
||||||
*
|
*
|
||||||
|
|||||||
@@ -0,0 +1,241 @@
|
|||||||
|
package org.libremediaconverter.convert
|
||||||
|
|
||||||
|
import android.app.Application
|
||||||
|
import android.net.Uri
|
||||||
|
import androidx.media3.common.util.UnstableApi
|
||||||
|
import androidx.work.Data
|
||||||
|
import androidx.work.workDataOf
|
||||||
|
import org.junit.After
|
||||||
|
import org.junit.Assert.assertEquals
|
||||||
|
import org.junit.Assert.assertNotEquals
|
||||||
|
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.joinActions
|
||||||
|
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.libremediaconverter.work.ConcatWorker
|
||||||
|
import org.robolectric.RobolectricTestRunner
|
||||||
|
import org.robolectric.RuntimeEnvironment
|
||||||
|
|
||||||
|
/**
|
||||||
|
* That each affordance is wired to the ViewModel method it is named after.
|
||||||
|
*
|
||||||
|
* ## What this covers that no other test can
|
||||||
|
*
|
||||||
|
* `ConverterScreenContentTest`, `ConverterStateAffordancesTest` and `JoinScreenContentTest` all
|
||||||
|
* drive the **stateless** content composables, which build their own `ConverterActions`. So the
|
||||||
|
* wiring — the list of `viewModel::` references the stateful outer hands down — was seen by nothing
|
||||||
|
* in the suite.
|
||||||
|
*
|
||||||
|
* ## The hazard is narrower than "seventeen bindings", and this says so
|
||||||
|
*
|
||||||
|
* #156 was filed claiming a transposition of any two bindings would survive the suite. That is not
|
||||||
|
* true, and it was worth checking rather than testing on the assumption:
|
||||||
|
*
|
||||||
|
* | swap | result |
|
||||||
|
* |---|---|
|
||||||
|
* | `onVideoCodec` ↔ `onAudioCodec` | **rejected by the compiler** |
|
||||||
|
* | `onCancel` ↔ `onReset` | **compiles** |
|
||||||
|
*
|
||||||
|
* Every typed binding — container, both codecs, preset, suggestion, quality, engine preference —
|
||||||
|
* takes a distinct parameter type, so the compiler is already the test. Writing assertions for
|
||||||
|
* those would be theatre.
|
||||||
|
*
|
||||||
|
* **The `() -> Unit` bindings are the real gap**, because they are interchangeable to the compiler:
|
||||||
|
* two on the converter screen (`onCancel`, `onReset`) and three on the join screen (`onJoin`,
|
||||||
|
* `onCancel`, `onReset`). A Cancel that discards the finished file, or a Join that cancels, is a
|
||||||
|
* one-character mistake that ships.
|
||||||
|
*
|
||||||
|
* ## How they are told apart
|
||||||
|
*
|
||||||
|
* By effect, not by a recording double. `reset()` sets the state to `Idle`; `cancel()` with no
|
||||||
|
* active job leaves it alone (`ConversionViewModel.cancel` is `activeWorkId?.let(...)`, and
|
||||||
|
* `SettingsEditsTest` pins that). Driving each from a non-`Idle` state is therefore enough to say
|
||||||
|
* which one ran.
|
||||||
|
*/
|
||||||
|
@UnstableApi
|
||||||
|
@RunWith(RobolectricTestRunner::class)
|
||||||
|
class ScreenWiringTest {
|
||||||
|
|
||||||
|
private lateinit var app: Application
|
||||||
|
|
||||||
|
@Before
|
||||||
|
fun setUp() {
|
||||||
|
app = RuntimeEnvironment.getApplication()
|
||||||
|
ConversionDependencies.publisher = { RecordingPublisher(app) }
|
||||||
|
ConversionDependencies.probe = { _, _ -> org.libremediaconverter.model.InputProbe() }
|
||||||
|
}
|
||||||
|
|
||||||
|
@After
|
||||||
|
fun tearDown() {
|
||||||
|
ConversionDependencies.reset()
|
||||||
|
}
|
||||||
|
|
||||||
|
// --- the converter screen ----------------------------------------------
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `Start over resets, and Cancel does not`() {
|
||||||
|
// The transposition that compiles. If onReset were bound to cancel, this stays on Ready.
|
||||||
|
installTestWorkManager(app, Data.EMPTY)
|
||||||
|
val pick = ParkedPickDispatcher()
|
||||||
|
val viewModel = ConversionViewModel(app, pickDispatcher = pick)
|
||||||
|
val actions = converterActions(viewModel, onPickInput = {}, onConvert = {}, onSave = {})
|
||||||
|
viewModel.onInputPicked(INPUT_URI)
|
||||||
|
pick.runAll()
|
||||||
|
assertNotEquals(
|
||||||
|
"the fixture needs a non-Idle state or neither action is observable",
|
||||||
|
ConversionState.Idle,
|
||||||
|
viewModel.state.value,
|
||||||
|
)
|
||||||
|
|
||||||
|
actions.onReset()
|
||||||
|
|
||||||
|
assertEquals(ConversionState.Idle, viewModel.state.value)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `Cancel leaves the picked file on screen`() {
|
||||||
|
// The other half. Without it, a wiring with BOTH actions bound to reset passes the test
|
||||||
|
// above -- and that is exactly what a copy-paste of the wrong line produces.
|
||||||
|
installTestWorkManager(app, Data.EMPTY)
|
||||||
|
val pick = ParkedPickDispatcher()
|
||||||
|
val viewModel = ConversionViewModel(app, pickDispatcher = pick)
|
||||||
|
val actions = converterActions(viewModel, onPickInput = {}, onConvert = {}, onSave = {})
|
||||||
|
viewModel.onInputPicked(INPUT_URI)
|
||||||
|
pick.runAll()
|
||||||
|
val before = viewModel.state.value
|
||||||
|
|
||||||
|
actions.onCancel()
|
||||||
|
|
||||||
|
assertEquals(
|
||||||
|
"Cancel must not throw away the pick the way Start over does",
|
||||||
|
before,
|
||||||
|
viewModel.state.value,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `each settings affordance reaches the setting it is named after`() {
|
||||||
|
// The typed bindings. The compiler already rejects a transposition among these, so this is
|
||||||
|
// not that assertion -- it is the cheaper one that each is bound to *something*, and that a
|
||||||
|
// binding dropped to `{}` during an edit would be caught.
|
||||||
|
installTestWorkManager(app, Data.EMPTY)
|
||||||
|
val pick = ParkedPickDispatcher()
|
||||||
|
val viewModel = ConversionViewModel(app, pickDispatcher = pick)
|
||||||
|
val actions = converterActions(viewModel, onPickInput = {}, onConvert = {}, onSave = {})
|
||||||
|
|
||||||
|
actions.onPreset(OutputFormat.WEBM_VP9)
|
||||||
|
assertEquals(OutputFormat.WEBM_VP9.spec, viewModel.settings.value.spec)
|
||||||
|
|
||||||
|
actions.onContainer(Container.MKV)
|
||||||
|
assertEquals(Container.MKV, viewModel.settings.value.spec.container)
|
||||||
|
|
||||||
|
actions.onVideoCodec(VideoCodec.H264)
|
||||||
|
assertEquals(VideoCodec.H264, viewModel.settings.value.spec.videoCodec)
|
||||||
|
|
||||||
|
actions.onAudioCodec(AudioCodec.FLAC)
|
||||||
|
assertEquals(AudioCodec.FLAC, viewModel.settings.value.spec.audioCodec)
|
||||||
|
|
||||||
|
actions.onQuality(QualityTier.BEST)
|
||||||
|
assertEquals(QualityTier.BEST, viewModel.settings.value.quality)
|
||||||
|
|
||||||
|
actions.onEnginePreference(EnginePreference.FORCE_SOFTWARE)
|
||||||
|
assertEquals(EnginePreference.FORCE_SOFTWARE, viewModel.settings.value.enginePreference)
|
||||||
|
|
||||||
|
actions.onSuggestion(OutputFormat.MP4_H264.spec)
|
||||||
|
assertEquals(OutputFormat.MP4_H264.spec, viewModel.settings.value.spec)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `the launcher-backed actions are the ones the screen supplies`() {
|
||||||
|
// Not wired to the ViewModel at all, deliberately -- they need an ActivityResultLauncher.
|
||||||
|
// Asserted so that a later edit routing one of them at the ViewModel is noticed.
|
||||||
|
installTestWorkManager(app, Data.EMPTY)
|
||||||
|
val pick = ParkedPickDispatcher()
|
||||||
|
val viewModel = ConversionViewModel(app, pickDispatcher = pick)
|
||||||
|
val called = mutableListOf<String>()
|
||||||
|
val actions = converterActions(
|
||||||
|
viewModel,
|
||||||
|
onPickInput = { called += "pick" },
|
||||||
|
onConvert = { called += "convert" },
|
||||||
|
onSave = { called += "save:$it" },
|
||||||
|
)
|
||||||
|
|
||||||
|
actions.onPickInput()
|
||||||
|
actions.onConvert()
|
||||||
|
actions.onSave("holiday.mp4")
|
||||||
|
|
||||||
|
assertEquals(listOf("pick", "convert", "save:holiday.mp4"), called)
|
||||||
|
}
|
||||||
|
|
||||||
|
// --- the join screen, where three are interchangeable -------------------
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `Start over resets the join, and Cancel does not`() {
|
||||||
|
installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
|
||||||
|
val pick = ParkedPickDispatcher()
|
||||||
|
val viewModel = JoinViewModel(app, pickDispatcher = pick)
|
||||||
|
val actions = joinActions(viewModel, onPickInputs = {}, onSave = {})
|
||||||
|
viewModel.onInputsPicked(TWO_INPUTS)
|
||||||
|
pick.runAll()
|
||||||
|
assertTrue(
|
||||||
|
"the fixture needs a non-Idle state: ${viewModel.state.value}",
|
||||||
|
viewModel.state.value !is JoinState.Idle,
|
||||||
|
)
|
||||||
|
|
||||||
|
actions.onReset()
|
||||||
|
|
||||||
|
assertEquals(JoinState.Idle, viewModel.state.value)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `Cancel leaves the picked files on screen`() {
|
||||||
|
installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
|
||||||
|
val pick = ParkedPickDispatcher()
|
||||||
|
val viewModel = JoinViewModel(app, pickDispatcher = pick)
|
||||||
|
val actions = joinActions(viewModel, onPickInputs = {}, onSave = {})
|
||||||
|
viewModel.onInputsPicked(TWO_INPUTS)
|
||||||
|
pick.runAll()
|
||||||
|
val before = viewModel.state.value
|
||||||
|
|
||||||
|
actions.onCancel()
|
||||||
|
|
||||||
|
assertEquals(before, viewModel.state.value)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `Join starts the job rather than cancelling or resetting it`() {
|
||||||
|
// The third of the join screen's interchangeable trio, and the one whose transposition is
|
||||||
|
// worst: a Join button bound to cancel does nothing at all, which reads as a dead button.
|
||||||
|
installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
|
||||||
|
val pick = ParkedPickDispatcher()
|
||||||
|
val viewModel = JoinViewModel(app, pickDispatcher = pick)
|
||||||
|
val actions = joinActions(viewModel, onPickInputs = {}, onSave = {})
|
||||||
|
viewModel.onInputsPicked(TWO_INPUTS)
|
||||||
|
pick.runAll()
|
||||||
|
|
||||||
|
actions.onJoin()
|
||||||
|
|
||||||
|
assertTrue(
|
||||||
|
"Join must leave Ready for a running state, not sit still and not go Idle: " +
|
||||||
|
"${viewModel.state.value}",
|
||||||
|
viewModel.state.value is JoinState.Joining || viewModel.state.value is JoinState.Joined,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
private companion object {
|
||||||
|
val INPUT_URI: Uri = Uri.parse("content://test/holiday.mov")
|
||||||
|
val TWO_INPUTS = listOf(
|
||||||
|
Uri.parse("content://test/a.mp4"),
|
||||||
|
Uri.parse("content://test/b.mp4"),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -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