diff --git a/app/src/test/java/org/libremediaconverter/convert/AdvancedPanelSavedStateTest.kt b/app/src/test/java/org/libremediaconverter/convert/AdvancedPanelSavedStateTest.kt new file mode 100644 index 0000000..4961381 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/convert/AdvancedPanelSavedStateTest.kt @@ -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 = 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" + } +} diff --git a/app/src/test/java/org/libremediaconverter/convert/AdvancedPickerTest.kt b/app/src/test/java/org/libremediaconverter/convert/AdvancedPickerTest.kt new file mode 100644 index 0000000..342b54e --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/convert/AdvancedPickerTest.kt @@ -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() + private val videoCodecs = mutableListOf() + private val audioCodecs = mutableListOf() + private val applied = mutableListOf() + + // --- 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(), 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, + ) + } +}