Compare commits

...
Author SHA1 Message Date
JMR-dev 3d55004286 Count the Robolectric tests, which JaCoCo has never counted
The three #52 test PRs landed 56 new tests and the coverage figure moved 29.8% -> 29.7%.
That looked like the tests being worthless. It was the measurement.

Robolectric loads every class it touches through its own sandbox classloader, and those
classes arrive with no source location. JaCoCo skips no-location classes unless told
otherwise, and nothing here told it. So not one Robolectric test has ever contributed
coverage in this repo -- and Robolectric is what exercises the framework edge: both
workers, the publisher, both ViewModels, every Compose screen.

Same commit, same 335 tests, same 0 failures, only the block below added:

  LINE    652/2194  29.7%  ->  1519/2194  69.2%
  BRANCH  425/1424  29.8%  ->   758/1424  53.2%

  OutputPublisher       0.0% -> 97.5%      MainActivityKt     6.8% -> 86.4%
  ConversionViewModel   0.0% -> 85.4%      ConverterScreenKt  6.6% -> 62.8%

The discriminator, so this is not cargo cult: 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. Across files the split is exactly
Robolectric-vs-not: StagingSweep, tested purely, 100%; OutputPublisher, ConversionViewModel
and FailureOutcome, tested under Robolectric, 0%.

`excludes = listOf("jdk.internal.*")` is not decoration. Without it JaCoCo walks
JDK-internal classes Robolectric has no location for either and the test JVM dies rather
than reporting a number.

CLAUDE.md's coverage bullet is rewritten, because it was wrong twice over. The figure was
an artifact, and the explanation attached to it -- that 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" -- described a cause that does not exist. The JVM reaches that
code fine. The new tests were disproportionately Robolectric, so each one added denominator
and no numerator: the measurement was punishing precisely the tests that were hardest to
write, and the conclusion drawn from it was that writing them had not helped.

Mutation, run both ways on this branch: remove the block and jacocoTestReport collapses
back to 29.7% / 29.8%; restore it and it returns to 69.2% / 53.2%.

Two things that were true stay true. There is still no coverage gate, and a floor still
needs a settled baseline -- this one just moved 39 points in one build change. And
"re-measure before quoting it" was already written down; following it is the only reason
this was found.
2026-08-24 18:45:30 -05:00
Jason Ross e06b0826a0 Merge pull request #73 from JMR-dev/docs/instrumented-tests-correction
Say where instrumented tests run, instead of where they used to not run
2026-08-24 17:17:18 -05:00
JMR-dev 7b578c1ccf Merge branch 'main' into docs/instrumented-tests-correction 2026-08-24 17:10:27 -05:00
Jason Ross 9e7f80feaa Merge pull request #72 from JMR-dev/test/r38-2-filecard
Say in tests what the file card says when it does not know
2026-08-24 17:10:06 -05:00
JMR-dev 3c5a37fd3c Merge branch 'main' into test/r38-2-filecard 2026-08-24 17:02:19 -05:00
Jason Ross af13155c27 Merge pull request #71 from JMR-dev/test/r38-4-advanced-picker
Hold the Advanced panel's gate, and the error card outside it
2026-08-24 17:01:33 -05:00
JMR-dev 4ea5afefe1 Merge branch 'main' into test/r38-4-advanced-picker 2026-08-24 16:54:10 -05:00
Jason Ross 7e4f22322b Merge pull request #69 from JMR-dev/tools/file-issue-script
Check the shell, and stop one-off issues falling off the board
2026-08-24 16:52:16 -05:00
JMR-dev 3d51fefeff Say where instrumented tests run, instead of where they used to not run
CLAUDE.md carried three claims about instrumented tests. All three were false, one of
them contradicted a paragraph forty lines below it in the same file, and a subagent
working on #58 hit the contradiction and had to stop and flag it rather than trust the
project's own instructions. That is the cost being paid here: this file is what every
contributor and every agent reads first.

  "Instrumented tests do not run locally"  -- they do, API 33-36, since 22c7914.
  "Emulators segfault on this host"        -- solved 2026-08-22; it was SwiftShader's
                                              Reactor JIT meeting SELinux execheap, not a
                                              broken machine, and another renderer avoids
                                              it. docs/local-emulator.md is titled
                                              "Emulators do run on this host".
  "CI's matrix therefore stops at API 36"  -- the matrix has been 33/34/35/36/37 since
                                              #56 merged, with a gating API 37 leg.

The contradiction was the worst of it. The testing-norm section added in #51 says "E2E is
runnable locally now", so the file simultaneously told you the emulator works and that it
segfaults on every AVD. A reader has no way to tell which half is current, and the wrong
half is the one that stops work: an agent that believes emulators are impossible here does
not try, and the local e2e half of the definition-of-done in #51 quietly stops being
enforceable.

The replacement says what is true now and names what is still true and why -- API 37 still
needs the manual Pixel check before a release, because the two advisory tests are the one
thing CI cannot answer for. It also states plainly that the advisory job is red on every
PR by design, which is the other thing agents keep rediscovering the hard way: three
separate subagents have now flagged that failure as possibly theirs.

