Compare commits
7
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
46ad95350b | ||
|
|
e968deb5a2 | ||
|
|
3d55004286 | ||
|
|
e06b0826a0 | ||
|
|
7b578c1ccf | ||
|
|
9e7f80feaa | ||
|
|
3d51fefeff |
@@ -64,16 +64,28 @@ of the first one that fails.
|
||||
on this machine (below), so without it an androidTest compile error is not discovered until CI.
|
||||
ktlint and detekt also cover the `test`/`androidTest` source sets that `lintDebug` skips.
|
||||
|
||||
## Instrumented tests do not run locally
|
||||
## Instrumented tests: where they actually run
|
||||
|
||||
Two independent reasons, so do not spend time on either:
|
||||
This section said the opposite until 2026-08-24, and both of its claims had been false for two
|
||||
days. Read it as the current answer, and see the git history if you need the old one.
|
||||
|
||||
- **Emulators segfault on this host.** qemu dies on every AVD. Instrumented tests run on CI or on
|
||||
the physical Pixel, never in a local emulator.
|
||||
- **The API 37 image is broken.** `android-37.0` crash-loops surfaceflinger inside its own gralloc
|
||||
mapper, so every test fails there regardless of this app. `docs/api-37-emulator-crash.md` records
|
||||
the evidence and the ruled-out fixes; CI's matrix therefore stops at API 36 even though targetSdk
|
||||
is 37. **API 37 needs a manual check on the Pixel 10 Pro XL before each release.**
|
||||
- **Local emulators work, for API 33-36.** `tools/local-emulator/run-e2e.sh` runs them on this
|
||||
host. The segfault that made this look impossible was not a broken machine: SwiftShader's Reactor
|
||||
JIT writes generated shader code onto the heap and executes it, Fedora's SELinux policy denies
|
||||
`execheap`, and qemu dies. Choosing a different renderer avoids it entirely — `-gpu host`,
|
||||
`angle_indirect` and `swangle_indirect` all boot, while `auto`, `off`, `guest` and
|
||||
`swiftshader_indirect` do not. `docs/local-emulator.md` has the evidence and the per-API renderer
|
||||
table.
|
||||
- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. Two Media3 hardware-transcode
|
||||
tests fail inside the emulator's own `c2.goldfish.h264.decoder` rather than on anything this app
|
||||
does; they carry `@FailsOnEmulatorApi37` and run in a separate `continue-on-error` job,
|
||||
`E2E API 37 Media3 hardware transcode (advisory)`. The gating leg runs the other 55.
|
||||
**That advisory job is red on every PR, by design** — do not read it as your change breaking
|
||||
something, and do not read a green run as evidence those two tests pass.
|
||||
`docs/api-37-emulator-crash.md` has the measurements.
|
||||
|
||||
Still true, and the reason the advisory job is not simply deleted: **API 37 needs a manual check on
|
||||
the Pixel 10 Pro XL before each release.** The advisory pair is the one thing CI cannot answer for.
|
||||
|
||||
On a device or emulator, build only the ABI it can execute:
|
||||
|
||||
@@ -96,12 +108,26 @@ 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
|
||||
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.
|
||||
- **Coverage is reported, not gated** — **29.8% of lines (629/2113), 28.7% of branches**, measured
|
||||
on `main` 2026-08-23 with `./gradlew :app:jacocoTestReport`. A floor needs a baseline that has
|
||||
settled first, and this one has not: the figure **fell** from the ~31% recorded earlier even
|
||||
though the JVM suite went from 11 test files to 43. Main source grew 4,114 -> 5,715 lines over
|
||||
the same period, so the denominator outran the numerator. Re-measure before quoting it; do not
|
||||
assume more tests means a higher percentage here.
|
||||
- **Coverage is reported, not gated** — **69.2% of lines (1519/2194), 53.2% of branches**,
|
||||
measured 2026-08-24 with `./gradlew :app:jacocoTestReport`.
|
||||
|
||||
**Every figure this file carried before that date was an artifact, roughly half the real one.**
|
||||
Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo
|
||||
skips no-location classes by default, and nothing told it otherwise — so **not one Robolectric
|
||||
test counted**, and Robolectric is what exercises the framework edge here. The
|
||||
`isIncludeNoLocationClasses` block in `app/build.gradle.kts` is what fixes it; **do not delete
|
||||
it as stray config**, and re-run the numbers if you ever touch it. Same commit, same 335 tests:
|
||||
29.7% -> 69.2% with that block alone.
|
||||
|
||||
The old entry also explained the wrong thing. It said coverage **fell** as the suite grew from 11
|
||||
test files to 43 because "the denominator outran the numerator" on framework-edge code "the JVM
|
||||
cannot reach". The JVM reaches that code fine. What actually happened is that the new tests were
|
||||
disproportionately Robolectric, so each one added denominator and no numerator — the measurement
|
||||
was punishing exactly the tests that were hardest to write.
|
||||
|
||||
Two things still hold. A floor needs a baseline that has settled, and this one has now moved by
|
||||
39 points in a single build change, so it has not. And **re-measure before quoting** — that
|
||||
instruction is the only reason this was caught.
|
||||
- **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit
|
||||
tests before it counts as done. If it is e2e testable, it gets e2e tests. Both clauses apply —
|
||||
a change that is both needs both.
|
||||
@@ -112,9 +138,9 @@ install for code that can never run — and on API 37 the full APK does not fit
|
||||
documents the reasoning — turns "needs a device" into "a pure function plus a thin edge".
|
||||
Robolectric is in the JVM source set, `compose-ui-test-junit4` with it, so Compose screens are
|
||||
unit testable too. Reach for the seam before concluding something cannot be unit tested.
|
||||
- **E2E is runnable locally now.** `tools/local-emulator/run-e2e.sh` runs API 33-36 on this
|
||||
machine; see `docs/local-emulator.md`. That was believed impossible until the SELinux/renderer
|
||||
cause was found, and it is what makes the e2e half of this norm enforceable.
|
||||
- **E2E is runnable locally**, API 33-36, via `tools/local-emulator/run-e2e.sh` — see
|
||||
"Instrumented tests: where they actually run" above. That was believed impossible until the
|
||||
SELinux/renderer cause was found, and it is what makes the e2e half of this norm enforceable.
|
||||
- **A test has to bite.** Revert the line it covers, confirm it goes red, restore. A review of
|
||||
this codebase ran 46 mutations against a 257-test suite and **9 were vacuous** — five of them
|
||||
passing the whole suite over a completely unguarded code path. Green is not evidence.
|
||||
|
||||
@@ -188,6 +188,30 @@ detekt {
|
||||
}
|
||||
|
||||
// Pin the coverage agent rather than inheriting whatever Gradle bundles.
|
||||
// Robolectric loads every class it touches through its own sandbox classloader, and those
|
||||
// classes arrive with no source location. JaCoCo skips no-location classes by default, so
|
||||
// without this block **not one Robolectric test counts** -- and Robolectric is what exercises
|
||||
// the framework edge here: the workers, the publisher, both ViewModels, every Compose screen.
|
||||
//
|
||||
// Measured on e06b082, same 335 tests, same 0 failures, only this block added:
|
||||
//
|
||||
// LINE 29.7% -> 69.2% OutputPublisher 0.0% -> 97.5%
|
||||
// BRANCH 29.8% -> 53.2% ConversionViewModel 0.0% -> 85.4%
|
||||
//
|
||||
// The discriminator, if this ever looks like superstition: inside ConverterScreenKt, `describe`
|
||||
// is the one non-Composable and is exercised by a plain JVM test -- it reported 8/8 covered while
|
||||
// every @Composable in the same class reported 0, including ones whose mutations demonstrably
|
||||
// failed the build when reverted.
|
||||
//
|
||||
// `excludes` is not optional. Without it JaCoCo walks JDK-internal classes that Robolectric has
|
||||
// no location for either, and the test JVM dies rather than reporting a number.
|
||||
tasks.withType<Test>().configureEach {
|
||||
extensions.configure<JacocoTaskExtension> {
|
||||
isIncludeNoLocationClasses = true
|
||||
excludes = listOf("jdk.internal.*")
|
||||
}
|
||||
}
|
||||
|
||||
jacoco {
|
||||
toolVersion = libs.versions.jacoco.get()
|
||||
}
|
||||
|
||||
@@ -87,6 +87,89 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
ActivityResultContracts.RequestPermission(),
|
||||
) { viewModel.convert() }
|
||||
|
||||
ConverterScreenContent(
|
||||
state = state,
|
||||
settings = settings,
|
||||
validation = validation,
|
||||
actions = ConverterActions(
|
||||
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) },
|
||||
onCancel = viewModel::cancel,
|
||||
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
|
||||
onReset = viewModel::reset,
|
||||
),
|
||||
modifier = modifier,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Everything [ConverterScreenContent] can ask for, in one value.
|
||||
*
|
||||
* A holder rather than twelve parameters because detekt's `LongParameterList` sits at its default
|
||||
* threshold of six and `config/detekt/detekt.yml` does not relax it for `@Composable` the way it
|
||||
* relaxes `LongMethod` and `CyclomaticComplexMethod` -- `AdvancedPicker` already sits exactly on
|
||||
* that threshold. The rule exempts data classes, so the callbacks travel together.
|
||||
*
|
||||
* In production every one of these is a launcher or a `ConversionViewModel` call. Naming them here
|
||||
* instead of handing the content a ViewModel is the whole point of the seam: a test can render a
|
||||
* [ConversionState] no ViewModel can be driven into, since `Waiting` needs a denied foreground
|
||||
* start and `Converted` needs a worker run that has already succeeded.
|
||||
*/
|
||||
internal data class ConverterActions(
|
||||
/** Open the document picker. The `Idle` and `Ready` branches both offer it. */
|
||||
val onPickInput: () -> Unit,
|
||||
val onPreset: (OutputFormat) -> Unit,
|
||||
val onContainer: (Container) -> Unit,
|
||||
val onVideoCodec: (VideoCodec) -> Unit,
|
||||
val onAudioCodec: (AudioCodec) -> Unit,
|
||||
val onSuggestion: (OutputSpec) -> Unit,
|
||||
val onQuality: (QualityTier) -> Unit,
|
||||
val onEnginePreference: (EnginePreference) -> Unit,
|
||||
/**
|
||||
* Start the job. It asks for the notification permission first, which is why the screen never
|
||||
* calls `convert` directly -- the launcher's result callback does, whichever way it went.
|
||||
*/
|
||||
val onConvert: () -> Unit,
|
||||
val onCancel: () -> Unit,
|
||||
/**
|
||||
* Open the save dialog for the finished output.
|
||||
*
|
||||
* Takes the suggested name rather than reading it back off the state, because the name comes
|
||||
* from the job -- see `ConversionWorker.KEY_SUGGESTED_NAME` -- and the branch that renders the
|
||||
* button is the only place that has it.
|
||||
*/
|
||||
val onSave: (suggestedName: String) -> Unit,
|
||||
val onReset: () -> Unit,
|
||||
)
|
||||
|
||||
/**
|
||||
* The converter screen, with its state handed in.
|
||||
*
|
||||
* Split from [ConverterScreen] so that state has somewhere to come from other than a live
|
||||
* `ConversionViewModel`. Driving the screen through a real one needs a `WorkManager` and a media
|
||||
* probe in the constructor, and even then two of the six states are unreachable: `Waiting` follows
|
||||
* a denied foreground start and `Converted` follows a completed worker.
|
||||
*
|
||||
* `internal` rather than private, because `src/test` is a friend of `main` and this is what the
|
||||
* state tests compose. The leaves below stay exactly where they were -- this function is a move,
|
||||
* not a redesign, and the tests that already pin those leaves are what says so.
|
||||
*/
|
||||
@UnstableApi
|
||||
@Composable
|
||||
internal fun ConverterScreenContent(
|
||||
state: ConversionState,
|
||||
settings: ConversionSettings,
|
||||
validation: Validation,
|
||||
actions: ConverterActions,
|
||||
modifier: Modifier = Modifier,
|
||||
) {
|
||||
Column(
|
||||
modifier = modifier
|
||||
.fillMaxSize()
|
||||
@@ -115,7 +198,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
modifier = Modifier.padding(bottom = 16.dp),
|
||||
)
|
||||
Button(
|
||||
onClick = { pickInput.launch(arrayOf("*/*")) },
|
||||
onClick = actions.onPickInput,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
@@ -132,21 +215,19 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
|
||||
is ConversionState.Ready -> {
|
||||
FileCard(s.input)
|
||||
FormatPicker(settings.matchingPreset, viewModel::setPreset)
|
||||
FormatPicker(settings.matchingPreset, actions.onPreset)
|
||||
AdvancedPicker(
|
||||
spec = settings.spec,
|
||||
validation = validation,
|
||||
onContainer = viewModel::setContainer,
|
||||
onVideoCodec = viewModel::setVideoCodec,
|
||||
onAudioCodec = viewModel::setAudioCodec,
|
||||
onSuggestion = viewModel::applySuggestion,
|
||||
onContainer = actions.onContainer,
|
||||
onVideoCodec = actions.onVideoCodec,
|
||||
onAudioCodec = actions.onAudioCodec,
|
||||
onSuggestion = actions.onSuggestion,
|
||||
)
|
||||
QualityPicker(settings.quality, viewModel::setQuality)
|
||||
EnginePicker(settings.enginePreference, viewModel::setEnginePreference)
|
||||
QualityPicker(settings.quality, actions.onQuality)
|
||||
EnginePicker(settings.enginePreference, actions.onEnginePreference)
|
||||
Button(
|
||||
onClick = {
|
||||
requestNotifications.launch(Manifest.permission.POST_NOTIFICATIONS)
|
||||
},
|
||||
onClick = actions.onConvert,
|
||||
// The Advanced picker lets an impossible combination be selected on
|
||||
// purpose, so this is what stops it from being run.
|
||||
enabled = validation.isValid,
|
||||
@@ -156,7 +237,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
.testTag(TestTags.Converter.CONVERT),
|
||||
) { Text("Convert") }
|
||||
OutlinedButton(
|
||||
onClick = { pickInput.launch(arrayOf("*/*")) },
|
||||
onClick = actions.onPickInput,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.testTag(TestTags.Converter.CHOOSE_DIFFERENT_FILE),
|
||||
@@ -173,7 +254,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
.testTag(TestTags.Converter.PROGRESS),
|
||||
)
|
||||
OutlinedButton(
|
||||
onClick = viewModel::cancel,
|
||||
onClick = actions.onCancel,
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
|
||||
) { Text("Cancel") }
|
||||
}
|
||||
@@ -192,7 +273,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
style = MaterialTheme.typography.bodyMedium,
|
||||
)
|
||||
OutlinedButton(
|
||||
onClick = viewModel::cancel,
|
||||
onClick = actions.onCancel,
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
|
||||
) { Text("Cancel") }
|
||||
}
|
||||
@@ -211,14 +292,14 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
AssistChip(onClick = {}, label = { Text(s.routeReason) })
|
||||
}
|
||||
Button(
|
||||
onClick = { chooseDestination.launch(s.suggestedName) },
|
||||
onClick = { actions.onSave(s.suggestedName) },
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.SAVE_FILE),
|
||||
) { Text("Save file") }
|
||||
OutlinedButton(
|
||||
onClick = viewModel::reset,
|
||||
onClick = actions.onReset,
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.START_OVER),
|
||||
) { Text("Start over") }
|
||||
}
|
||||
@@ -226,7 +307,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
is ConversionState.Saved -> {
|
||||
Text("Saved ${s.displayName}.", style = MaterialTheme.typography.bodyLarge)
|
||||
Button(
|
||||
onClick = viewModel::reset,
|
||||
onClick = actions.onReset,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
@@ -241,7 +322,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
|
||||
style = MaterialTheme.typography.bodyMedium,
|
||||
)
|
||||
Button(
|
||||
onClick = viewModel::reset,
|
||||
onClick = actions.onReset,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
|
||||
@@ -53,6 +53,46 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
remember(destinationMime) { ActivityResultContracts.CreateDocument(destinationMime) },
|
||||
) { uri -> uri?.let(viewModel::save) }
|
||||
|
||||
JoinScreenContent(
|
||||
state = state,
|
||||
actions = JoinActions(
|
||||
onPickInputs = { pickInputs.launch(arrayOf("video/*")) },
|
||||
onJoin = viewModel::join,
|
||||
onCancel = viewModel::cancel,
|
||||
onSave = { suggestedName -> chooseDestination.launch(suggestedName) },
|
||||
onReset = viewModel::reset,
|
||||
),
|
||||
modifier = modifier,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Everything [JoinScreenContent] can ask for, in one value.
|
||||
*
|
||||
* Five callbacks would fit under detekt's `LongParameterList` threshold, unlike the converter's
|
||||
* twelve. It is a holder anyway, so both screens present the same shape to the state tests and
|
||||
* neither one has to be reworked the first time a branch grows a button.
|
||||
*/
|
||||
internal data class JoinActions(
|
||||
/** Open the multi-document picker. The `Idle` and `Ready` branches both offer it. */
|
||||
val onPickInputs: () -> Unit,
|
||||
val onJoin: () -> Unit,
|
||||
val onCancel: () -> Unit,
|
||||
/** Open the save dialog. Takes the name the job chose -- see `ConcatWorker.KEY_SUGGESTED_NAME`. */
|
||||
val onSave: (suggestedName: String) -> Unit,
|
||||
val onReset: () -> Unit,
|
||||
)
|
||||
|
||||
/**
|
||||
* The join screen, with its state handed in.
|
||||
*
|
||||
* The same split as [org.libremediaconverter.convert.ConverterScreenContent], for the same reason:
|
||||
* `JoinState.Waiting` follows a denied foreground start and `JoinState.Joined` follows a completed
|
||||
* concatenation, so neither is reachable by driving a real `JoinViewModel`.
|
||||
*/
|
||||
@UnstableApi
|
||||
@Composable
|
||||
internal fun JoinScreenContent(state: JoinState, actions: JoinActions, modifier: Modifier = Modifier) {
|
||||
Column(
|
||||
modifier = modifier
|
||||
.fillMaxSize()
|
||||
@@ -81,7 +121,7 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
modifier = Modifier.padding(bottom = 16.dp),
|
||||
)
|
||||
Button(
|
||||
onClick = { pickInputs.launch(arrayOf("video/*")) },
|
||||
onClick = actions.onPickInputs,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
@@ -99,14 +139,14 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
is JoinState.Ready -> {
|
||||
s.inputs.forEach { FileRow(it) }
|
||||
Button(
|
||||
onClick = viewModel::join,
|
||||
onClick = actions.onJoin,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.Join.JOIN),
|
||||
) { Text("Join ${s.inputs.size} files") }
|
||||
OutlinedButton(
|
||||
onClick = { pickInputs.launch(arrayOf("video/*")) },
|
||||
onClick = actions.onPickInputs,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.testTag(TestTags.Join.CHOOSE_DIFFERENT_FILES),
|
||||
@@ -124,7 +164,7 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
.testTag(TestTags.Join.PROGRESS),
|
||||
)
|
||||
OutlinedButton(
|
||||
onClick = viewModel::cancel,
|
||||
onClick = actions.onCancel,
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
|
||||
) { Text("Cancel") }
|
||||
}
|
||||
@@ -138,7 +178,7 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
style = MaterialTheme.typography.bodyMedium,
|
||||
)
|
||||
OutlinedButton(
|
||||
onClick = viewModel::cancel,
|
||||
onClick = actions.onCancel,
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL),
|
||||
) { Text("Cancel") }
|
||||
}
|
||||
@@ -157,14 +197,14 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
style = MaterialTheme.typography.bodySmall,
|
||||
)
|
||||
Button(
|
||||
onClick = { chooseDestination.launch(s.suggestedName) },
|
||||
onClick = { actions.onSave(s.suggestedName) },
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
.testTag(TestTags.SAVE_FILE),
|
||||
) { Text("Save file") }
|
||||
OutlinedButton(
|
||||
onClick = viewModel::reset,
|
||||
onClick = actions.onReset,
|
||||
modifier = Modifier.fillMaxWidth().testTag(TestTags.START_OVER),
|
||||
) { Text("Start over") }
|
||||
}
|
||||
@@ -172,7 +212,7 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
is JoinState.Saved -> {
|
||||
Text("Saved ${s.displayName}.", style = MaterialTheme.typography.bodyLarge)
|
||||
Button(
|
||||
onClick = viewModel::reset,
|
||||
onClick = actions.onReset,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
@@ -187,7 +227,7 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
|
||||
style = MaterialTheme.typography.bodyMedium,
|
||||
)
|
||||
Button(
|
||||
onClick = viewModel::reset,
|
||||
onClick = actions.onReset,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(PrimaryButtonHeight)
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.net.Uri
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.compose.ui.test.performScrollTo
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.createDrainedComposeRule
|
||||
import org.libremediaconverter.model.Validation
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
* The seam carries a `ConversionState` in and an action back out.
|
||||
*
|
||||
* The defect this bites on is the extraction having quietly stopped being an extraction: a
|
||||
* `ConverterScreenContent` that ignores the `state` it was handed, or renders the finished job's
|
||||
* affordances without wiring them to the callbacks the entry point supplies. Neither shows up at
|
||||
* compile time -- an unread parameter compiles, and a `Button` whose `onClick` does nothing is a
|
||||
* valid `Button` -- and neither is visible from the leaf tests, which compose `FileCard`,
|
||||
* `AdvancedPicker` and the pickers directly and never see a state at all.
|
||||
*
|
||||
* **Both assertions were unreachable before R38.5**, which is the point of the ticket rather than
|
||||
* a remark about it. `ConversionState.Converted` is produced only by a `ConversionWorker` run that
|
||||
* has already succeeded, so no test can drive a real `ConversionViewModel` into it: it would need
|
||||
* a `WorkManager`, a media probe, a staged output file and a completed job. Handing the state in
|
||||
* is the only way to ask what the screen does with it.
|
||||
*
|
||||
* Deliberately not the state matrix. Which affordances each of the six `ConversionState`s renders
|
||||
* is R38.6 (#62); this file asserts only that the injection point exists and works in both
|
||||
* directions, so the two PRs cannot collide over the same cases.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class ConverterScreenContentTest {
|
||||
|
||||
// Not `createComposeRule()` directly: see [org.libremediaconverter.drainEscapedCoroutineErrors].
|
||||
@get:Rule
|
||||
val composeRule = createDrainedComposeRule()
|
||||
|
||||
/** What the screen asked to save, in the order it asked. Empty until Save is tapped. */
|
||||
private val savedAs = mutableListOf<String>()
|
||||
|
||||
@Test
|
||||
fun `a converted job renders the save button`() {
|
||||
setContent(converted())
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.SAVE_FILE).assertExists()
|
||||
}
|
||||
|
||||
/**
|
||||
* The direction that did not exist before this change.
|
||||
*
|
||||
* Asserting the *name* rather than just that something was called: the suggested name comes
|
||||
* from the job -- `ConversionWorker.KEY_SUGGESTED_NAME` -- and is what the save dialog opens
|
||||
* with, so a Save button wired to the wrong branch's state would hand over the wrong one and
|
||||
* a bare "was called" check would stay green.
|
||||
*/
|
||||
@Test
|
||||
fun `tapping save hands back the name the finished job chose`() {
|
||||
setContent(converted())
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.SAVE_FILE).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("holiday.mp4"), savedAs)
|
||||
}
|
||||
|
||||
/**
|
||||
* `staged` names a file that does not exist, on purpose.
|
||||
*
|
||||
* The branch renders `formatBytes(s.staged.length())`, and `length()` answers `0L` for a
|
||||
* missing path rather than throwing, so the size line reads `0 B` and no temporary folder is
|
||||
* needed. `routeReason` stays blank, which is what keeps the routing chip out of the tree --
|
||||
* that chip is R38.6's case, not this file's.
|
||||
*/
|
||||
private fun converted() = ConversionState.Converted(
|
||||
input = InputFile(
|
||||
uri = Uri.parse("content://test/holiday.mkv"),
|
||||
displayName = "holiday.mkv",
|
||||
sizeBytes = 12_345_678L,
|
||||
),
|
||||
staged = File("no-such-staged-output.mp4"),
|
||||
suggestedName = "holiday.mp4",
|
||||
mimeType = "video/mp4",
|
||||
)
|
||||
|
||||
private fun setContent(state: ConversionState) {
|
||||
composeRule.setContent {
|
||||
ConverterScreenContent(
|
||||
state = state,
|
||||
settings = ConversionSettings(),
|
||||
validation = Validation.Valid,
|
||||
actions = ConverterActions(
|
||||
onPickInput = {},
|
||||
onPreset = {},
|
||||
onContainer = {},
|
||||
onVideoCodec = {},
|
||||
onAudioCodec = {},
|
||||
onSuggestion = {},
|
||||
onQuality = {},
|
||||
onEnginePreference = {},
|
||||
onConvert = {},
|
||||
onCancel = {},
|
||||
onSave = { suggestedName -> savedAs += suggestedName },
|
||||
onReset = {},
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,83 @@
|
||||
package org.libremediaconverter.join
|
||||
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.compose.ui.test.performScrollTo
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.createDrainedComposeRule
|
||||
import org.libremediaconverter.model.ConcatStrategy
|
||||
import org.libremediaconverter.ui.TestTags
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
* The join screen's half of the same seam, and the same two directions.
|
||||
*
|
||||
* The defect is the one `ConverterScreenContentTest` describes -- a content composable that
|
||||
* ignores the state handed to it, or renders the finished job's affordances unwired -- and it has
|
||||
* to be asked separately here because the two screens share no code. `JoinScreen` and
|
||||
* `ConverterScreen` were extracted in the same commit by the same hand, which is exactly the
|
||||
* circumstance in which one of them gets the wiring right and the other does not.
|
||||
*
|
||||
* `JoinState.Joined` is unreachable through a real `JoinViewModel` for the same reason
|
||||
* `ConversionState.Converted` is: only a `ConcatWorker` run that has already succeeded produces
|
||||
* one, carrying the strategy it chose and the name it picked.
|
||||
*
|
||||
* `JoinScreenKt` is the honest remaining coverage gap on this repo, and closing it is R38.7 (#63),
|
||||
* not this file. Which affordances each `JoinState` renders belongs there; this asserts only that
|
||||
* the injection point exists.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class JoinScreenContentTest {
|
||||
|
||||
// Not `createComposeRule()` directly: see [org.libremediaconverter.drainEscapedCoroutineErrors].
|
||||
@get:Rule
|
||||
val composeRule = createDrainedComposeRule()
|
||||
|
||||
/** What the screen asked to save, in the order it asked. Empty until Save is tapped. */
|
||||
private val savedAs = mutableListOf<String>()
|
||||
|
||||
@Test
|
||||
fun `a finished join renders the save button`() {
|
||||
setContent(joined())
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.SAVE_FILE).assertExists()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `tapping save hands back the name the finished join chose`() {
|
||||
setContent(joined())
|
||||
|
||||
composeRule.onNodeWithTag(TestTags.SAVE_FILE).performScrollTo().performClick()
|
||||
|
||||
assertEquals(listOf("joined.mp4"), savedAs)
|
||||
}
|
||||
|
||||
/** `staged` names a missing file deliberately -- see the same helper on the converter side. */
|
||||
private fun joined() = JoinState.Joined(
|
||||
staged = File("no-such-staged-output.mp4"),
|
||||
strategy = ConcatStrategy.STREAM_COPY,
|
||||
suggestedName = "joined.mp4",
|
||||
mimeType = "video/mp4",
|
||||
)
|
||||
|
||||
private fun setContent(state: JoinState) {
|
||||
composeRule.setContent {
|
||||
JoinScreenContent(
|
||||
state = state,
|
||||
actions = JoinActions(
|
||||
onPickInputs = {},
|
||||
onJoin = {},
|
||||
onCancel = {},
|
||||
onSave = { suggestedName -> savedAs += suggestedName },
|
||||
onReset = {},
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user