diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 5db1707..66bedbe 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -290,6 +290,22 @@ dependencies { // drive a ViewModel through a real WorkManager to SUCCEEDED, which is where the cleanup // handle is set -- the wiring the leak actually lived in. testImplementation(libs.androidx.work.testing) + // Compose's own test rules, on the JVM source set as well as androidTest. Already in the + // catalog, already inside the prerelease guard via its androidx. group, and versioned by + // the BOM, so this adds no new pinning argument. + // + // Here rather than only in androidTest because ui-test-junit4 runs under Robolectric: + // createComposeRule() drives a real composition on the JVM. The defect it was added for + // -- the selected tab not surviving recreation -- is caught by StateRestorationTester, + // and putting that test where the instrumented suite lives would mean nobody on this + // host could ever watch it go red. + // + // ui-test-manifest is deliberately NOT repeated here. It supplies the ComponentActivity + // the rule launches, and the debugImplementation entry below already puts it in the + // merged manifest the unit tests build against -- checked by removing it and watching + // the tests stay green. + testImplementation(platform(libs.compose.bom)) + testImplementation(libs.compose.ui.test.junit4) androidTestImplementation(platform(libs.compose.bom)) androidTestImplementation(libs.androidx.junit) diff --git a/app/src/main/java/org/libremediaconverter/MainActivity.kt b/app/src/main/java/org/libremediaconverter/MainActivity.kt index 6fd219f..b03f7c1 100644 --- a/app/src/main/java/org/libremediaconverter/MainActivity.kt +++ b/app/src/main/java/org/libremediaconverter/MainActivity.kt @@ -20,7 +20,8 @@ import androidx.compose.material3.windowsizeclass.calculateWindowSizeClass import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.Saver +import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.media3.common.util.UnstableApi @@ -28,11 +29,41 @@ import org.libremediaconverter.convert.ConverterScreen import org.libremediaconverter.join.JoinScreen import org.libremediaconverter.ui.theme.LibreMediaConverterTheme -private enum class Destination(val label: String) { +/** + * The tabs of the adaptive shell. + * + * `internal` rather than `private` so the unit tests can name a tab. The JVM test source + * set is a friend of `main`, so this stays invisible to anything outside the module. + */ +internal enum class Destination(val label: String) { CONVERT("Convert"), JOIN("Join"), } +/** + * Saves a [Destination] as its constant name. + * + * A saver is needed at all because `rememberSaveable`'s default only accepts what a + * `Bundle` can hold. An enum does qualify -- it is `Serializable`, so `autoSaver` would take + * it without complaint -- and that is the reason to be explicit rather than the reason not + * to be: nothing in the declaration says this type has to stay `Serializable`, so the + * implicit route would keep working until someone made it a value class or a sealed + * interface, and then quietly stop. + * + * The name and not the ordinal. An ordinal is a position, so inserting a tab between the + * existing two would silently redefine every value already written down; the name only + * changes when someone renames a constant, which is a visible edit. It also reads as itself + * in a `Bundle` dump. + * + * An unknown name restores to null, which `rememberSaveable` treats as "nothing saved" and + * falls back to the default tab. That is the state a downgrade or a renamed constant + * produces, and landing on Convert is the right answer for it. + */ +internal val DestinationSaver: Saver = Saver( + save = { it.name }, + restore = { name -> Destination.entries.firstOrNull { it.name == name } }, +) + @UnstableApi class MainActivity : ComponentActivity() { @@ -57,12 +88,28 @@ class MainActivity : ComponentActivity() { * `resizableActivity` and aspect-ratio limits on any display at least 600dp wide, and * the Android 16 opt-out no longer applies. The app will be resized and rotated * whether or not it is ready, so it has to lay out properly at every width. + * + * [content] is a parameter with a default rather than a direct call to [Content] so a test + * can drive the shell -- which tab is selected, and whether that survives recreation -- + * without standing up either screen. Both screens resolve a ViewModel, which builds a + * WorkManager and a media probe, none of which the tab selection depends on. The app + * itself never passes it. */ @UnstableApi @OptIn(ExperimentalMaterial3Api::class) @Composable -private fun AppRoot(widthSizeClass: WindowWidthSizeClass) { - var destination by remember { mutableStateOf(Destination.CONVERT) } +internal fun AppRoot( + widthSizeClass: WindowWidthSizeClass, + content: @Composable (Destination, Modifier) -> Unit = { destination, modifier -> + Content(destination, modifier) + }, +) { + // rememberSaveable, NOT remember. MainActivity declares no configChanges, so every + // rotation and every resize recreates it -- exactly the case the KDoc above says the + // shell exists for -- and remember does not survive that. + var destination by rememberSaveable(stateSaver = DestinationSaver) { + mutableStateOf(Destination.CONVERT) + } val useRail = widthSizeClass != WindowWidthSizeClass.Compact if (useRail) { @@ -78,7 +125,7 @@ private fun AppRoot(widthSizeClass: WindowWidthSizeClass) { } } Scaffold(modifier = Modifier.fillMaxSize()) { padding -> - Content(destination, Modifier.padding(padding)) + content(destination, Modifier.padding(padding)) } } } else { @@ -97,7 +144,7 @@ private fun AppRoot(widthSizeClass: WindowWidthSizeClass) { } }, ) { padding -> - Content(destination, Modifier.padding(padding)) + content(destination, Modifier.padding(padding)) } } } diff --git a/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt b/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt new file mode 100644 index 0000000..b0e1c78 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/AppRootRestorationTest.kt @@ -0,0 +1,113 @@ +package org.libremediaconverter + +import androidx.compose.foundation.layout.Box +import androidx.compose.material3.windowsizeclass.WindowWidthSizeClass +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.test.assertIsSelected +import androidx.compose.ui.test.junit4.StateRestorationTester +import androidx.compose.ui.test.junit4.v2.createComposeRule +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.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * The selected tab has to survive activity recreation, not just recomposition. + * + * `remember` covers recomposition only, and `MainActivity` declares no `configChanges`, so + * every rotation and every resize destroys and recreates the Activity. That is the exact + * case [AppRoot]'s own KDoc says the shell exists for: from targetSdk 37 the app is resized + * and rotated whether or not it is ready. + * + * [StateRestorationTester] is the tool for it -- `emulateSavedInstanceStateRestore()` + * disposes the composition and rebuilds it, so anything held only by `remember` is gone and + * only saved state comes back. It is Compose's own stand-in for the recreation rather than + * the real thing: it saves into an in-memory map instead of parcelling through a `Bundle`, + * so it proves `rememberSaveable` is being used -- not that a particular saved + * representation survives a `Bundle` round trip. A JVM round-trip test on the + * saver covers the representation. + * + * Robolectric rather than the instrumented suite, deliberately. The instrumented tests + * cannot run on the development host at all (see CLAUDE.md), and a red test nobody can + * execute is not a loop anyone can work in. + */ +@UnstableApi +@RunWith(RobolectricTestRunner::class) +class AppRootRestorationTest { + + @get:Rule + val composeRule = createComposeRule() + + private val restoration = StateRestorationTester(composeRule) + + /** + * The stub screen is matched by a test tag rather than by text: the label on the bar + * ("Join") and the enum constant ("JOIN") differ only in case, and a matcher that could + * pick up either is not an assertion. + */ + private fun tagFor(destination: Destination) = "content:${destination.name}" + + private fun assertShowing(destination: Destination) { + composeRule.onNodeWithTag(tagFor(destination)).assertExists() + composeRule.onNodeWithText(destination.label).assertIsSelected() + } + + private fun setShell(width: () -> WindowWidthSizeClass) { + restoration.setContent { + AppRoot(width()) { destination, modifier -> + Box(modifier.testTag(tagFor(destination))) + } + } + } + + @Test + fun `the selected tab survives recreation on a phone`() { + setShell { WindowWidthSizeClass.Compact } + assertShowing(Destination.CONVERT) + + composeRule.onNodeWithText(Destination.JOIN.label).performClick() + assertShowing(Destination.JOIN) + + restoration.emulateSavedInstanceStateRestore() + + assertShowing(Destination.JOIN) + } + + @Test + fun `the selected tab survives recreation on the rail layout`() { + setShell { WindowWidthSizeClass.Expanded } + + composeRule.onNodeWithText(Destination.JOIN.label).performClick() + assertShowing(Destination.JOIN) + + restoration.emulateSavedInstanceStateRestore() + + assertShowing(Destination.JOIN) + } + + /** + * The real rotation: the width class changes across the recreation, so the shell comes + * back as a rail where it went out as a bottom bar. The tab still has to be the one the + * user chose. + */ + @Test + fun `the selected tab survives a rotation that also changes the width class`() { + var width by mutableStateOf(WindowWidthSizeClass.Compact) + setShell { width } + + composeRule.onNodeWithText(Destination.JOIN.label).performClick() + assertShowing(Destination.JOIN) + + width = WindowWidthSizeClass.Expanded + restoration.emulateSavedInstanceStateRestore() + + assertShowing(Destination.JOIN) + } +} diff --git a/app/src/test/java/org/libremediaconverter/DestinationSaverTest.kt b/app/src/test/java/org/libremediaconverter/DestinationSaverTest.kt new file mode 100644 index 0000000..3f13363 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/DestinationSaverTest.kt @@ -0,0 +1,45 @@ +package org.libremediaconverter + +import androidx.compose.runtime.saveable.SaverScope +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * What [AppRootRestorationTest] cannot see. + * + * `StateRestorationTester` saves into an in-memory map, so it proves the shell uses + * `rememberSaveable` and stops there -- it would be just as green if the saved value were an + * ordinal, or if the enum were left to `autoSaver`. The saved *representation* is a separate + * decision with separate consequences, and this is where it is pinned. + */ +class DestinationSaverTest { + + /** `canBeSaved` is the host registry's question; a String always can. */ + private val scope = SaverScope { true } + + private fun save(destination: Destination): Any? = with(DestinationSaver) { scope.save(destination) } + + @Test + fun `a destination is saved as its constant name, not its position`() { + // JOIN is ordinal 1. If this ever reads `1`, inserting a tab above it silently + // redefines every value already saved. + assertEquals("CONVERT", save(Destination.CONVERT)) + assertEquals("JOIN", save(Destination.JOIN)) + } + + @Test + fun `every destination survives the round trip`() { + Destination.entries.forEach { destination -> + assertEquals(destination, DestinationSaver.restore(save(destination) as String)) + } + } + + @Test + fun `a name no longer in the enum restores to nothing`() { + // A downgrade, or a renamed constant, leaves a name that no longer resolves. + // Returning null is what makes rememberSaveable fall back to the default tab + // instead of throwing on the way back from a rotation. + assertNull(DestinationSaver.restore("SETTINGS")) + } +}