diff --git a/app/src/main/java/org/libremediaconverter/MainActivity.kt b/app/src/main/java/org/libremediaconverter/MainActivity.kt index b03f7c1..48f758c 100644 --- a/app/src/main/java/org/libremediaconverter/MainActivity.kt +++ b/app/src/main/java/org/libremediaconverter/MainActivity.kt @@ -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, diff --git a/app/src/main/java/org/libremediaconverter/ui/TestTags.kt b/app/src/main/java/org/libremediaconverter/ui/TestTags.kt index f6fe883..8a3e0eb 100644 --- a/app/src/main/java/org/libremediaconverter/ui/TestTags.kt +++ b/app/src/main/java/org/libremediaconverter/ui/TestTags.kt @@ -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" diff --git a/app/src/test/java/org/libremediaconverter/AdaptiveShellTest.kt b/app/src/test/java/org/libremediaconverter/AdaptiveShellTest.kt new file mode 100644 index 0000000..9bdfb8d --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/AdaptiveShellTest.kt @@ -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() + + @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) } + } +} diff --git a/app/src/test/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilderTest.kt b/app/src/test/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilderTest.kt index 936717a..9f9624d 100644 --- a/app/src/test/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilderTest.kt +++ b/app/src/test/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilderTest.kt @@ -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) diff --git a/app/src/test/java/org/libremediaconverter/ui/TagTableUniquenessTest.kt b/app/src/test/java/org/libremediaconverter/ui/TagTableUniquenessTest.kt index 333b051..bd59f2f 100644 --- a/app/src/test/java/org/libremediaconverter/ui/TagTableUniquenessTest.kt +++ b/app/src/test/java/org/libremediaconverter/ui/TagTableUniquenessTest.kt @@ -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, ) diff --git a/app/src/test/java/org/libremediaconverter/work/NotificationProgressTextTest.kt b/app/src/test/java/org/libremediaconverter/work/NotificationProgressTextTest.kt new file mode 100644 index 0000000..7bbef1c --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/work/NotificationProgressTextTest.kt @@ -0,0 +1,89 @@ +package org.libremediaconverter.work + +import android.app.Notification +import androidx.media3.common.util.UnstableApi +import androidx.work.Data +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.libremediaconverter.convert.installTestWorkManager +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import java.util.UUID + +/** + * The two things a progress notification can say, and that they are not the same thing. + * + * An assertion gap rather than a coverage one, and the distinction is the reason this file exists. + * JaCoCo is green on `build`'s `if (indeterminate)`, because `ProgressNotificationTest` drives it + * through a real worker -- but that test reads only the notification id and + * `Notification.EXTRA_PROGRESS`. **Nothing had ever read the text.** Swapping the two branches, or + * collapsing them into one string, passed the entire suite. + * + * What it costs to get wrong is small and constant: a conversion that has been running for four + * minutes still saying "Preparing", or one that has not started reporting yet claiming 0%. Neither + * is a crash, and neither would be found by anything else here -- which is exactly the kind of + * thing that survives for a long time. + * + * Nothing else in the suite constructs [ConversionNotifications] directly. + */ +@UnstableApi +@RunWith(RobolectricTestRunner::class) +class NotificationProgressTextTest { + + /** + * `build` reaches `WorkManager.getInstance` for the Cancel action's PendingIntent, so the + * notification cannot be built at all without one. That coupling is why nothing had ever + * constructed this class directly and read what it produced. + */ + @Before + fun setUp() { + installTestWorkManager(RuntimeEnvironment.getApplication(), Data.EMPTY) + } + + @Test + fun `an indeterminate notification says something different from a measured one`() { + val context = RuntimeEnvironment.getApplication() + val notifications = ConversionNotifications(context) + + val preparing = notifications.build(JOB_ID, TITLE, percent = 0, indeterminate = true).text() + val measured = notifications.build(JOB_ID, TITLE, percent = 42, indeterminate = false).text() + + assertNotEquals( + "the two states have to read differently, or the text says nothing at all", + preparing, + measured, + ) + assertTrue( + "a measured notification has to carry its percentage, got \"$measured\"", + measured.contains("42"), + ) + assertTrue( + "an indeterminate one must not invent one, got \"$preparing\"", + !preparing.contains("42") && !preparing.contains("0"), + ) + } + + /** + * The title is the caller's, not the builder's -- it is the file the user picked, and it is what + * tells two simultaneous conversions apart in the shade. + */ + @Test + fun `the notification is titled with the file it is converting`() { + val context = RuntimeEnvironment.getApplication() + + val built = ConversionNotifications(context).build(JOB_ID, TITLE, percent = 10) + + assertEquals(TITLE, built.extras.getString(Notification.EXTRA_TITLE)) + } + + private fun Notification.text(): String = extras.getString(Notification.EXTRA_TEXT).orEmpty() + + private companion object { + const val TITLE = "holiday.mp4" + val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-00000000000a") + } +}