The norm bullet now points at the section rather than re-arguing it, so there is one place
to correct next time rather than two that can drift apart again.

Same defect class as R14, R15, R20 and R25, all of which were documentation claims this
repo's own review falsified. The pattern is not that the docs were careless; it is that
they were written at a moment and the moment moved.
2026-08-24 16:26:47 -05:00
JMR-devandClaude Opus 5 fea88a281f Say in tests what the file card says when it does not know
"Size unknown" is the line a stream fixing D5 reported as untestable. It is two
assertTextEquals calls, and it needed two rather than one: the size line renders
independently of the probe, so it is asserted with a probe and without one. That
independence is the contract, and a test of the probed case alone would leave the
branch a user hits first -- the card is on screen before the probe finishes --
unguarded.

The rest of the card degrades in words the same way, and none of it was covered:
the four InputKind branches, "No video track", "No audio track", describeVideo's
"Unknown", and the two `> 0` guards that drop the dimension and length rows
rather than printing 0 and 0:00. Each guard gets a case on both sides, because
the present side alone stays green when the guard is deleted -- what deleting it
produces is "Size: 0x0" and "Length: 0:00", the same invented-measurement defect
as "0 B".

The four pure helpers go in a plain JVM class beside it, with formatBytes pinned
at each threshold and one byte below it. A `>=` quietly becoming a `>` is only
visible from a value sitting exactly on the boundary.

Two things the issue could not have known:

- Its second acceptance criterion, "delete the return@Column and watch the
  Reading... test go red", cannot happen -- it does not compile. The early return
  is what smart-casts `probe` non-null, so ten uses below it fail with "Only safe
  (?.) or non-null asserted (!!.) calls are allowed on a nullable receiver". The
  exit is enforced by the compiler, not by a test. Both compilable regressions
  someone would land instead are covered and were run red.
- CodecNames.describeAudio has no UNPARSEABLE arm, unlike describeVideo, so it
  answers the raw sentinel rather than "Unrecognised". Unreachable today, because
  the UNPARSEABLE kind renders the explanatory line instead of rows. Left alone;
  recorded on the PR for R38.5.

The divider's absence is not asserted and cannot be: Material 3 renders it as a
Box with no semantics modifier, so it contributes no node. What is asserted is
everything it precedes, plus the card's child count. The class KDoc says so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-24 16:10:38 -05:00
JMR-devandClaude Opus 5 7c69d0699a Hold the Advanced panel's gate, and the error card outside it
`AdvancedPicker` is the one leaf on the converter screen that carries its own
state, and `ValidationError` is deliberately invoked after the
`AnimatedVisibility` that gates the chip rows -- so an invalid spec explains
itself and offers one-tap fixes while the section is collapsed. That is the
only route out of an invalid spec for a user who never opened Advanced, it was
completely untested, and folding the two `if` blocks into one is a plausible
tidy-up that compiles.

`AdvancedPickerTest` covers the gate in both directions, clicks each of the
four colliding chip labels through its own row tag, and does every assertion
about the error card with the toggle untouched.

