Compare commits

...
Author SHA1 Message Date
JMR-devandClaude Opus 5 eded47d666 B1 (#173): tell the rail from the bottom bar, and the Convert tab from the Join tab
Two assertion gaps, not coverage gaps, which is why they lasted. AppRootRestorationTest
already drives AppRoot at Compact and Expanded, so JaCoCo is green on useRail -- but it
asserts only that the selected tab survives recreation, through a stub `content`
composable. Nothing anywhere queried for a rail or a bar, and nothing composed the real
screens. Measured before this file existed:

  - transposing the NavigationRail and NavigationBar bodies passed the entire suite
  - transposing Content's two arms passed it too

A tablet showing phone chrome, or the Convert tab opening the Join screen, and 546 tests
with nothing to say about either. AppRoot's own KDoc is why that matters more than it
looks: from targetSdk 37 the app is resized and rotated whether or not it is ready, so the
width class is not a preference.

WindowWidthSizeClass.Medium appears in no test in either source set today. useRail is
`!= Compact`, so Medium takes the rail; narrowing it to `== Expanded` is one character and
breaks every tablet and unfolded foldable. That mutation is red now, and it is red only
because of the Medium test -- the Compact and Expanded ones both survive it.

Two things this needed:

**createAndroidComposeRule rather than createComposeRule.** Rendering AppRoot with its
default content reaches ConverterScreen's `viewModel = viewModel()`, which needs a
ViewModelStoreOwner. It works because both ViewModels are
`@JvmOverloads constructor(app: Application, ...)` so AndroidViewModelFactory can build
them, and because ui-test-manifest's debugImplementation entry already puts a
ComponentActivity in the merged manifest the unit tests build against -- which
app/build.gradle.kts says in terms. Checked with a throwaway spike before the ticket was
filed, rather than discovered here.

**Two tags, applied inside main.** The only production change: TestTags.Shell, set on the
rail and the bar. There is no other way to tell the two apart -- both render the same two
destinations with the same labels and the same selection state, so any assertion writable
without them is satisfied by either layout. In TestTags and applied by the shell rather
than handed down by the test, for the reason that file's KDoc gives: a tag the test
supplies proves only that the test set it. TagTableUniquenessTest covers the new group.

564 -> 568 JVM tests, 0 failures.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 21:54:50 -05:00
JMR-devandClaude Opus 5 b41341a1cb B3 (#175): pin the AAC arm every ordinary conversion takes
audioArgs has six arms. Five are named codecs with tests; AAC arrives through the `else`,
so nothing named it -- neither "aac" nor "192k" appeared anywhere in
FFmpegCommandBuilderTest. It is the audio MP4 and M4A get, which is to say the audio the
picker offers first and most conversions produce.

Both halves are asserted, and the bitrate is the half worth arguing for: an -b:a that
quietly changed would fail nothing, look wrong in no command line, and surface only as
files that sound different from the ones the app produced last month. Both mutations
confirmed red -- 192k -> 128k and aac -> libfdk_aac.

Asserted through MP4_H264 and M4A_AAC rather than one of them, so an AAC arm added above
the `else` later has to keep answering the same way for both.

**Deliberately not added here: an ENCODABLE_AUDIO-vs-audioArgs agreement test**, the
obvious companion to VideoCodecMimeAgreementTest. It would freeze the answer to F1, which
is open: ContainerCapabilities.kt:84 says "nothing here emits a Vorbis encoder" and
FFmpegCommandBuilder.kt:188 does. docs/coverage-read-findings.md says in terms that the
tempting fix there locks in the wrong answer and that the decision comes first. This is
the AAC arm only.

563 -> 564 JVM tests, 0 failures. No production code changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 21:50:31 -05:00
5 changed files with 172 additions and 2 deletions
@@ -24,9 +24,11 @@ import androidx.compose.runtime.saveable.Saver
import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.testTag
import androidx.media3.common.util.UnstableApi
import org.libremediaconverter.convert.ConverterScreen
import org.libremediaconverter.join.JoinScreen
import org.libremediaconverter.ui.TestTags
import org.libremediaconverter.ui.theme.LibreMediaConverterTheme
/**
@@ -114,7 +116,7 @@ internal fun AppRoot(
if (useRail) {
Row(modifier = Modifier.fillMaxSize()) {
NavigationRail {
NavigationRail(modifier = Modifier.testTag(TestTags.Shell.NAVIGATION_RAIL)) {
Destination.entries.forEach { item ->
NavigationRailItem(
selected = destination == item,
@@ -132,7 +134,7 @@ internal fun AppRoot(
Scaffold(
modifier = Modifier.fillMaxSize(),
bottomBar = {
NavigationBar {
NavigationBar(modifier = Modifier.testTag(TestTags.Shell.NAVIGATION_BAR)) {
Destination.entries.forEach { item ->
NavigationBarItem(
selected = destination == item,
@@ -56,6 +56,20 @@ object TestTags {
*/
const val RETRY_SAVE: String = "action.retrySave"
/**
* The adaptive shell around both screens -- `AppRoot`'s two layouts.
*
* Named because there is no other way to tell them apart from a test. Both render the same two
* destinations with the same labels and the same selection state, so every assertion that could
* be written without these tags is satisfied by either layout, and transposing the two bodies
* passed the whole suite. Exactly one of the two exists at a time, which is what makes
* `assertExists` / `assertDoesNotExist` on this pair a statement about the width class.
*/
object Shell {
const val NAVIGATION_RAIL: String = "shell.navigationRail"
const val NAVIGATION_BAR: String = "shell.navigationBar"
}
/** `ConverterScreen`. */
object Converter {
const val CHOOSE_FILE: String = "converter.chooseFile"
@@ -0,0 +1,129 @@
package org.libremediaconverter
import androidx.activity.ComponentActivity
import androidx.compose.material3.windowsizeclass.WindowWidthSizeClass
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
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 androidx.work.Data
import org.junit.After
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.installTestWorkManager
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.ui.TestTags
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
/**
* Which navigation affordance the shell actually renders, and which screen it actually shows.
*
* Assertion gaps rather than coverage gaps, both of them, and that is why they lasted.
* `AppRootRestorationTest` already drives `AppRoot` at `Compact` and `Expanded`, so JaCoCo is green
* on `useRail` -- but it asserts only that the *selected tab* survives recreation, through a stub
* `content` composable. Nothing anywhere queried for a rail or a bar, and nothing rendered the real
* screens. Two consequences, both measured before this file existed:
*
* - **Transposing the `NavigationRail` and `NavigationBar` bodies passed the entire suite.**
* - **Transposing `Content`'s two arms passed it too** -- a tablet showing the phone chrome, or the
* Convert tab opening the Join screen, and 546 tests with nothing to say about either.
*
* `AppRoot`'s own KDoc is why this matters more than it looks: from targetSdk 37 the app is resized
* and rotated whether or not it is ready, so the width class is not a preference, it is whatever
* the system hands over.
*
* ## Two things this needed that the rest of the suite does not
*
* **`createAndroidComposeRule`, not `createComposeRule`.** Rendering `AppRoot` with its *default*
* content reaches `ConverterScreen`'s `viewModel = viewModel()`, which needs a
* `ViewModelStoreOwner`; the plain rule supplies none. It works because both ViewModels are
* `@JvmOverloads constructor(app: Application, …)`, so `AndroidViewModelFactory` can build them,
* and because `app/build.gradle.kts` already puts `ui-test-manifest`'s `ComponentActivity` in the
* merged manifest the unit tests build against -- which that file says in terms.
*
* **Tags on the two bars.** They are in `TestTags`, applied inside `main`, for the reason that
* file's KDoc gives: a tag the test hands down proves only that the test set it.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class AdaptiveShellTest {
@get:Rule
val composeRule = createAndroidComposeRule<ComponentActivity>()
@Before
fun setUp() {
val app = RuntimeEnvironment.getApplication()
installTestWorkManager(app, Data.EMPTY)
// The real screens are composed here, so their ViewModels are real too. Neither test is
// about probing or publishing; left alone they would reach the FFprobe loader and this
// machine's codec list, and decide things no assertion mentions.
ConversionDependencies.probe = { _, _ -> InputProbe() }
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
@Test
fun `a phone gets the bottom bar and a tablet gets the rail`() {
setShell(WindowWidthSizeClass.Compact)
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertExists()
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertDoesNotExist()
}
@Test
fun `an expanded window gets the rail`() {
setShell(WindowWidthSizeClass.Expanded)
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertExists()
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertDoesNotExist()
}
/**
* The width class no test had ever passed.
*
* `useRail` is `!= Compact`, so Medium takes the rail with Expanded. Narrowing it to
* `== Expanded` is a one-character change that breaks every tablet and unfolded foldable and
* nothing else -- and until this test, nothing in either source set used `Medium` at all.
*/
@Test
fun `a medium window is a rail window, not a phone`() {
setShell(WindowWidthSizeClass.Medium)
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_RAIL).assertExists()
composeRule.onNodeWithTag(TestTags.Shell.NAVIGATION_BAR).assertDoesNotExist()
}
/**
* The mapping every other test stubs out: which screen each destination actually opens.
*
* Matched on each screen's own "choose a file" affordance rather than on a title, because those
* tags are applied by the screens themselves -- so this fails if the destinations are
* transposed, and it fails for the right reason.
*/
@Test
fun `Convert opens the converter and Join opens the join screen`() {
setShell(WindowWidthSizeClass.Compact)
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).assertExists()
composeRule.onNodeWithTag(TestTags.Join.CHOOSE_FILES).assertDoesNotExist()
composeRule.onNodeWithText(Destination.JOIN.label).performClick()
composeRule.onNodeWithTag(TestTags.Join.CHOOSE_FILES).assertExists()
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).assertDoesNotExist()
}
/** [AppRoot] with its real content, which is the half nothing else composes. */
private fun setShell(width: WindowWidthSizeClass) {
composeRule.setContent { AppRoot(width) }
}
}
@@ -166,6 +166,30 @@ class FFmpegCommandBuilderTest {
assertPair(cmd(OutputFormat.OPUS), "-c:a", "libopus")
}
/**
* The arm most conversions actually take, and the only one in `audioArgs` with no test.
*
* `flac wav and opus select the right encoders` above covers the three named arms; MP3 has its
* own. AAC arrives through the `else`, so nothing named it and nothing pinned either half of
* what it emits -- neither `aac` nor `192k` appeared anywhere in this file. Both are shipped
* defaults: MP4 and M4A are the formats the picker offers first, so this is the audio
* every ordinary conversion gets.
*
* The bitrate is asserted as well as the encoder because it is the half a refactor is likelier
* to lose. An `-b:a` that quietly changed would not fail anything, would not look wrong in a
* command line, and would show up only as files that sound different from the ones the app
* produced last month.
*/
@Test
fun `aac is the default encoder, at the bitrate the app ships`() {
assertPair(cmd(OutputFormat.MP4_H264), "-c:a", "aac")
assertPair(cmd(OutputFormat.MP4_H264), "-b:a", "192k")
// Through the `else` rather than through a named arm, so an AAC branch added above it later
// has to keep answering the same way.
assertPair(cmd(OutputFormat.M4A_AAC), "-c:a", "aac")
assertPair(cmd(OutputFormat.M4A_AAC), "-b:a", "192k")
}
@Test
fun `audio only formats never carry a video encoder`() {
listOf(OutputFormat.MP3, OutputFormat.FLAC, OutputFormat.WAV, OutputFormat.OPUS)
@@ -28,6 +28,7 @@ class TagTableUniquenessTest {
fun `every tag constant has its own value`() {
val tags = tagsIn(
TestTags::class.java,
TestTags.Shell::class.java,
TestTags.Converter::class.java,
TestTags.Join::class.java,
)