From 2bed40d0806c4e7aa28dd8a9790bc733f01f8c26 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Mon, 24 Aug 2026 15:58:48 -0500 Subject: [PATCH] Hold the three pickers to the constant they hand back The format, quality and engine pickers are the same dozen lines with a different enum substituted, and both ways they can go wrong are silent. An onClick that closes over the picker's `selected` parameter instead of the chip's own entry returns one constant for every chip; an inverted `entry == selected` lights every chip but the right one. Neither throws, neither changes the labels on screen, and a test that only asserted the callback ran would pass over the first of them. So each click test presses every chip in the row and compares the whole recorded list against `entries`, which makes the constant load-bearing rather than the click count, and each selection test asserts over every chip rather than only the one that should be lit. Verified by mutation, not by the suite going green: - `onSelect(format)` -> `onSelect(OutputFormat.MP4_H264)` fails with `expected:<[MP4_H264, MP4_H265, WEBM_VP9, ...]> but was:<[MP4_H264, MP4_H264, MP4_H264, ...]>` - `format == selected` -> `format != selected` fails both format selection tests on `Selected = 'true'` for a chip that should not be - `onSelect(preference)` -> `{}` fails with `expected:<[AUTO, PREFER_HARDWARE, FORCE_SOFTWARE]> but was:<[]>` - `selected.description` -> `QualityTier.FAST.description` fails the quality prose test on the missing BEST line Labels are read off the enums so a reword cannot redden this file for the wrong reason. `EnginePreference` has no label of its own, so the screen's own `label()` supplies that set. The one display literal with no symbol behind it, the custom-spec line, was copied out of the source byte for byte because it holds a U+2014 that would fail silently if retyped. Co-Authored-By: Claude Opus 5 (1M context) --- .../convert/ConverterPickerSelectionTest.kt | 169 ++++++++++++++++++ 1 file changed, 169 insertions(+) create mode 100644 app/src/test/java/org/libremediaconverter/convert/ConverterPickerSelectionTest.kt diff --git a/app/src/test/java/org/libremediaconverter/convert/ConverterPickerSelectionTest.kt b/app/src/test/java/org/libremediaconverter/convert/ConverterPickerSelectionTest.kt new file mode 100644 index 0000000..3fdbbb6 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/convert/ConverterPickerSelectionTest.kt @@ -0,0 +1,169 @@ +package org.libremediaconverter.convert + +import androidx.compose.ui.test.SemanticsNodeInteraction +import androidx.compose.ui.test.assertIsNotSelected +import androidx.compose.ui.test.assertIsSelected +import androidx.compose.ui.test.hasAnyAncestor +import androidx.compose.ui.test.hasTestTag +import androidx.compose.ui.test.hasText +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.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.libremediaconverter.createDrainedComposeRule +import org.libremediaconverter.model.EnginePreference +import org.libremediaconverter.model.OutputFormat +import org.libremediaconverter.model.QualityTier +import org.libremediaconverter.ui.TestTags +import org.robolectric.RobolectricTestRunner + +/** + * Each picker lights the chip it was handed and reports the constant that was pressed. + * + * The defect this bites on is a picker that renders perfectly and answers wrongly. All three are + * the same dozen lines with a different enum substituted, so the failure mode is a copy-paste that + * survives review: an `onClick` that closes over the picker's `selected` parameter instead of the + * chip's own entry hands back one constant no matter which chip was tapped, and an inverted + * `entry == selected` lights every chip except the right one. Neither throws, neither changes the + * set of labels on screen, and a test that only asserted "the callback ran" would pass over both. + * + * Clicking every chip in turn and comparing the whole recorded list against `entries` is what makes + * the constant load-bearing rather than the click count -- a hardcoded `onSelect` fires the same + * number of times as a correct one. Selection is asserted over every chip for the same reason: the + * one that should be lit proves nothing on its own, because `!=` lights it too whenever the enum + * has exactly one entry, and lights all its siblings whenever it has more. + * + * Labels come from `OutputFormat.label` and `QualityTier.label`; [label], which the screen owns + * because `EnginePreference` carries no label of its own, supplies the third set. Retyping any of + * them here would turn a rename into a red test that named the wrong cause. + * + * Not covered, deliberately: the `"Output format"`, `"Quality"` and `"Engine"` headings, which are + * untagged `Text` calls with no enum behind them and no behaviour to bite on. + */ +@UnstableApi +@RunWith(RobolectricTestRunner::class) +class ConverterPickerSelectionTest { + + @get:Rule + val composeRule = createDrainedComposeRule() + + /** + * The chip carrying [label] inside the row tagged [rowTag]. + * + * By ancestor rather than by direct child: how many semantics nodes Material 3 puts between a + * `FlowRow` and its chips is that library's business, and a matcher that assumed "one" would + * break on an upgrade that changed nothing this test is about. + */ + private fun chipIn(rowTag: String, label: String): SemanticsNodeInteraction = + composeRule.onNode(hasAnyAncestor(hasTestTag(rowTag)) and hasText(label)) + + private fun assertOnlySelected(rowTag: String, labels: List, selected: String?) { + labels.forEach { label -> + val chip = chipIn(rowTag, label) + if (label == selected) chip.assertIsSelected() else chip.assertIsNotSelected() + } + } + + @Test + fun `the format picker lights the selected format and no other`() { + composeRule.setContent { FormatPicker(OutputFormat.WEBM_VP9) {} } + + assertOnlySelected( + rowTag = TestTags.Converter.FORMAT_CHIPS, + labels = OutputFormat.entries.map { it.label }, + selected = OutputFormat.WEBM_VP9.label, + ) + } + + /** A spec no preset can express lights nothing, which is what the custom line stands in for. */ + @Test + fun `the format picker lights nothing when the spec is custom`() { + composeRule.setContent { FormatPicker(null) {} } + + assertOnlySelected( + rowTag = TestTags.Converter.FORMAT_CHIPS, + labels = OutputFormat.entries.map { it.label }, + selected = null, + ) + composeRule.onNodeWithText(CUSTOM_SPEC_NOTE).assertExists() + } + + @Test + fun `a selected format hides the custom line`() { + composeRule.setContent { FormatPicker(OutputFormat.MP3) {} } + + composeRule.onNodeWithText(CUSTOM_SPEC_NOTE).assertDoesNotExist() + } + + @Test + fun `clicking a format chip reports that format`() { + val picked = mutableListOf() + composeRule.setContent { FormatPicker(null) { picked += it } } + + OutputFormat.entries.forEach { chipIn(TestTags.Converter.FORMAT_CHIPS, it.label).performClick() } + + assertEquals(OutputFormat.entries.toList(), picked) + } + + @Test + fun `the quality picker lights the selected tier and no other`() { + composeRule.setContent { QualityPicker(QualityTier.BEST) {} } + + assertOnlySelected( + rowTag = TestTags.Converter.QUALITY_CHIPS, + labels = QualityTier.entries.map { it.label }, + selected = QualityTier.BEST.label, + ) + } + + /** The line under the chips describes what was chosen, not whichever tier was written first. */ + @Test + fun `the quality picker explains the tier that is selected`() { + composeRule.setContent { QualityPicker(QualityTier.BEST) {} } + + composeRule.onNodeWithText(QualityTier.BEST.description).assertExists() + composeRule.onNodeWithText(QualityTier.FAST.description).assertDoesNotExist() + } + + @Test + fun `clicking a quality chip reports that tier`() { + val picked = mutableListOf() + composeRule.setContent { QualityPicker(QualityTier.FAST) { picked += it } } + + QualityTier.entries.forEach { chipIn(TestTags.Converter.QUALITY_CHIPS, it.label).performClick() } + + assertEquals(QualityTier.entries.toList(), picked) + } + + @Test + fun `the engine picker lights the selected preference and no other`() { + composeRule.setContent { EnginePicker(EnginePreference.FORCE_SOFTWARE) {} } + + assertOnlySelected( + rowTag = TestTags.Converter.ENGINE_CHIPS, + labels = EnginePreference.entries.map { it.label() }, + selected = EnginePreference.FORCE_SOFTWARE.label(), + ) + } + + @Test + fun `clicking an engine chip reports that preference`() { + val picked = mutableListOf() + composeRule.setContent { EnginePicker(EnginePreference.AUTO) { picked += it } } + + EnginePreference.entries.forEach { chipIn(TestTags.Converter.ENGINE_CHIPS, it.label()).performClick() } + + assertEquals(EnginePreference.entries.toList(), picked) + } + + private companion object { + /** + * Copied byte for byte out of `ConverterScreen.kt` -- it holds a U+2014 em dash, which + * retyped as ASCII would match nothing and fail as "no node found" rather than as a reword. + */ + const val CUSTOM_SPEC_NOTE: String = "Custom — set below." + } +} -- 2.47.3