`AdvancedPanelSavedStateTest` is the `DestinationSaverTest` split for
`expanded`: `StateRestorationTester` saves into an in-memory map, so it proves
`rememberSaveable` is in use and nothing about the representation. Driving a
real `SaveableStateRegistry` shows the picker saves the `MutableState` itself
rather than the `Boolean`, which only survives a rotation because
`mutableStateOf` on Android returns a `Parcelable` one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-24 16:06:00 -05:00
6 changed files with 953 additions and 17 deletions
+43 -17
View File
@@ -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.
+24
View File
@@ -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()
}
@@ -0,0 +1,159 @@
package org.libremediaconverter.convert
import android.os.Bundle
import android.os.Parcel
import android.os.Parcelable
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.saveable.LocalSaveableStateRegistry
import androidx.compose.runtime.saveable.SaveableStateRegistry
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.performClick
import androidx.media3.common.util.UnstableApi
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.createDrainedComposeRule
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.OutputSpec
import org.libremediaconverter.model.Validation
import org.libremediaconverter.model.VideoCodec
import org.libremediaconverter.ui.TestTags
import org.robolectric.RobolectricTestRunner
/**
* What `AdvancedPickerTest`'s restoration test cannot see.
*
* `StateRestorationTester` saves into an **in-memory map**, never a `Bundle`. That is enough to
* discriminate `rememberSaveable` from `remember`, and it is where it stops: the map holds object
* references, so a value the platform could never parcel goes in and comes back out looking green.
* `AppRootRestorationTest` has the same blind spot and `DestinationSaverTest` is the split it
* prompted; this is that split for `expanded`, the only `rememberSaveable` on either screen's
* leaves.
*
* ### The saved representation is not the Boolean
*
* `var expanded by rememberSaveable { mutableStateOf(false) }` passes no `stateSaver`, so
* `autoSaver` saves **the `MutableState` itself**, not the `false` inside it. That works only
* because `mutableStateOf` on Android returns a `Parcelable` implementation -- the same call on a
* plain JVM returns one that is not. So what stands between an open panel and a rotation that
* closes it is a platform-specific detail of a factory function nothing here names directly, and
* an in-memory map cannot tell the two apart.
*
* Pinning it is the move `DestinationSaverTest` makes about names versus ordinals. Passing an
* explicit `stateSaver` would save a bare `Boolean` instead and is a perfectly reasonable edit --
* it is just not the one in the tree, and it should be made on purpose rather than discovered
* after a rotation.
*
* ### Shared bite, stated rather than implied
*
* `rememberSaveable` -> `remember` empties the registry, so it reddens this file *and* the
* restoration test in `AdvancedPickerTest`. Both failures belong in any report of that mutation.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class AdvancedPanelSavedStateTest {
// Not `createComposeRule()` directly: see [drainEscapedCoroutineErrors].
@get:Rule
val composeRule = createDrainedComposeRule()
/**
* `canBeSaved = { true }` deliberately.
*
* A predicate mirroring what a `Bundle` accepts would be a hand-written copy of the thing
* under test, and a `false` from it *drops* the entry silently -- so the test would fail by
* finding nothing saved, which is also how a `remember` regression fails. Two causes, one
* symptom, is not a test. The type is checked on the way out instead.
*/
private val registry = SaveableStateRegistry(restoredValues = null, canBeSaved = { true })
@Test
fun `the panel registers its open state with the registry, and nothing else`() {
setPicker()
// Collapsed is a saved value, not an absent one: `rememberSaveable` registers its provider
// on first composition, whatever the state happens to be. Exactly one, because `expanded`
// is the only saveable in the subtree -- a second would mean something else began saving.
assertEquals(1, savedValues().size)
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
val saved = theOneSavedValue()
assertTrue("saved as ${saved?.javaClass?.name}", saved is MutableState<*>)
assertEquals(true, (saved as MutableState<*>).value)
}
@Test
fun `the open panel survives a real Parcel, not just an in-memory map`() {
setPicker()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
val saved = theOneSavedValue()
// The claim the restoration test cannot make. A `MutableState` that was not `Parcelable`
// would satisfy `StateRestorationTester` and then be dropped by the platform.
assertTrue("saved as ${saved?.javaClass?.name}", saved is Parcelable)
val restored = throughARealBundle(saved as Parcelable)
assertTrue("restored as ${restored.javaClass.name}", restored is MutableState<*>)
assertEquals(true, (restored as MutableState<*>).value)
}
private fun setPicker() {
composeRule.setContent {
CompositionLocalProvider(LocalSaveableStateRegistry provides registry) {
AdvancedPicker(
spec = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.AAC),
validation = Validation.Valid,
onContainer = {},
onVideoCodec = {},
onAudioCodec = {},
onSuggestion = {},
)
}
}
}
/** Every value the picker hands the host to persist, keys dropped -- they are positional. */
private fun savedValues(): List<Any?> = composeRule.runOnIdle { registry.performSave().values.flatten() }
/**
* The single saved value, asserted rather than assumed.
*
* `single()` on an empty list throws `NoSuchElementException: List is empty`, which names
* neither the panel nor the registry -- and an empty registry is exactly how the
* `rememberSaveable` -> `remember` regression shows up here.
*/
private fun theOneSavedValue(): Any? {
val values = savedValues()
assertEquals("the panel should register exactly one saved value", 1, values.size)
return values.first()
}
/** A write and a read through a real `Parcel`, which is what the tester's map stands in for. */
private fun throughARealBundle(value: Parcelable): Parcelable {
val bundle = Bundle().apply { putParcelable(KEY, value) }
val parcel = Parcel.obtain()
return try {
parcel.writeBundle(bundle)
parcel.setDataPosition(0)
val restored = requireNotNull(parcel.readBundle(javaClass.classLoader)) {
"the Bundle did not survive the Parcel"
}
requireNotNull(restored.getParcelable(KEY, Parcelable::class.java)) {
"the saved state did not survive the Parcel"
}
} finally {
parcel.recycle()
}
}
private companion object {
const val KEY = "expanded"
}
}
@@ -0,0 +1,321 @@
package org.libremediaconverter.convert
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.assertTextEquals
import androidx.compose.ui.test.hasAnyAncestor
import androidx.compose.ui.test.hasTestTag
import androidx.compose.ui.test.hasText
import androidx.compose.ui.test.junit4.StateRestorationTester
import androidx.compose.ui.test.onAllNodesWithTag
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.media3.common.util.UnstableApi
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.createDrainedComposeRule
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.ContainerCapabilities
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.OutputSpec
import org.libremediaconverter.model.Validation
import org.libremediaconverter.model.VideoCodec
import org.libremediaconverter.ui.TestTags
import org.robolectric.RobolectricTestRunner
/**
* The gate over the Advanced chips, and the error card that deliberately sits outside it.
*
* Two defects, and they pull in opposite directions.
*
* The first is the chips escaping the gate, or never being reachable through it. `AdvancedPicker`
* is the one leaf on this screen that is not stateless -- `expanded` is its own `rememberSaveable`
* -- and Container, Video and Audio live inside `AnimatedVisibility(visible = expanded)`. Nothing
* else on the screen hides anything, so a refactor that flattened the panel, or wired the toggle to
* a state nobody reads, would render an app that looks reasonable in a screenshot and is wrong.
*
* The second is the opposite mistake, and it is the one this file exists for: **moving the
* `ValidationError` call inside the `AnimatedVisibility`**. It is invoked after that block, so an
* invalid spec explains itself and offers one-tap fixes *while the section is collapsed*. That is
* the only route out of an invalid spec for a user who never opened Advanced -- and since the only
* way to reach an invalid spec is through Advanced, hiding the way out behind the same toggle looks
* locally sensible and is a trap. Tidying the two `if` blocks into one is a plausible edit, it
* compiles, and until this file existed nothing went red. Every assertion about the error card here
* therefore runs with the toggle untouched, and asserts the panel is absent in the same test, so a
* future `expanded = true` default cannot quietly satisfy it either.
*
* The invalid specs come from [ContainerCapabilities.validate] rather than from a hand-built
* [Validation.Invalid], so the messages and the suggestions are the real pairing. A hand-built one
* would keep passing after `validate` stopped producing anything like it.
*
* Node location is by the three separate chip-row tags, never by text. `"Copy"` and `"None"` are
* each both a [VideoCodec] and an [AudioCodec], and `"MP3"` and `"FLAC"` are each both a
* [Container] and an [AudioCodec], so a text matcher over the open panel is ambiguous for four
* chips -- which is what the separate tags are for.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class AdvancedPickerTest {
// Not `createComposeRule()` directly: see [drainEscapedCoroutineErrors].
@get:Rule
val composeRule = createDrainedComposeRule()
private val restoration = StateRestorationTester(composeRule)
private val containers = mutableListOf<Container>()
private val videoCodecs = mutableListOf<VideoCodec>()
private val audioCodecs = mutableListOf<AudioCodec>()
private val applied = mutableListOf<OutputSpec>()
// --- the expand gate ----------------------------------------------------
@Test
fun `the three chip rows appear only while the panel is expanded`() {
setPicker()
assertPanelHidden()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertExists()
ROW_TAGS.forEach { composeRule.onNodeWithTag(it).assertExists() }
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
// The exit transition outlives the click, so absence has to be waited for rather than
// asserted straight away -- unlike the initial collapsed state, which has no animation
// in flight.
composeRule.waitUntil { nodeCount(TestTags.Converter.ADVANCED_PANEL) == 0 }
assertPanelHidden()
}
/** The toggle is the only affordance the collapsed picker offers, so it has to say so. */
@Test
fun `the toggle names the direction it will move in`() {
setPicker()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).assertTextEquals("Advanced")
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE)
.assertTextEquals("Hide advanced")
}
/**
* The four colliding labels, one per row.
*
* `"Copy"` is a video codec *and* an audio codec; `"MP3"` is a container *and* an audio codec.
* Clicking each through its own row is what proves the rows are wired to different callbacks
* -- a picker that handed every chip to `onAudioCodec` would look identical on screen.
*/
@Test
fun `each chip row reports to its own callback, including the labels that collide`() {
setPicker()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
chipIn(TestTags.Converter.ADVANCED_VIDEO_CHIPS, "Copy").performClick()
assertEquals(listOf(VideoCodec.COPY), videoCodecs)
assertEquals(emptyList<AudioCodec>(), audioCodecs)
chipIn(TestTags.Converter.ADVANCED_AUDIO_CHIPS, "Copy").performClick()
assertEquals(listOf(AudioCodec.COPY), audioCodecs)
chipIn(TestTags.Converter.ADVANCED_CONTAINER_CHIPS, "MP3").performClick()
assertEquals(listOf(Container.MP3), containers)
// Still only the one audio click. `MP3` is an AudioCodec label too, and the container row
// must not be reporting through that callback.
assertEquals(listOf(AudioCodec.COPY), audioCodecs)
}
// --- the error card, which is outside the gate --------------------------
/**
* The headline case. Dropping both tracks is reachable from the collapsed screen -- the
* `None`/`None` pair is set inside Advanced, but the user can close it again -- and the
* explanation has to still be there.
*/
@Test
fun `an empty output explains itself while the section is collapsed`() {
val spec = OutputSpec(Container.MP4, VideoCodec.NONE, AudioCodec.NONE)
val invalid = invalidFor(spec)
assertEquals("This would produce an empty file — keep at least one track.", invalid.message)
setPicker(spec, invalid)
assertPanelHidden()
composeRule.onNodeWithTag(TestTags.Converter.VALIDATION_ERROR).assertExists()
composeRule.onNodeWithText(invalid.message).assertIsDisplayed()
}
@Test
fun `a codec the container cannot hold explains itself while the section is collapsed`() {
val spec = OutputSpec(Container.WEBM, VideoCodec.H264, AudioCodec.OPUS)
val invalid = invalidFor(spec)
assertEquals("WebM cannot hold H.264 video.", invalid.message)
setPicker(spec, invalid)
assertPanelHidden()
composeRule.onNodeWithText(invalid.message).assertIsDisplayed()
}
/**
* Clicking a suggestion, with the toggle never touched.
*
* The second suggestion rather than the first, and its count pinned first: with one suggestion
* a picker that handed every chip `suggestions[0]` would pass, and `onNodeWithTag` on a
* suggestion index that no longer exists reports an unhelpful matcher failure rather than
* saying the list shrank.
*/
@Test
fun `a suggestion chip applies its own spec without the section ever being opened`() {
val spec = OutputSpec(Container.WEBM, VideoCodec.H264, AudioCodec.OPUS)
val invalid = invalidFor(spec)
assertEquals(2, invalid.suggestions.size)
val second = invalid.suggestions[1]
setPicker(spec, invalid)
assertPanelHidden()
composeRule.onNodeWithTag(TestTags.Converter.suggestion(1)).assertTextEquals(describe(second))
composeRule.onNodeWithTag(TestTags.Converter.suggestion(1)).performClick()
assertEquals(listOf(second), applied)
// What the chips offer is what `validate` said would work, not a repair of the test's own.
assertTrue(
"suggestion $second should itself validate",
ContainerCapabilities.validate(second, PROBE).isValid,
)
}
/** A valid spec has nothing to say, collapsed or not. */
@Test
fun `a valid spec renders no error card`() {
setPicker()
composeRule.onNodeWithTag(TestTags.Converter.VALIDATION_ERROR).assertDoesNotExist()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
composeRule.onNodeWithTag(TestTags.Converter.VALIDATION_ERROR).assertDoesNotExist()
}
// --- recreation ---------------------------------------------------------
/**
* `expanded` is the only `rememberSaveable` on either screen's leaves.
*
* `MainActivity` declares no `configChanges`, so a rotation destroys and rebuilds the whole
* composition. A panel the user opened, set three chips in, and left open must not close
* itself on the way back. `remember` would.
*
* What this cannot see is the saved *representation* -- `StateRestorationTester` saves into an
* in-memory map rather than a `Bundle`. `AdvancedPanelSavedStateTest` covers that half.
*/
@Test
fun `an open panel is still open after recreation`() {
restoration.setContent {
AdvancedPicker(
spec = VALID_SPEC,
validation = Validation.Valid,
onContainer = {},
onVideoCodec = {},
onAudioCodec = {},
onSuggestion = {},
)
}
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertExists()
restoration.emulateSavedInstanceStateRestore()
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertExists()
ROW_TAGS.forEach { composeRule.onNodeWithTag(it).assertExists() }
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE)
.assertTextEquals("Hide advanced")
}
/** The default has to survive too, or the panel would spring open on every rotation. */
@Test
fun `a collapsed panel is still collapsed after recreation`() {
restoration.setContent {
AdvancedPicker(
spec = VALID_SPEC,
validation = Validation.Valid,
onContainer = {},
onVideoCodec = {},
onAudioCodec = {},
onSuggestion = {},
)
}
assertPanelHidden()
restoration.emulateSavedInstanceStateRestore()
assertPanelHidden()
}
// --- helpers ------------------------------------------------------------
private fun setPicker(spec: OutputSpec = VALID_SPEC, validation: Validation = Validation.Valid) {
composeRule.setContent {
AdvancedPicker(
spec = spec,
validation = validation,
onContainer = { containers += it },
onVideoCodec = { videoCodecs += it },
onAudioCodec = { audioCodecs += it },
onSuggestion = { applied += it },
)
}
}
/** The whole panel, by every tag it owns, so a partial escape counts as a failure. */
private fun assertPanelHidden() {
composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertDoesNotExist()
ROW_TAGS.forEach { composeRule.onNodeWithTag(it).assertDoesNotExist() }
}
private fun nodeCount(tag: String) = composeRule.onAllNodesWithTag(tag).fetchSemanticsNodes().size
private fun chipIn(rowTag: String, label: String) =
composeRule.onNode(hasText(label) and hasAnyAncestor(hasTestTag(rowTag)))
private fun invalidFor(spec: OutputSpec): Validation.Invalid {
val validation = ContainerCapabilities.validate(spec, PROBE)
return validation as? Validation.Invalid
?: throw AssertionError("$spec was expected to be invalid, but validate said $validation")
}
private companion object {
val ROW_TAGS = listOf(
TestTags.Converter.ADVANCED_CONTAINER_CHIPS,
TestTags.Converter.ADVANCED_VIDEO_CHIPS,
TestTags.Converter.ADVANCED_AUDIO_CHIPS,
)
val VALID_SPEC = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.AAC)
/** An ordinary H.264/AAC MP4, so the suggestions have a real source to repair towards. */
val PROBE = InputProbe(
videoCodec = "h264",
audioCodec = "aac",
durationMs = 90_000,
container = Container.MP4,
)
}
}
@@ -0,0 +1,152 @@
package org.libremediaconverter.convert
import org.junit.Assert.assertEquals
import org.junit.Test
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.EnginePreference
import org.libremediaconverter.model.OutputSpec
import org.libremediaconverter.model.VideoCodec
/**
* The four pure helpers behind the converter screen's prose, pinned at the points where they
* change what they say.
*
* No Compose rule and no Robolectric: these are `String` in, `String` out, and running them under a
* device sandbox would buy nothing while hiding the boundaries in a rendered tree.
*
* The defect each group bites on:
*
* - **[formatBytes] picks a unit by comparing against three thresholds.** Every one of them is a
* `>=`, and a `>` would move a file sitting exactly on a boundary into the unit below -- `1 GB`
* shown as `1000.0 MB`. Only a value *on* the threshold can tell the two apart, so each of the
* three is asserted at the boundary and one below it. The unit prefixes are decimal, matching
* what the file manager and the provider report, not powers of two.
* - **[formatDuration] has no hours field.** An hour-long recording reads `60:00`, and that is the
* contract rather than an oversight -- the row is a length, not a clock. Pinned so that adding
* hours is a deliberate change with a red test in front of it instead of a silent reformat.
* - **[describe] builds the suggestion-chip label out of up to three parts**, and the parts are
* conditional: [VideoCodec.NONE] and [AudioCodec.NONE] drop out entirely, so an image output
* with neither track has to render as the container alone rather than as a container followed
* by a dangling separator.
* - **[EnginePreference] carries no `label` property**, unlike every other enum the screen
* renders; its three display strings live in a `when` in the screen file. Adding a constant is
* caught by the compiler because that `when` is exhaustive, but nothing stops two constants
* being given the same string, which is what the distinctness assertion is for.
*/
class ConverterFormattersTest {
@Test
fun `bytes below a kilobyte are counted exactly`() {
assertEquals("0 B", formatBytes(0))
assertEquals("1 B", formatBytes(1))
assertEquals("999 B", formatBytes(999))
}
@Test
fun `each unit starts exactly on its threshold rather than one byte past it`() {
assertEquals("1 kB", formatBytes(1_000))
assertEquals("1.0 MB", formatBytes(1_000_000))
assertEquals("1.0 GB", formatBytes(1_000_000_000))
}
/**
* One byte below each threshold, which is the half a `>=` to `>` change leaves alone. Both
* halves are needed: the boundary values alone would still pass if the comparison let
* everything through.
*/
@Test
fun `a value just below a threshold stays in the smaller unit`() {
assertEquals("999 B", formatBytes(999))
assertEquals("1000 kB", formatBytes(999_999))
assertEquals("1000.0 MB", formatBytes(999_999_999))
}
@Test
fun `a real file size reads as one decimal place`() {
assertEquals("12.3 MB", formatBytes(12_345_678))
assertEquals("1.5 GB", formatBytes(1_500_000_000))
}
@Test
fun `a duration is minutes and zero-padded seconds`() {
assertEquals("0:00", formatDuration(0))
assertEquals("0:01", formatDuration(1_000))
assertEquals("0:59", formatDuration(59_000))
assertEquals("1:00", formatDuration(60_000))
assertEquals("1:30", formatDuration(90_000))
}
/** Sub-second remainders are dropped rather than rounded up into the next second. */
@Test
fun `a partial second does not become a whole one`() {
assertEquals("0:00", formatDuration(999))
assertEquals("0:59", formatDuration(59_999))
}
/** No hours field, deliberately: an hour is `60:00` and two hours are `120:00`. */
@Test
fun `an hour and beyond keeps counting in minutes`() {
assertEquals("60:00", formatDuration(3_600_000))
assertEquals("61:01", formatDuration(3_661_000))
assertEquals("120:00", formatDuration(7_200_000))
}
@Test
fun `a spec with both tracks names the container and joins the two codecs`() {
assertEquals(
"MP4 · H.264 + AAC",
describe(OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.AAC)),
)
}
@Test
fun `a track set to none is left out instead of being named none`() {
assertEquals(
"MP3 · MP3",
describe(OutputSpec(Container.MP3, VideoCodec.NONE, AudioCodec.MP3)),
)
assertEquals(
"MP4 · H.264",
describe(OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.NONE)),
)
}
/** An image output has neither track, so there is nothing for the separator to separate. */
@Test
fun `a spec with no tracks at all is the container alone, with no trailing separator`() {
assertEquals("GIF", describe(OutputSpec(Container.GIF, VideoCodec.NONE, AudioCodec.NONE)))
assertEquals(
"PNG frames",
describe(OutputSpec(Container.IMAGE_SEQUENCE, VideoCodec.NONE, AudioCodec.NONE)),
)
}
/** `Copy` is a codec here, not the absence of one, so a remux describes both tracks. */
@Test
fun `a remux names copy on both tracks rather than dropping them`() {
assertEquals(
"Matroska · Copy + Copy",
describe(OutputSpec(Container.MKV, VideoCodec.COPY, AudioCodec.COPY)),
)
}
@Test
fun `each engine preference has the wording the chips show`() {
assertEquals("Automatic", EnginePreference.AUTO.label())
assertEquals("Prefer hardware", EnginePreference.PREFER_HARDWARE.label())
assertEquals("Force software", EnginePreference.FORCE_SOFTWARE.label())
}
/**
* Two constants sharing a label would render as two identical chips, one of which the user
* could not choose deliberately. The exhaustive `when` cannot catch that; this does.
*/
@Test
fun `no two engine preferences render the same chip`() {
val labels = EnginePreference.entries.map { it.label() }
assertEquals(EnginePreference.entries.size, labels.toSet().size)
assertEquals(emptyList<String>(), labels.filter { it.isBlank() })
}
}
@@ -0,0 +1,254 @@
package org.libremediaconverter.convert
import android.net.Uri
import androidx.compose.ui.test.assertCountEquals
import androidx.compose.ui.test.assertTextEquals
import androidx.compose.ui.test.onChildren
import androidx.compose.ui.test.onNodeWithTag
import androidx.media3.common.util.UnstableApi
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.createDrainedComposeRule
import org.libremediaconverter.model.AudioCodec
import org.libremediaconverter.model.Container
import org.libremediaconverter.model.InputKind
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.VideoCodec
import org.libremediaconverter.ui.TestTags
import org.robolectric.RobolectricTestRunner
/**
* What the source-info card says when it does not know something.
*
* The defect is a card that invents an answer instead of admitting it has none. Two of them are
* live here and neither had a test before this file:
*
* - **`InputFile.sizeBytes` is nullable and the card is the reader that has to say so in words.**
* `sizeBytes` used to be `0L` for "nobody told me", and [UnknownInputSizeTest] records what that
* cost at the space check. The card is the other reader, and its failure mode is the mirror
* image: hand the null to `formatBytes` and it renders `"0 B"` -- a measurement, shown to the
* user, that no provider ever made. It renders **independently of the probe**, which is why the
* same assertion appears twice below, with the probe present and absent. That independence is
* the contract; a test covering only the probed case would leave the branch a user actually hits
* first -- the card is on screen before the probe finishes -- unguarded.
* - **The codec rows degrade in words too.** `CodecNames.describeVideo`/`describeAudio` answer
* `"Unknown"` for a codec nothing named, the `VIDEO` branch answers `"No audio track"` for a file
* with no audio, and the two `> 0` guards drop the dimension and length rows rather than printing
* `0` and `0:00`. Each of those has a case below on **both** sides of the guard, because a test
* of the present side alone stays green with the guard deleted.
*
* ### What cannot be asserted here, so that it is a decision rather than an omission
*
* The `probe == null` branch exits before `HorizontalDivider`, and **the divider's absence is not
* observable from a test**: Material 3 renders it as a `Box` with no semantics modifier, so it
* contributes no node to the semantics tree at all. What is asserted instead is everything the
* divider precedes -- no detail row for any label the four kind branches can emit -- plus the
* card's child count, which pins "these three texts and nothing else" without having to enumerate.
*
* The early exit itself is enforced by the compiler rather than by this file, which the PR body
* records: deleting `return@Column` un-smart-casts `probe`, and the `probe.kind` below it stops
* compiling. The mutation that reddens the test here is the compilable form of that regression --
* defaulting the null away with `?: InputProbe()` and letting the kind rows render.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class FileCardTest {
@get:Rule
val composeRule = createDrainedComposeRule()
@Test
fun `a file no provider could measure says so in words rather than showing a zero`() {
setFileCard(input(sizeBytes = null, probe = VIDEO_PROBE))
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_BYTES)
.assertTextEquals("Size unknown")
}
/**
* The same line, with no probe at all. Separate from the case above rather than folded into
* it because `setContent` may only be called once per rule, and because two independent reds
* are the evidence that the size line does not depend on the probe.
*/
@Test
fun `the size line says the same thing while the probe is still running`() {
setFileCard(input(sizeBytes = null, probe = null))
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_BYTES)
.assertTextEquals("Size unknown")
}
@Test
fun `a size that was reported is formatted rather than replaced by the unknown line`() {
setFileCard(input(sizeBytes = 12_345_678L, probe = VIDEO_PROBE))
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NAME).assertTextEquals("clip.mkv")
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_BYTES).assertTextEquals("12.3 MB")
}
/**
* The note and the emptiness are one behaviour, so they are one test: a regression that keeps
* the note but renders the rows anyway would leave a note-only test green.
*/
@Test
fun `while the probe is still running the card shows the reading note and nothing else`() {
setFileCard(input(probe = null))
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NOTE)
.assertTextEquals("Reading…")
assertNoDetailRows()
// Name, size, note. Catches a row whose label is not in EVERY_ROW_LABEL as well.
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD).onChildren().assertCountEquals(3)
}
@Test
fun `a file nothing could read gets the explanatory line instead of unknown codecs`() {
setFileCard(input(probe = InputProbe(kind = InputKind.UNPARSEABLE)))
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NOTE)
.assertTextEquals("Could not identify this file. It will be converted with FFmpeg.")
assertNoDetailRows()
}
@Test
fun `an image gets its type and its pixel dimensions`() {
setFileCard(input(probe = InputProbe(kind = InputKind.IMAGE, width = 1920, height = 1080)))
assertRow("Type", "Image")
assertRow("Size", "1920×1080")
}
/** The `width > 0` guard, from the side that would print `0×0` if it were dropped. */
@Test
fun `an image whose dimensions nothing reported gets the type row alone`() {
setFileCard(input(probe = InputProbe(kind = InputKind.IMAGE)))
assertRow("Type", "Image")
assertNoRow("Size")
}
@Test
fun `an audio-only file says it has no video track rather than leaving the row blank`() {
setFileCard(
input(
probe = InputProbe(
audioCodec = "aac",
hasVideo = false,
durationMs = 90_000,
kind = InputKind.AUDIO_ONLY,
container = Container.MP3,
),
),
)
assertRow("Container", Container.MP3.label)
assertRow("Video", "No video track")
assertRow("Audio", AudioCodec.AAC.label)
assertRow("Length", "1:30")
assertNoRow("Type")
assertNoRow("Size")
}
/**
* Everything the audio branch can fail to know, at once: no container, no codec name, no
* duration. Each degrades in its own words, and the length row disappears rather than
* claiming `0:00`.
*/
@Test
fun `an audio-only file nothing else could describe degrades one row at a time`() {
setFileCard(input(probe = InputProbe(hasVideo = false, kind = InputKind.AUDIO_ONLY)))
assertRow("Container", "Unknown")
assertRow("Video", "No video track")
assertRow("Audio", "Unknown")
assertNoRow("Length")
}
@Test
fun `a video file composes its codec with its dimensions on one row`() {
setFileCard(input(probe = VIDEO_PROBE))
assertRow("Container", Container.MP4.label)
assertRow("Video", "${VideoCodec.H264.label} · 1920×1080")
assertRow("Audio", AudioCodec.AAC.label)
assertRow("Length", "1:30")
}
/**
* `"No audio track"` rather than `describeAudio(null)`'s `"Unknown"`. The video branch knows
* the difference between a track it could not name and a track that is not there; the audio
* branch above cannot, because a file with no audio is not audio-only.
*/
@Test
fun `a video file with no audio track says so instead of naming an unknown codec`() {
setFileCard(input(probe = VIDEO_PROBE.copy(audioCodec = null)))
assertRow("Audio", "No audio track")
}
/** Both `> 0` guards on the video branch, plus the codec name nothing supplied. */
@Test
fun `a video file missing its codec, dimensions and duration omits them rather than faking them`() {
setFileCard(
input(
probe = VIDEO_PROBE.copy(
videoCodec = null,
width = 0,
height = 0,
durationMs = 0,
),
),
)
assertRow("Video", "Unknown")
assertNoRow("Length")
}
/**
* The row is one node, not a label node beside a value node. A test matching on `"Container"`
* alone would pass against either shape.
*/
@Test
fun `a detail row renders its label and its value as a single node`() {
composeRule.setContent { DetailRow("Container", "Matroska") }
composeRule.onNodeWithTag(TestTags.Converter.detailRow("Container"))
.assertTextEquals("Container: Matroska")
}
private fun setFileCard(input: InputFile) = composeRule.setContent { FileCard(input) }
private fun input(sizeBytes: Long? = 12_345_678L, probe: InputProbe? = VIDEO_PROBE) = InputFile(
uri = Uri.parse("content://test/clip.mkv"),
displayName = "clip.mkv",
sizeBytes = sizeBytes,
probe = probe,
)
private fun assertRow(label: String, value: String) {
composeRule.onNodeWithTag(TestTags.Converter.detailRow(label))
.assertTextEquals("$label: $value")
}
private fun assertNoRow(label: String) {
composeRule.onNodeWithTag(TestTags.Converter.detailRow(label)).assertDoesNotExist()
}
private fun assertNoDetailRows() = EVERY_ROW_LABEL.forEach(::assertNoRow)
private companion object {
/** Every label the four kind branches can emit, so absence can be asserted exhaustively. */
val EVERY_ROW_LABEL = listOf("Container", "Video", "Audio", "Length", "Type", "Size")
val VIDEO_PROBE = InputProbe(
videoCodec = "h264",
audioCodec = "aac",
durationMs = 90_000,
kind = InputKind.VIDEO,
container = Container.MP4,
width = 1920,
height = 1080,
)
}
}