diff --git a/app/src/main/java/org/libremediaconverter/convert/ConverterScreen.kt b/app/src/main/java/org/libremediaconverter/convert/ConverterScreen.kt index 921b348..bfdf9c8 100644 --- a/app/src/main/java/org/libremediaconverter/convert/ConverterScreen.kt +++ b/app/src/main/java/org/libremediaconverter/convert/ConverterScreen.kt @@ -33,6 +33,7 @@ import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.testTag import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp import androidx.lifecycle.compose.collectAsStateWithLifecycle @@ -51,6 +52,7 @@ import org.libremediaconverter.model.VideoCodec import org.libremediaconverter.ui.PrimaryButtonHeight import org.libremediaconverter.ui.ScreenPaddingHorizontal import org.libremediaconverter.ui.ScreenPaddingVertical +import org.libremediaconverter.ui.TestTags import java.util.Locale @UnstableApi @@ -116,7 +118,8 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode onClick = { pickInput.launch(arrayOf("*/*")) }, modifier = Modifier .fillMaxWidth() - .height(PrimaryButtonHeight), + .height(PrimaryButtonHeight) + .testTag(TestTags.Converter.CHOOSE_FILE), ) { Text("Choose file") } } @@ -147,11 +150,16 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode // The Advanced picker lets an impossible combination be selected on // purpose, so this is what stops it from being run. enabled = validation.isValid, - modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight), + modifier = Modifier + .fillMaxWidth() + .height(PrimaryButtonHeight) + .testTag(TestTags.Converter.CONVERT), ) { Text("Convert") } OutlinedButton( onClick = { pickInput.launch(arrayOf("*/*")) }, - modifier = Modifier.fillMaxWidth(), + modifier = Modifier + .fillMaxWidth() + .testTag(TestTags.Converter.CHOOSE_DIFFERENT_FILE), ) { Text("Choose a different file") } } @@ -160,11 +168,13 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode Text("Converting… ${s.percent}%") LinearProgressIndicator( progress = { s.percent / 100f }, - modifier = Modifier.fillMaxWidth(), + modifier = Modifier + .fillMaxWidth() + .testTag(TestTags.Converter.PROGRESS), ) OutlinedButton( onClick = viewModel::cancel, - modifier = Modifier.fillMaxWidth(), + modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL), ) { Text("Cancel") } } @@ -183,7 +193,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode ) OutlinedButton( onClick = viewModel::cancel, - modifier = Modifier.fillMaxWidth(), + modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL), ) { Text("Cancel") } } @@ -202,11 +212,14 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode } Button( onClick = { chooseDestination.launch(s.suggestedName) }, - modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight), + modifier = Modifier + .fillMaxWidth() + .height(PrimaryButtonHeight) + .testTag(TestTags.SAVE_FILE), ) { Text("Save file") } OutlinedButton( onClick = viewModel::reset, - modifier = Modifier.fillMaxWidth(), + modifier = Modifier.fillMaxWidth().testTag(TestTags.START_OVER), ) { Text("Start over") } } @@ -214,7 +227,10 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode Text("Saved ${s.displayName}.", style = MaterialTheme.typography.bodyLarge) Button( onClick = viewModel::reset, - modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight), + modifier = Modifier + .fillMaxWidth() + .height(PrimaryButtonHeight) + .testTag(TestTags.Converter.CONVERT_ANOTHER), ) { Text("Convert another") } } @@ -226,7 +242,10 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode ) Button( onClick = viewModel::reset, - modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight), + modifier = Modifier + .fillMaxWidth() + .height(PrimaryButtonHeight) + .testTag(TestTags.START_OVER), ) { Text("Start over") } } } @@ -237,9 +256,12 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode @OptIn(ExperimentalLayoutApi::class) @Composable -private fun FormatPicker(selected: OutputFormat?, onSelect: (OutputFormat) -> Unit) { +internal fun FormatPicker(selected: OutputFormat?, onSelect: (OutputFormat) -> Unit) { Text("Output format", style = MaterialTheme.typography.titleSmall) - FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + FlowRow( + horizontalArrangement = Arrangement.spacedBy(8.dp), + modifier = Modifier.testTag(TestTags.Converter.FORMAT_CHIPS), + ) { OutputFormat.entries.forEach { format -> FilterChip( selected = format == selected, @@ -267,7 +289,7 @@ private fun FormatPicker(selected: OutputFormat?, onSelect: (OutputFormat) -> Un */ @OptIn(ExperimentalLayoutApi::class) @Composable -private fun AdvancedPicker( +internal fun AdvancedPicker( spec: OutputSpec, validation: Validation, onContainer: (Container) -> Unit, @@ -277,14 +299,23 @@ private fun AdvancedPicker( ) { var expanded by rememberSaveable { mutableStateOf(false) } - TextButton(onClick = { expanded = !expanded }) { + TextButton( + onClick = { expanded = !expanded }, + modifier = Modifier.testTag(TestTags.Converter.ADVANCED_TOGGLE), + ) { Text(if (expanded) "Hide advanced" else "Advanced") } AnimatedVisibility(visible = expanded) { - Column(verticalArrangement = Arrangement.spacedBy(12.dp)) { + Column( + verticalArrangement = Arrangement.spacedBy(12.dp), + modifier = Modifier.testTag(TestTags.Converter.ADVANCED_PANEL), + ) { Text("Container", style = MaterialTheme.typography.titleSmall) - FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + FlowRow( + horizontalArrangement = Arrangement.spacedBy(8.dp), + modifier = Modifier.testTag(TestTags.Converter.ADVANCED_CONTAINER_CHIPS), + ) { Container.entries.forEach { container -> FilterChip( selected = container == spec.container, @@ -295,7 +326,10 @@ private fun AdvancedPicker( } Text("Video", style = MaterialTheme.typography.titleSmall) - FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + FlowRow( + horizontalArrangement = Arrangement.spacedBy(8.dp), + modifier = Modifier.testTag(TestTags.Converter.ADVANCED_VIDEO_CHIPS), + ) { VideoCodec.entries.forEach { codec -> FilterChip( selected = codec == spec.videoCodec, @@ -306,7 +340,10 @@ private fun AdvancedPicker( } Text("Audio", style = MaterialTheme.typography.titleSmall) - FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + FlowRow( + horizontalArrangement = Arrangement.spacedBy(8.dp), + modifier = Modifier.testTag(TestTags.Converter.ADVANCED_AUDIO_CHIPS), + ) { AudioCodec.entries.forEach { codec -> FilterChip( selected = codec == spec.audioCodec, @@ -331,9 +368,11 @@ private fun AdvancedPicker( @OptIn(ExperimentalLayoutApi::class) @Composable -private fun ValidationError(invalid: Validation.Invalid, onSuggestion: (OutputSpec) -> Unit) { +internal fun ValidationError(invalid: Validation.Invalid, onSuggestion: (OutputSpec) -> Unit) { Card( - modifier = Modifier.fillMaxWidth(), + modifier = Modifier + .fillMaxWidth() + .testTag(TestTags.Converter.VALIDATION_ERROR), colors = CardDefaults.cardColors( containerColor = MaterialTheme.colorScheme.errorContainer, contentColor = MaterialTheme.colorScheme.onErrorContainer, @@ -347,10 +386,11 @@ private fun ValidationError(invalid: Validation.Invalid, onSuggestion: (OutputSp if (invalid.suggestions.isNotEmpty()) { Text("Try instead:", style = MaterialTheme.typography.labelMedium) FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) { - invalid.suggestions.forEach { suggestion -> + invalid.suggestions.forEachIndexed { index, suggestion -> AssistChip( onClick = { onSuggestion(suggestion) }, label = { Text(describe(suggestion)) }, + modifier = Modifier.testTag(TestTags.Converter.suggestion(index)), ) } } @@ -359,7 +399,7 @@ private fun ValidationError(invalid: Validation.Invalid, onSuggestion: (OutputSp } } -private fun describe(spec: OutputSpec): String { +internal fun describe(spec: OutputSpec): String { val video = when (spec.videoCodec) { VideoCodec.NONE -> null else -> spec.videoCodec.label @@ -374,9 +414,12 @@ private fun describe(spec: OutputSpec): String { @OptIn(ExperimentalLayoutApi::class) @Composable -private fun QualityPicker(selected: QualityTier, onSelect: (QualityTier) -> Unit) { +internal fun QualityPicker(selected: QualityTier, onSelect: (QualityTier) -> Unit) { Text("Quality", style = MaterialTheme.typography.titleSmall) - FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + FlowRow( + horizontalArrangement = Arrangement.spacedBy(8.dp), + modifier = Modifier.testTag(TestTags.Converter.QUALITY_CHIPS), + ) { QualityTier.entries.forEach { tier -> FilterChip( selected = tier == selected, @@ -390,9 +433,12 @@ private fun QualityPicker(selected: QualityTier, onSelect: (QualityTier) -> Unit @OptIn(ExperimentalLayoutApi::class) @Composable -private fun EnginePicker(selected: EnginePreference, onSelect: (EnginePreference) -> Unit) { +internal fun EnginePicker(selected: EnginePreference, onSelect: (EnginePreference) -> Unit) { Text("Engine", style = MaterialTheme.typography.titleSmall) - FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + FlowRow( + horizontalArrangement = Arrangement.spacedBy(8.dp), + modifier = Modifier.testTag(TestTags.Converter.ENGINE_CHIPS), + ) { EnginePreference.entries.forEach { preference -> FilterChip( selected = preference == selected, @@ -403,7 +449,7 @@ private fun EnginePicker(selected: EnginePreference, onSelect: (EnginePreference } } -private fun EnginePreference.label(): String = when (this) { +internal fun EnginePreference.label(): String = when (this) { EnginePreference.AUTO -> "Automatic" EnginePreference.PREFER_HARDWARE -> "Prefer hardware" EnginePreference.FORCE_SOFTWARE -> "Force software" @@ -418,21 +464,34 @@ private fun EnginePreference.label(): String = when (this) { * pretending it has an unknown codec. */ @Composable -private fun FileCard(input: InputFile) { - Card(modifier = Modifier.fillMaxWidth()) { +internal fun FileCard(input: InputFile) { + Card( + modifier = Modifier + .fillMaxWidth() + .testTag(TestTags.Converter.FILE_CARD), + ) { Column(modifier = Modifier.padding(16.dp)) { - Text(input.displayName, style = MaterialTheme.typography.titleMedium) + Text( + input.displayName, + style = MaterialTheme.typography.titleMedium, + modifier = Modifier.testTag(TestTags.Converter.FILE_CARD_NAME), + ) // The null is handled here rather than inside formatBytes, because "no provider would // say" is not a number and a formatter that invented one -- "0 B" -- is the defect // this card would be showing. It degrades in words, like the codec rows below it. Text( input.sizeBytes?.let(::formatBytes) ?: "Size unknown", style = MaterialTheme.typography.bodySmall, + modifier = Modifier.testTag(TestTags.Converter.FILE_CARD_BYTES), ) val probe = input.probe if (probe == null) { - Text("Reading…", style = MaterialTheme.typography.bodySmall) + Text( + "Reading…", + style = MaterialTheme.typography.bodySmall, + modifier = Modifier.testTag(TestTags.Converter.FILE_CARD_NOTE), + ) return@Column } @@ -442,6 +501,7 @@ private fun FileCard(input: InputFile) { InputKind.UNPARSEABLE -> Text( "Could not identify this file. It will be converted with FFmpeg.", style = MaterialTheme.typography.bodySmall, + modifier = Modifier.testTag(TestTags.Converter.FILE_CARD_NOTE), ) InputKind.IMAGE -> { @@ -481,22 +541,23 @@ private fun FileCard(input: InputFile) { } @Composable -private fun DetailRow(label: String, value: String) { +internal fun DetailRow(label: String, value: String) { Text( "$label: $value", style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.testTag(TestTags.Converter.detailRow(label)), ) } -private fun formatDuration(ms: Long): String { +internal fun formatDuration(ms: Long): String { val totalSeconds = ms / 1000 val minutes = totalSeconds / 60 val seconds = totalSeconds % 60 return String.format(Locale.US, "%d:%02d", minutes, seconds) } -private fun formatBytes(bytes: Long): String = when { +internal fun formatBytes(bytes: Long): String = when { bytes >= 1_000_000_000 -> String.format(Locale.US, "%.1f GB", bytes / 1e9) bytes >= 1_000_000 -> String.format(Locale.US, "%.1f MB", bytes / 1e6) bytes >= 1_000 -> String.format(Locale.US, "%.0f kB", bytes / 1e3) diff --git a/app/src/main/java/org/libremediaconverter/join/JoinScreen.kt b/app/src/main/java/org/libremediaconverter/join/JoinScreen.kt index c1a395e..a247d65 100644 --- a/app/src/main/java/org/libremediaconverter/join/JoinScreen.kt +++ b/app/src/main/java/org/libremediaconverter/join/JoinScreen.kt @@ -21,6 +21,7 @@ import androidx.compose.runtime.getValue import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.testTag import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp import androidx.lifecycle.compose.collectAsStateWithLifecycle @@ -31,6 +32,7 @@ import org.libremediaconverter.model.ConcatStrategy import org.libremediaconverter.ui.PrimaryButtonHeight import org.libremediaconverter.ui.ScreenPaddingHorizontal import org.libremediaconverter.ui.ScreenPaddingVertical +import org.libremediaconverter.ui.TestTags import org.libremediaconverter.work.ConcatWorker @UnstableApi @@ -82,7 +84,8 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod onClick = { pickInputs.launch(arrayOf("video/*")) }, modifier = Modifier .fillMaxWidth() - .height(PrimaryButtonHeight), + .height(PrimaryButtonHeight) + .testTag(TestTags.Join.CHOOSE_FILES), ) { Text("Choose files") } } @@ -97,11 +100,16 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod s.inputs.forEach { FileRow(it) } Button( onClick = viewModel::join, - modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight), + modifier = Modifier + .fillMaxWidth() + .height(PrimaryButtonHeight) + .testTag(TestTags.Join.JOIN), ) { Text("Join ${s.inputs.size} files") } OutlinedButton( onClick = { pickInputs.launch(arrayOf("video/*")) }, - modifier = Modifier.fillMaxWidth(), + modifier = Modifier + .fillMaxWidth() + .testTag(TestTags.Join.CHOOSE_DIFFERENT_FILES), ) { Text("Choose different files") } } @@ -110,10 +118,14 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod // Indeterminate on purpose: FFmpeg reports progress against a // single input's duration, which means nothing across a // concatenation. A fabricated percentage would be worse than none. - LinearProgressIndicator(modifier = Modifier.fillMaxWidth()) + LinearProgressIndicator( + modifier = Modifier + .fillMaxWidth() + .testTag(TestTags.Join.PROGRESS), + ) OutlinedButton( onClick = viewModel::cancel, - modifier = Modifier.fillMaxWidth(), + modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL), ) { Text("Cancel") } } @@ -127,7 +139,7 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod ) OutlinedButton( onClick = viewModel::cancel, - modifier = Modifier.fillMaxWidth(), + modifier = Modifier.fillMaxWidth().testTag(TestTags.CANCEL), ) { Text("Cancel") } } @@ -146,11 +158,14 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod ) Button( onClick = { chooseDestination.launch(s.suggestedName) }, - modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight), + modifier = Modifier + .fillMaxWidth() + .height(PrimaryButtonHeight) + .testTag(TestTags.SAVE_FILE), ) { Text("Save file") } OutlinedButton( onClick = viewModel::reset, - modifier = Modifier.fillMaxWidth(), + modifier = Modifier.fillMaxWidth().testTag(TestTags.START_OVER), ) { Text("Start over") } } @@ -158,7 +173,10 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod Text("Saved ${s.displayName}.", style = MaterialTheme.typography.bodyLarge) Button( onClick = viewModel::reset, - modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight), + modifier = Modifier + .fillMaxWidth() + .height(PrimaryButtonHeight) + .testTag(TestTags.Join.JOIN_MORE), ) { Text("Join more") } } @@ -170,7 +188,10 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod ) Button( onClick = viewModel::reset, - modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight), + modifier = Modifier + .fillMaxWidth() + .height(PrimaryButtonHeight) + .testTag(TestTags.START_OVER), ) { Text("Start over") } } } @@ -180,8 +201,12 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod } @Composable -private fun FileRow(input: InputFile) { - Card(modifier = Modifier.fillMaxWidth()) { +internal fun FileRow(input: InputFile) { + Card( + modifier = Modifier + .fillMaxWidth() + .testTag(TestTags.Join.fileRow(input.displayName)), + ) { Column(modifier = Modifier.padding(12.dp)) { Text(input.displayName, style = MaterialTheme.typography.bodyMedium) } diff --git a/app/src/main/java/org/libremediaconverter/ui/TestTags.kt b/app/src/main/java/org/libremediaconverter/ui/TestTags.kt new file mode 100644 index 0000000..00aa19c --- /dev/null +++ b/app/src/main/java/org/libremediaconverter/ui/TestTags.kt @@ -0,0 +1,142 @@ +package org.libremediaconverter.ui + +/** + * Where a test finds each affordance on the two screens. + * + * Every button, picker and card in `ConverterScreen` and `JoinScreen` carries one of these through + * `Modifier.testTag`, so a test names a symbol and never a literal. That is the whole reason the + * table exists: `"Cancel"`, `"Start over"` and `"Save file"` are each rendered by both screens and + * by more than one state branch, so rewording one of them would otherwise redden several + * independent test files at once, and none of those diffs would explain why. + * + * Tags are applied inside `main`, never handed in by the caller. A tag a test passes down as a + * `Modifier` proves only that the test set it -- it would stay green with the affordance's own tag + * deleted, which is exactly the vacuous test `CLAUDE.md` records nine of. + * + * ### Public rather than `internal`, deliberately + * + * `androidTest` **is** a friend source set of `main` here: an `androidTest` file referencing the + * `internal` `Destination.CONVERT` compiles clean through `:app:compileDebugAndroidTestKotlin` + * under AGP 9.3.1 (measured 2026-08-24 -- nothing in the repo referenced a main `internal` from + * `androidTest`, so the question had no in-tree answer until then). `internal` would compile today. + * + * It is public anyway. That friendship is AGP wiring rather than something this project states, and + * this table is a contract read from three source sets: `main` applies the tags, `src/test` and + * `src/androidTest` name them. Public buys no external exposure in an application module -- nothing + * consumes it from outside -- so the durable answer costs nothing here. + * + * ### Invariants + * + * Values are distinct, which `TagTableUniquenessTest` asserts. Two affordances sharing a tag would + * break the "resolves to exactly one node" assertion in a file nobody had touched. + */ +object TestTags { + + /** + * Affordances both screens render, under one name each. + * + * Shared rather than per-screen because only one screen is composed at a time -- the shell + * swaps them -- so a tag can only ever resolve within the screen under test. + */ + const val CANCEL: String = "action.cancel" + + /** Rendered by `Converted`/`Joined` and again by `Failed` on both screens. */ + const val START_OVER: String = "action.startOver" + + const val SAVE_FILE: String = "action.saveFile" + + /** `ConverterScreen`. */ + object Converter { + const val CHOOSE_FILE: String = "converter.chooseFile" + const val CONVERT: String = "converter.convert" + const val CHOOSE_DIFFERENT_FILE: String = "converter.chooseDifferentFile" + const val CONVERT_ANOTHER: String = "converter.convertAnother" + + /** The determinate bar in `Converting`. It carries no text, so nothing else can find it. */ + const val PROGRESS: String = "converter.progress" + + const val FILE_CARD: String = "converter.fileCard" + const val FILE_CARD_NAME: String = "converter.fileCard.name" + + /** + * The byte size, or `"Size unknown"`. + * + * Named for bytes rather than "size" because the `IMAGE` branch also renders a row labelled + * `Size` -- pixel dimensions -- through [detailRow], and the two mean different things. + */ + const val FILE_CARD_BYTES: String = "converter.fileCard.bytes" + + /** + * The one-line explanation that stands in for the detail rows: `"Reading…"` while the probe + * is still running, or the unreadable-file line once it has finished and found nothing. + * The two are mutually exclusive, so one tag covers both. + */ + const val FILE_CARD_NOTE: String = "converter.fileCard.note" + + /** + * The chip rows, not the pickers around them. + * + * Each tag sits on the `FlowRow` of chips, so the prose a picker renders beside it -- the + * `"Custom — set below."` line under the formats, the tier description under the quality + * chips -- is outside the tagged node. Tagging the picker as a whole would mean wrapping + * three sibling emissions in a layout that does not exist today. + */ + const val FORMAT_CHIPS: String = "converter.formatChips" + + const val QUALITY_CHIPS: String = "converter.qualityChips" + const val ENGINE_CHIPS: String = "converter.engineChips" + + /** The `Advanced` / `Hide advanced` toggle. Present whether or not the panel is open. */ + const val ADVANCED_TOGGLE: String = "converter.advanced.toggle" + + /** The panel the toggle gates. Absent from the tree while collapsed. */ + const val ADVANCED_PANEL: String = "converter.advanced.panel" + + /** + * The three chip rows inside the panel, separately. + * + * Separately because their labels collide: `"Copy"` and `"None"` are both a `VideoCodec` + * and an `AudioCodec`, and `"MP3"` and `"FLAC"` are both a `Container` and an `AudioCodec`, + * so a text matcher over the open panel is ambiguous for four of the chips. + */ + const val ADVANCED_CONTAINER_CHIPS: String = "converter.advanced.containerChips" + + const val ADVANCED_VIDEO_CHIPS: String = "converter.advanced.videoChips" + const val ADVANCED_AUDIO_CHIPS: String = "converter.advanced.audioChips" + + /** The error card. Rendered outside the panel, so it is reachable while collapsed. */ + const val VALIDATION_ERROR: String = "converter.validationError" + + /** One detail line of the file card, by the label it renders: `Container`, `Video`, ... */ + fun detailRow(label: String): String = "converter.fileCard.row:$label" + + /** + * One suggested output on the validation card, by position. + * + * By position rather than by the text of the suggestion, because that text comes from + * `describe`, which is itself under test -- a tag derived from it would move whenever the + * thing it is meant to locate changed. + */ + fun suggestion(index: Int): String = "converter.validationError.suggestion:$index" + } + + /** `JoinScreen`. */ + object Join { + const val CHOOSE_FILES: String = "join.chooseFiles" + const val JOIN: String = "join.join" + const val CHOOSE_DIFFERENT_FILES: String = "join.chooseDifferentFiles" + const val JOIN_MORE: String = "join.joinMore" + + /** The indeterminate bar in `Joining`. */ + const val PROGRESS: String = "join.progress" + + /** + * One picked input, by the name it displays. + * + * By name rather than by position, so the tag is derived from data the row already holds + * and can stay inside `FileRow`. Passing an index down would mean the call site owned the + * tag, and a test that supplies its own tag asserts nothing about the screen. + */ + fun fileRow(displayName: String): String = "join.fileRow:$displayName" + } +} diff --git a/app/src/test/java/org/libremediaconverter/convert/ConverterLeafTagsTest.kt b/app/src/test/java/org/libremediaconverter/convert/ConverterLeafTagsTest.kt new file mode 100644 index 0000000..0a64b24 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/convert/ConverterLeafTagsTest.kt @@ -0,0 +1,196 @@ +package org.libremediaconverter.convert + +import android.net.Uri +import androidx.compose.ui.test.assertCountEquals +import androidx.compose.ui.test.onAllNodesWithTag +import androidx.compose.ui.test.onNodeWithTag +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.libremediaconverter.createDrainedComposeRule +import org.libremediaconverter.model.AudioCodec +import org.libremediaconverter.model.Container +import org.libremediaconverter.model.EnginePreference +import org.libremediaconverter.model.InputKind +import org.libremediaconverter.model.InputProbe +import org.libremediaconverter.model.OutputFormat +import org.libremediaconverter.model.OutputSpec +import org.libremediaconverter.model.QualityTier +import org.libremediaconverter.model.Validation +import org.libremediaconverter.model.VideoCodec +import org.libremediaconverter.ui.TestTags +import org.robolectric.RobolectricTestRunner + +/** + * Each leaf of the converter screen renders, and each tag it claims resolves to exactly one node. + * + * The defect this bites on is a tag that is not where the table says it is: dropped by a refactor + * that rewrote a `Modifier` chain, applied to the wrong one of two siblings, or duplicated onto a + * leaf that is rendered twice. None of that is visible at compile time -- a `testTag` is a string + * handed to a modifier -- and none of it shows up in the app either, because nothing but a test + * ever reads one. + * + * It has to be caught here rather than by the children that consume the tags. R38.2, R38.3 and + * R38.4 all *begin* by locating a node through one of these, so a tag that had quietly moved would + * surface as three unrelated PRs failing on a line their own diffs do not touch. Counting the nodes + * rather than asserting existence is deliberate: `onNodeWithTag` on two matches throws about + * ambiguity in one place and passes in another, so "exactly one" is the property worth pinning. + * + * Deliberately *not* the state matrix. Which affordances each `ConversionState` renders is R38.6, + * and it needs the state seam R38.5 extracts -- the branch buttons tagged in this change (Convert, + * Cancel, Save file, Start over, ...) therefore have no bite yet, which the PR body records. + */ +@UnstableApi +@RunWith(RobolectricTestRunner::class) +class ConverterLeafTagsTest { + + @get:Rule + val composeRule = createDrainedComposeRule() + + private fun assertResolvesToOneNode(tag: String) { + composeRule.onAllNodesWithTag(tag).assertCountEquals(1) + } + + private fun input(sizeBytes: Long? = 12_345_678L, probe: InputProbe? = VIDEO_PROBE) = InputFile( + uri = Uri.parse("content://test/clip.mkv"), + displayName = "clip.mkv", + sizeBytes = sizeBytes, + probe = probe, + ) + + @Test + fun `the format picker tags its chip row`() { + composeRule.setContent { FormatPicker(OutputFormat.MP4_H264) {} } + + assertResolvesToOneNode(TestTags.Converter.FORMAT_CHIPS) + } + + @Test + fun `the quality picker tags its chip row`() { + composeRule.setContent { QualityPicker(QualityTier.FAST) {} } + + assertResolvesToOneNode(TestTags.Converter.QUALITY_CHIPS) + } + + @Test + fun `the engine picker tags its chip row`() { + composeRule.setContent { EnginePicker(EnginePreference.AUTO) {} } + + assertResolvesToOneNode(TestTags.Converter.ENGINE_CHIPS) + } + + @Test + fun `the advanced picker tags its toggle, which is all it renders while collapsed`() { + setAdvancedPicker() + + assertResolvesToOneNode(TestTags.Converter.ADVANCED_TOGGLE) + composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_PANEL).assertDoesNotExist() + } + + /** + * The panel and its three rows only exist once the toggle has been clicked, which is R38.4's + * subject. Expanding is the only way to reach the tags at all, so the smoke test has to do it. + */ + @Test + fun `expanding the advanced picker tags the panel and each of its three chip rows`() { + setAdvancedPicker() + + composeRule.onNodeWithTag(TestTags.Converter.ADVANCED_TOGGLE).performClick() + + assertResolvesToOneNode(TestTags.Converter.ADVANCED_PANEL) + assertResolvesToOneNode(TestTags.Converter.ADVANCED_CONTAINER_CHIPS) + assertResolvesToOneNode(TestTags.Converter.ADVANCED_VIDEO_CHIPS) + assertResolvesToOneNode(TestTags.Converter.ADVANCED_AUDIO_CHIPS) + } + + @Test + fun `the validation card tags itself and every suggestion on it`() { + composeRule.setContent { + ValidationError( + Validation.Invalid( + message = "WebM cannot hold H.264 video.", + suggestions = listOf( + OutputSpec(Container.MKV, VideoCodec.H264, AudioCodec.AAC), + OutputSpec(Container.WEBM, VideoCodec.VP9, AudioCodec.OPUS), + ), + ), + ) {} + } + + assertResolvesToOneNode(TestTags.Converter.VALIDATION_ERROR) + assertResolvesToOneNode(TestTags.Converter.suggestion(0)) + assertResolvesToOneNode(TestTags.Converter.suggestion(1)) + } + + @Test + fun `the file card tags itself, its name and its size line`() { + composeRule.setContent { FileCard(input()) } + + assertResolvesToOneNode(TestTags.Converter.FILE_CARD) + assertResolvesToOneNode(TestTags.Converter.FILE_CARD_NAME) + assertResolvesToOneNode(TestTags.Converter.FILE_CARD_BYTES) + } + + /** + * Both writers of the note line get their own case. They are two separate `Text` calls in two + * branches that share one tag, so a test of either alone would leave the other unguarded. + */ + @Test + fun `the file card tags the note it shows while the probe is still running`() { + composeRule.setContent { FileCard(input(probe = null)) } + + assertResolvesToOneNode(TestTags.Converter.FILE_CARD_NOTE) + } + + @Test + fun `the file card tags the note it shows when nothing could read the file`() { + composeRule.setContent { FileCard(input(probe = InputProbe(kind = InputKind.UNPARSEABLE))) } + + assertResolvesToOneNode(TestTags.Converter.FILE_CARD_NOTE) + } + + @Test + fun `a detail row tags itself with the label it renders`() { + composeRule.setContent { DetailRow("Container", "Matroska") } + + assertResolvesToOneNode(TestTags.Converter.detailRow("Container")) + } + + /** The rows the file card builds carry the same per-label tags, one per row it renders. */ + @Test + fun `the file card's detail rows are each tagged by their own label`() { + composeRule.setContent { FileCard(input()) } + + assertResolvesToOneNode(TestTags.Converter.detailRow("Container")) + assertResolvesToOneNode(TestTags.Converter.detailRow("Video")) + assertResolvesToOneNode(TestTags.Converter.detailRow("Audio")) + assertResolvesToOneNode(TestTags.Converter.detailRow("Length")) + } + + private fun setAdvancedPicker() { + composeRule.setContent { + AdvancedPicker( + spec = OutputSpec(Container.MP4, VideoCodec.H264, AudioCodec.AAC), + validation = Validation.Valid, + onContainer = {}, + onVideoCodec = {}, + onAudioCodec = {}, + onSuggestion = {}, + ) + } + } + + private companion object { + val VIDEO_PROBE = InputProbe( + videoCodec = "video/avc", + audioCodec = "audio/mp4a-latm", + durationMs = 90_000, + kind = InputKind.VIDEO, + container = Container.MKV, + width = 1920, + height = 1080, + ) + } +} diff --git a/app/src/test/java/org/libremediaconverter/join/JoinLeafTagsTest.kt b/app/src/test/java/org/libremediaconverter/join/JoinLeafTagsTest.kt new file mode 100644 index 0000000..6316057 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/join/JoinLeafTagsTest.kt @@ -0,0 +1,53 @@ +package org.libremediaconverter.join + +import android.net.Uri +import androidx.compose.ui.test.assertCountEquals +import androidx.compose.ui.test.onAllNodesWithTag +import androidx.media3.common.util.UnstableApi +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.libremediaconverter.convert.InputFile +import org.libremediaconverter.createDrainedComposeRule +import org.libremediaconverter.ui.TestTags +import org.robolectric.RobolectricTestRunner + +/** + * The join screen's one leaf renders, and tags itself with the file it is showing. + * + * `FileRow` is the only place on either screen where the same leaf is rendered more than once at a + * time -- one row per picked input -- so it is the only tag that cannot be a constant. It is + * derived from `displayName`, inside `FileRow` itself, and that is the part worth a test: a row + * that took its tag from the call site would let R38.7 pass a tag in and assert nothing, which is + * the vacuous shape `CLAUDE.md` records nine of in one review. + * + * Two rows are rendered here rather than one, because a tag derived from the wrong thing -- a + * constant, an index the row does not have -- would still resolve to one node with a single input + * on screen. + * + * Deliberately not the state matrix: which affordances each `JoinState` renders is R38.7. + */ +@UnstableApi +@RunWith(RobolectricTestRunner::class) +class JoinLeafTagsTest { + + @get:Rule + val composeRule = createDrainedComposeRule() + + private fun input(displayName: String) = InputFile( + uri = Uri.parse("content://test/$displayName"), + displayName = displayName, + sizeBytes = 4_000_000L, + ) + + @Test + fun `each file row is tagged with the name it displays`() { + composeRule.setContent { + FileRow(input("first.mp4")) + FileRow(input("second.mp4")) + } + + composeRule.onAllNodesWithTag(TestTags.Join.fileRow("first.mp4")).assertCountEquals(1) + composeRule.onAllNodesWithTag(TestTags.Join.fileRow("second.mp4")).assertCountEquals(1) + } +} diff --git a/app/src/test/java/org/libremediaconverter/ui/TagTableUniquenessTest.kt b/app/src/test/java/org/libremediaconverter/ui/TagTableUniquenessTest.kt new file mode 100644 index 0000000..333b051 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/ui/TagTableUniquenessTest.kt @@ -0,0 +1,40 @@ +package org.libremediaconverter.ui + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * No two entries of [TestTags] may share a value. + * + * A duplicated value is the one mistake this table invites -- the constants are added in blocks of + * near-identical lines, and a copy-paste that keeps the old string still compiles, still reads + * correctly at the call site, and still passes every test in the file that placed it. It surfaces + * later, in someone else's PR, as an affordance that "resolves to exactly one node" finding two, + * with nothing in that diff to explain it. + * + * Read by reflection rather than from a hand-written list, because a hand-written list would be a + * second copy of the table with the same copy-paste failure in it. + */ +class TagTableUniquenessTest { + + private fun tagsIn(vararg holders: Class<*>): List = holders.flatMap { holder -> + holder.declaredFields + .filter { it.type == String::class.java } + .map { it.get(null) as String } + } + + @Test + fun `every tag constant has its own value`() { + val tags = tagsIn( + TestTags::class.java, + TestTags.Converter::class.java, + TestTags.Join::class.java, + ) + + // Without this the check would pass on an empty list, which is what a reflection call + // that stopped finding the constants would hand it. + assertTrue("reflection found only ${tags.size} tag constants, so it is not reading the table", tags.size > 20) + assertEquals(emptyList(), tags.groupBy { it }.filterValues { it.size > 1 }.keys.toList()) + } +}