Merge remote-tracking branch 'origin/main' into m-127-tmp

This commit is contained in:
2026-08-25 23:57:00 -05:00
7 changed files with 397 additions and 11 deletions
+193
View File
@@ -0,0 +1,193 @@
#!/usr/bin/env bash
#
# Exercises e2e-report-shape.sh's baseline counter against fixture source, with no emulator and
# no CI run. Run it directly:
#
# .github/scripts/e2e-report-shape-test.sh
#
# WHY THIS CAN EXIST AT ALL: the counter is a pure function of the working tree. It greps
# `app/src/androidTest` for `@FailsOnEmulatorApi37` and compares the total against the number
# committed in FailsOnEmulatorApi37.kt. Nothing about that needs a device, which is the whole
# reason #120 could be measured rather than argued about.
#
# WHY A THROWAWAY REPO ROOT rather than a knob on the script. The report finds its own root from
# `BASH_SOURCE`, so a copy of it placed at `<root>/.github/scripts/` reads `<root>/app/src/...`.
# Building that root is three mkdirs and costs the shipped script nothing:
#
# - the REAL script is what runs, byte for byte, so reverting the matcher reddens this test
# rather than a testing-only code path beside it;
# - no environment variable exists that could point the LIVE count somewhere else, which is
# the failure mode #83 built the baseline check to prevent in the first place;
# - XML_DIR resolves inside the throwaway root, so a stale app/build/outputs left by a real
# run on a developer machine cannot leak into the numbers here.
#
# WHAT IT GUARDS (#120). The old matcher looked for the string anywhere on any line, so a KDoc
# saying `Deliberately not @FailsOnEmulatorApi37` counted as a marked test and every PR got a
# deviation notice that was wrong. The obvious repair -- count only lines that are nothing but
# the annotation -- silently stops counting `@FailsOnEmulatorApi37 @Test`, which is legal Kotlin,
# and undercounting is the direction that hides a genuine new marker. The fixture carries every
# shape at once -- three that count and three that must not, enumerated in its own header -- so
# both mistakes fail here instead of on a PR: against testdata/marker-shapes the old matcher says
# 5, own-line-only says 2, and the shipped one 3.
#
# NOT WIRED INTO CI, deliberately and as a known gap. Adding a step to the Static analysis job
# would add a new way for a gating job to go red, and #120 was explicit that nothing about it may
# change any job's status or the pass/fail rules. shellcheck still covers this file, since that
# step reads `git ls-files '*.sh'` rather than a fixed list.
set -uo pipefail
SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
REPORT="$SCRIPT_DIR/e2e-report-shape.sh"
FIXTURE_DIR="$SCRIPT_DIR/testdata/marker-shapes"
FIXTURE="$FIXTURE_DIR/MarkerShapes.kt"
TMP="$(mktemp -d)"
# Single quotes: the path is expanded when the trap fires, not when it is set.
trap 'rm -rf -- "$TMP"' EXIT
failures=0
pass() { printf 'ok %s\n' "$1"; }
fail() {
failures=$((failures + 1))
printf 'FAIL %s\n' "$1"
shift
printf ' %s\n' "$@"
}
# assert_contains <name> <haystack> <needle>
# `case` rather than grep: the strings being matched carry backticks and an em dash, and this
# way neither the shell nor a regex engine gets an opinion about them.
assert_contains() {
case "$2" in
*"$3"*) pass "$1" ;;
*) fail "$1" "wanted to find: $3" "in:" "$2" ;;
esac
}
assert_absent() {
case "$2" in
*"$3"*) fail "$1" "did NOT want to find: $3" "in:" "$2" ;;
*) pass "$1" ;;
esac
}
# make_root <marked-tree-dir> <baseline-number>
# Assembles a throwaway repo root around the given tree and prints its path.
make_root() {
local tree="$1" baseline="$2" root testdir
root="$(mktemp -d "$TMP/root.XXXXXX")"
testdir="$root/app/src/androidTest/java/org/libremediaconverter"
mkdir -p "$root/.github/scripts" "$testdir"
cp -- "$REPORT" "$root/.github/scripts/"
cp -- "$tree"/*.kt "$testdir/"
# The synthetic stand-in for the committed baseline. Its KDoc names the marker the way the real
# file does -- in brackets, never with an `@` -- because the real file lives inside the tree
# being counted, so an `@` spelling here would add a phantom to every number below.
cat > "$testdir/FailsOnEmulatorApi37.kt" <<EOF
package org.libremediaconverter
/** Stand-in for the real marker file. Only [FAILS_ON_EMULATOR_API37_BASELINE] is read. */
const val FAILS_ON_EMULATOR_API37_BASELINE = $baseline
EOF
# A clean, untruncated run of exactly <baseline> tests, all failing -- which is what the
# advisory leg looks like when nothing has drifted. It leaves the marked count as the only
# field that can deviate, so every assertion below is about the thing under test.
cat > "$root/gradle.log" <<EOF
> Task :app:connectedDebugAndroidTest
Starting $baseline tests on test(AVD) - 16
There was $baseline failure(s).
EOF
printf '%s\n' "$root"
}
# run_report <root> -- stdout of the real script; its summary lands in <root>/summary.md.
run_report() {
E2E_WEDGED_AFTER='' GITHUB_STEP_SUMMARY="$1/summary.md" \
bash "$1/.github/scripts/e2e-report-shape.sh" 37 "$1/gradle.log" \
"$1/app/src/androidTest/java/org/libremediaconverter/FailsOnEmulatorApi37.kt"
}
# ---------------------------------------------------------------------------
# The fixture still carries every shape.
#
# Three of the checks below are covered twice over -- deleting a real annotation moves the count
# and fails a case further down. The two decoys are not: drop the KDoc mention and the count
# stays 3, so the precision this whole ticket is about would stop being tested and nothing would
# say so. That asymmetry is why the shapes are asserted by name rather than only by their effect
# on the total.
# ---------------------------------------------------------------------------
fixture_text="$(cat -- "$FIXTURE")"
assert_contains "fixture: the import" "$fixture_text" 'import org.libremediaconverter.FailsOnEmulatorApi37'
assert_contains "fixture: annotation own line" "$fixture_text" '
@FailsOnEmulatorApi37
@Test'
assert_contains "fixture: annotation with @Test on one line" "$fixture_text" '@FailsOnEmulatorApi37 @Test'
assert_contains "fixture: annotation nested and indented" "$fixture_text" '
@FailsOnEmulatorApi37'
assert_contains "fixture: KDoc mention (this is #120)" "$fixture_text" "* Deliberately not \`@FailsOnEmulatorApi37\`"
assert_contains "fixture: commented-out annotation" "$fixture_text" '// @FailsOnEmulatorApi37'
# ---------------------------------------------------------------------------
# 1. The fixture's three real annotations against a baseline of 3: no deviation.
#
# This one case fails under both wrong matchers -- the old one counts 5, own-line-only counts 2 --
# which is why it is first.
# ---------------------------------------------------------------------------
root="$(make_root "$FIXTURE_DIR" 3)"
out="$(run_report "$root")"
assert_contains "3 real markers, baseline 3: reports a match" "$out" ' baseline: matches (3 expected, 3 failed)'
assert_absent "3 real markers, baseline 3: says nothing about the tree" "$out" 'the tree carries'
assert_contains "3 real markers, baseline 3: summary agrees" \
"$(cat -- "$root/summary.md")" '**Matches the committed baseline of 3**'
# ---------------------------------------------------------------------------
# 2. A fourth REAL annotation. The count has to move and the deviation has to fire.
#
# The important half of #120: precision was the bug, but a matcher that stopped noticing a new
# marker would have been a worse one, silently.
# ---------------------------------------------------------------------------
plus_one="$(mktemp -d "$TMP/plusone.XXXXXX")"
cp -- "$FIXTURE" "$plus_one/"
cat > "$plus_one/FourthMarker.kt" <<'EOF'
package org.libremediaconverter.fixture
class FourthMarker {
@FailsOnEmulatorApi37
@Test
fun addedToday() = Unit
}
EOF
root="$(make_root "$plus_one" 3)"
out="$(run_report "$root")"
assert_contains "a 4th real marker: the deviation fires, and counts 4" "$out" \
" baseline DEVIATION: the tree carries 4 tests marked \`@FailsOnEmulatorApi37\` but the baseline says 3 — update FAILS_ON_EMULATOR_API37_BASELINE"
assert_contains "a 4th real marker: the summary carries it too" "$(cat -- "$root/summary.md")" \
"- the tree carries 4 tests marked \`@FailsOnEmulatorApi37\` but the baseline says 3"
# ---------------------------------------------------------------------------
# 3. Delete the same-line annotation and the count must drop to 2.
#
# This is the trap, pinned down. `@FailsOnEmulatorApi37 @Test` on one line is what separates the
# shipped matcher from `^[[:space:]]*@NAME[[:space:]]*$`, and without this case the fixture entry
# guarding it could be deleted as decoration -- case 1 would then pass under the wrong matcher.
# Here the same-line entry is worth exactly one, and it is asserted to be.
# ---------------------------------------------------------------------------
minus_same_line="$(mktemp -d "$TMP/minus.XXXXXX")"
sed -e '/@FailsOnEmulatorApi37 @Test/d' -- "$FIXTURE" > "$minus_same_line/MarkerShapes.kt"
root="$(make_root "$minus_same_line" 3)"
out="$(run_report "$root")"
assert_contains "same-line annotation removed: counts 2, so it was worth 1" "$out" \
" baseline DEVIATION: the tree carries 2 tests marked \`@FailsOnEmulatorApi37\` but the baseline says 3 — update FAILS_ON_EMULATOR_API37_BASELINE"
echo
if [ "$failures" -eq 0 ]; then
echo "e2e-report-shape-test.sh: all checks passed"
exit 0
fi
echo "e2e-report-shape-test.sh: $failures check(s) failed"
exit 1
+63 -4
View File
@@ -30,16 +30,31 @@
#
# Usage:
# e2e-report-shape.sh <label> <gradle-log> [<baseline-file>]
# E2E_WEDGED_AFTER=<seconds> the wrapper timeout killed gradle after that many seconds
#
# With a third argument the run is compared against the baseline in that file (advisory mode)
# and a `::notice::` is emitted per deviation. NEVER `::error::`: the advisory job is
# `continue-on-error: true` and stays that way, and an error annotation would be a new way for
# a diagnostic to change a conclusion.
#
# WHY THE WEDGE ARRIVES AS AN ENV VAR (#118) rather than being read out of the log like every
# other field: there is nothing in the log to read. A wedge is gradle never returning, so gradle
# never printed a verdict, never printed a truncation line, and never aborted instrumentation --
# the log of a wedged leg is the log of a run that simply stops. Measured on job 98035980326:
# `expected: 59`, `received: 59`, `completed cleanly: yes`, six seconds before the wedge warning,
# for a leg that the timeout had killed 22 minutes in. Only e2e-run.sh knows, because only it
# saw `timeout` exit 124, so it says so. Guessing it from a log that ends abruptly would call
# every cancelled run a wedge.
#
# It is read as a STRING and only ever interpolated into one. `[ -n ... ]`, never `-gt`: it
# crosses a process boundary from a shell that deliberately sets it EMPTY on every non-wedge
# path, and an arithmetic test on an empty string is the header's rule four paragraphs up.
set -uo pipefail
LABEL="${1:-unknown}"
LOG="${2:-}"
BASELINE_FILE="${3:-}"
WEDGED_AFTER="${E2E_WEDGED_AFTER:-}"
SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
REPO_ROOT="$(cd -- "$SCRIPT_DIR/../.." && pwd)"
@@ -115,6 +130,13 @@ elif [ -n "$abort_received" ]; then
elif [ -n "$expected" ] && [ -z "$abort_line" ]; then
received="$expected"
received_src="the run was not truncated, so every expected test reported"
# ... unless it was killed, in which case "not truncated" is only "gradle never got as far as
# saying so". This is the branch the wedged leg in #118 took -- with no XML written yet, the
# number is what the runner was TOLD to run, and the source line said the opposite in the same
# table that called the leg clean. The number is deliberately left alone: it is still the best
# available answer, and only the claim about where it came from was wrong.
[ -n "$WEDGED_AFTER" ] \
&& received_src="no test XML was written and gradle never printed a truncation line — but the leg was killed mid-run, so this is what it was told to run, not what reported"
fi
failed="unknown"
@@ -152,6 +174,12 @@ elif [ "$no_run" = "nothing" ]; then
# "cleanly" would be a lie about a run that left no evidence it happened.
completed="unknown"
completed_src="no runner output to read"
elif [ -n "$WEDGED_AFTER" ]; then
# The wedge is checked LAST of the four, so it only ever overrides the `yes`. The two "no"s
# above are already right and name the abort, which the wedge row does not; `unknown` is
# already right too. A wedge on top of an abort is both facts, and both get printed.
completed="**no**"
completed_src="the wrapper timeout killed gradle after ${WEDGED_AFTER}s — instrumentation itself was never aborted, which is why nothing in the log says so"
else
completed="yes"
completed_src="no truncation line and no \`INSTRUMENTATION_ABORTED\`"
@@ -180,11 +208,34 @@ if [ -n "$BASELINE_FILE" ]; then
advisory="yes"
[ -f "$BASELINE_FILE" ] \
&& baseline="$(sed -nE 's/^const val FAILS_ON_EMULATOR_API37_BASELINE = ([0-9]+).*/\1/p' "$BASELINE_FILE" | head -1)"
# The #81 check, verbatim: what the tree actually carries. Reported next to the baseline so a
# stale baseline shows up here rather than only once the emulator disagrees with it.
# What the tree actually carries. Reported next to the baseline so a stale baseline shows up
# here rather than only once the emulator disagrees with it.
#
# ANCHORED AT LINE START, AND WHITESPACE-OR-END-OF-LINE AFTER THE NAME (#120). The #81 check
# this replaces matched the string anywhere on any line, so #113's KDoc reading `Deliberately
# not @FailsOnEmulatorApi37` counted as a fourth marked test and the report announced a
# deviation on every PR. That is worse than a wrong number: #83 built this so a new failure
# could not be invisible, and a notice that is wrong every time teaches everyone to skim past
# deviation notices.
#
# THE OBVIOUS REPAIR IS A TRAP, and the reason for the second half of the pattern.
# `^[[:space:]]*@NAME[[:space:]]*$` -- "the annotation on a line of its own" -- also stops
# counting `@FailsOnEmulatorApi37 @Test`, which is legal Kotlin, and UNDERcounting is the
# dangerous direction: it hides a genuine new marker, which is the one thing this exists to
# catch. Measured against `testdata/marker-shapes`, a fixture carrying every shape at once:
# the old matcher says 5, own-line-only says 2, this one says 3. On the real tree, 4 / 3 / 3.
# e2e-report-shape-test.sh runs that fixture through this whole script.
#
# `^[[:space:]]*@` cannot match an `import` line, so the old `grep -v import` goes with it
# rather than staying to imply a filter is still doing work.
#
# This is a regex over source text and not a parser. An annotation inside a multi-line string,
# or inside a `/* */` block that opened mid-line, would still be counted. Neither exists here;
# if one ever does, this check wants a different tool rather than a longer regex.
if [ -d "$REPO_ROOT/app/src/androidTest" ]; then
marked="$(grep -rn "@FailsOnEmulatorApi37" "$REPO_ROOT/app/src/androidTest" --include='*.kt' \
| grep -v import | grep -c FailsOn || true)"
marked="$(grep -rhcE '^[[:space:]]*@FailsOnEmulatorApi37([[:space:]]|$)' \
"$REPO_ROOT/app/src/androidTest" --include='*.kt' \
| awk '{ total += $1 } END { print total + 0 }' || true)"
fi
fi
@@ -217,6 +268,11 @@ echo "----- RUN SHAPE (api${LABEL}) -----"
echo " expected: $expected"
echo " received: $received"
echo " failed: $failed"
# Above `completed cleanly`, because it is the line that says what happened to the leg and the
# other one only qualifies it. A reader who stops after three rows still sees it.
if [ -n "$WEDGED_AFTER" ]; then
echo " wedged: yes -- gradle was killed after ${WEDGED_AFTER}s and never returned"
fi
echo " completed cleanly: ${completed//\*/}"
if [ -n "$abort_received" ]; then
echo " received before the abort: $abort_received"
@@ -249,6 +305,9 @@ if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then
echo "| expected | $expected | $expected_src |"
echo "| received | $received | $received_src |"
echo "| failed | $failed | $failed_src |"
if [ -n "$WEDGED_AFTER" ]; then
echo "| wedged | **yes** | \`timeout\` fired after ${WEDGED_AFTER}s and killed gradle (exit 124), which is what e2e-run.sh then captured the wedge diagnostics for |"
fi
echo "| completed cleanly | $completed | $completed_src |"
if [ -n "$abort_received" ]; then
echo "| received before the abort | $abort_received | the same line — the XML above counts the truncated test as a failure, this number does not |"
+17 -3
View File
@@ -239,20 +239,34 @@ timeout -k 30s "$WEDGE_TIMEOUT" \
${E2E_EXTRA_GRADLE_ARGS:-} 2>&1 | tee "$GRADLE_LOG" || status=$?
echo "::endgroup::"
# Whether the wrapper timeout fired, decided ONCE. 124 is `timeout` saying it killed the
# command, and two places downstream need that fact: capture_wedge below, and the report, which
# otherwise calls a killed leg `completed cleanly: yes` (#118). Deriving it twice is how those
# two would drift apart -- the report would keep printing after someone changed what a wedge
# means here. It stays a string: empty on every other path, so those legs pass an empty
# E2E_WEDGED_AFTER and the report behaves exactly as before.
wedged=""
[ "$status" -eq 124 ] && wedged="$WEDGE_TIMEOUT"
# The run-shape report: expected/received/failed and whether the run finished, every time,
# green or red. It never changes `status` -- it is a diagnostic, and the header's rule about
# diagnostics applies to it as much as to every probe below.
#
# E2E_WEDGED_AFTER is the wedge, told to the report rather than left for it to infer. It cannot
# be inferred: a wedge is gradle never returning, so gradle printed no verdict at all, and the
# log the report reads looks like a run that simply stopped. Only this script knows the
# difference, because only this script saw the exit status.
#
# The baseline argument, and only it, turns on the comparison, and only the advisory API 37 job
# passes E2E_ADVISORY=1. Comparing on the gating legs would announce a deviation on all five of
# them every run, since they run the whole suite rather than the marked three. They still get
# the report: a truncated run reporting fewer results than it ran is what #108 looks like, and
# `completed cleanly` is the field that shows it.
if [ "${E2E_ADVISORY:-}" = "1" ]; then
bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" \
E2E_WEDGED_AFTER="$wedged" bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" \
"$REPO_ROOT/app/src/androidTest/java/org/libremediaconverter/FailsOnEmulatorApi37.kt" || true
else
bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" || true
E2E_WEDGED_AFTER="$wedged" bash "$SCRIPT_DIR/e2e-report-shape.sh" "$LABEL" "$GRADLE_LOG" || true
fi
if [ "$status" -eq 0 ]; then
@@ -260,7 +274,7 @@ if [ "$status" -eq 0 ]; then
exit 0
fi
if [ "$status" -eq 124 ]; then
if [ -n "$wedged" ]; then
capture_wedge "api${LABEL}"
else
echo "::error::E2E api${LABEL} failed (exit $status)"
@@ -0,0 +1,54 @@
// NOT A TEST, AND NEVER COMPILED. This is fixture data for e2e-report-shape-test.sh, which
// copies it into a throwaway repo root and runs the real report script against that. It lives
// under .github/ deliberately: Gradle only compiles app/src/**, ktlint and detekt are applied to
// :app only, and the report's own count reads app/src/androidTest -- so nothing here can reach
// the build, the linters, or the number the advisory job compares against. Verified by running
// the report against the real repo root with this file committed: still 3.
//
// It carries every shape the counter has to tell apart, in one file, because the bug in #120 was
// exactly that two of them look alike to a substring match. Three count and three must not:
//
// COUNTS the annotation on its own line
// COUNTS the annotation sharing a line with @Test -- legal Kotlin, and the case the
// obvious "own line only" repair silently drops
// COUNTS the annotation indented inside a nested class
// must NOT a KDoc mentioning it -- this is #120 itself, copied from Media3EngineTest
// must NOT a commented-out annotation
// must NOT the import
//
// Three count. That is what the synthetic baseline in the test is set to, so the fixture and the
// baseline agree exactly the way the real tree and FAILS_ON_EMULATOR_API37_BASELINE do.
//
// The `@Test` here is spelled the way a real test spells it so the fixture reads like source
// rather than like a regex exercise. Nothing runs it.
package org.libremediaconverter.fixture
import org.junit.Test
import org.libremediaconverter.FailsOnEmulatorApi37
class MarkerShapes {
@FailsOnEmulatorApi37
@Test
fun ownLine() = Unit
@FailsOnEmulatorApi37 @Test
fun sameLineAsTest() = Unit
/**
* Deliberately not `@FailsOnEmulatorApi37`: nothing here decodes or encodes, so no emulator
* codec is involved and the API 37 image has no quarrel with it.
*/
@Test
fun mentionedInKdoc() = Unit
// @FailsOnEmulatorApi37 -- taken off on 2026-01-01, kept as a note rather than deleted
@Test
fun commentedOut() = Unit
class Nested {
@FailsOnEmulatorApi37
@Test
fun indentedDeeper() = Unit
}
}
+7
View File
@@ -98,6 +98,13 @@ days. Read it as the current answer, and see the git history if you need the old
is written anyway and says nothing about it — `.github/scripts/e2e-report-shape.sh` is where that
is measured and explained.
Every leg prints that table, advisory or not, and **on the wedge path it also carries a `wedged:`
row** (#118). `completed cleanly` only ever meant "instrumentation was not aborted", which stays
true of a leg the `WEDGE_TIMEOUT` killed 22 minutes in — so without that row the table read
`received: 59, completed cleanly: yes` for a leg that had just died. The wedge is passed to the
report as `E2E_WEDGED_AFTER` by `e2e-run.sh`, which is the only thing that can know it: a wedge
is gradle never returning, so the log it left says nothing about it.
Still true, and the reason the advisory job is not simply deleted: **API 37 needs a manual check on
the Pixel 10 Pro XL before each release.** Those three tests are the one thing CI cannot answer
for.
@@ -178,7 +178,12 @@ object ContainerCapabilities {
if (!probe.hasVideo) {
return Validation.Invalid(
"This file has no video track to copy.",
listOf(spec.copy(videoCodec = VideoCodec.NONE)),
// Dropping the video is the right shape of answer, but it is only half of one:
// `spec.copy(videoCodec = NONE)` is valid exactly when the audio axis already
// happened to be fine, and refused otherwise — a Vorbis or PCM source into MP4,
// an MP3 into WebM. Handing it to the shared path repairs both axes and drops
// anything that still fails, so the chip cannot lead to a second error.
suggestions(spec.copy(videoCodec = VideoCodec.NONE), probe, exclude = spec),
)
}
val source = CodecNames.videoFromName(probe.videoCodec)
@@ -11,6 +11,11 @@ import org.junit.Test
* `OutputFormat` used to be twelve hand-picked triples, and its KDoc defended that on the grounds
* that a closed set was what made routing decidable. Opening it up moves that burden here, so this
* is where decidability now has to be proven.
*
* That includes what a refusal offers instead. `Validation.Invalid` promises every suggestion is
* itself valid and names this class as the proof, so a branch that assembles its own suggestion
* list rather than going through `suggestions()` is only checked here if some row happens to reach
* it — which is how a dead-end chip survived two widenings of that table.
*/
class ContainerCapabilitiesTest {
@@ -35,6 +40,29 @@ class ContainerCapabilitiesTest {
container = Container.MP3,
)
/**
* An audio-only input carrying a codec MP4 has no place for at all.
*
* Vorbis lives in Ogg and Matroska; MP4 carries AAC, MP3, Opus and FLAC. That gap is what turns
* a suggestion which merely drops the video track into a second refusal.
*/
private val vorbisSource = InputProbe(
videoCodec = null,
audioCodec = "vorbis",
hasVideo = false,
kind = InputKind.AUDIO_ONLY,
container = Container.OGG,
)
/** The same shape, for the other codec MP4 refuses. One case is a coincidence; two is the rule. */
private val pcmSource = InputProbe(
videoCodec = null,
audioCodec = "pcm_s16le",
hasVideo = false,
kind = InputKind.AUDIO_ONLY,
container = Container.WAV,
)
// --- copy and encode are different questions ----------------------------
/**
@@ -97,7 +125,17 @@ class ContainerCapabilitiesTest {
}
}
/** A suggestion that is itself invalid is worse than no suggestion. */
/**
* A suggestion that is itself invalid is worse than no suggestion.
*
* Only a branch that assembles its own suggestion list can break that promise: [suggestions]
* ends by filtering on `validate(...).isValid`, so everything routed through it is valid by
* construction. Those branches are what this table has to cover — the image output, and copy
* the video from a file that has none, which built its list by hand and came back refused for
* a Vorbis or PCM source into MP4 and an MP3 into WebM. The Advanced picker showed a one-tap
* fix that led straight to a second error, through two widenings of this table that never
* reached the branch.
*/
@Test
fun `every suggestion is itself valid`() {
val cases = listOf(
@@ -109,15 +147,31 @@ class ContainerCapabilitiesTest {
OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.NONE) to mp3Source,
OutputSpec(Container.MP4, VideoCodec.NONE, AudioCodec.NONE) to mp3Source,
OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.AAC) to mp3Source,
// Copy-the-video-from-a-file-with-no-video, the last branch that built its offer by
// hand. It escaped the five rows above because `spec.copy(videoCodec = NONE)` is valid
// exactly when the audio axis happens to be fine — true for the AAC and MP3 sources
// used there, false for any audio the target container cannot carry.
OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.COPY) to vorbisSource,
OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.COPY) to pcmSource,
OutputSpec(Container.WEBM, VideoCodec.COPY, AudioCodec.COPY) to mp3Source,
// The same branch with audio the container *can* hold, which is the half that already
// worked and must keep working: the repair here is a copy, so the offer is the very
// spec the caller handed to `suggestions`. It survives only because the exclusion is
// against what the user asked for rather than against the repair.
OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.COPY) to mp3Source,
// The one branch that still builds its list by hand, so that it is asserted rather
// than merely reasoned about: an image container takes `None + None` and nothing else,
// which makes its single offer valid by construction.
OutputSpec(Container.GIF, VideoCodec.H264, AudioCodec.AAC) to h264Source,
)
cases.forEach { (spec, probe) ->
val invalid = ContainerCapabilities.validate(spec, probe) as? Validation.Invalid
?: throw AssertionError("expected $spec to be rejected")
assertTrue("no alternatives offered for $spec", invalid.suggestions.isNotEmpty())
assertTrue("no alternatives offered for $spec on $probe", invalid.suggestions.isNotEmpty())
invalid.suggestions.forEach { suggestion ->
assertTrue(
"suggested $suggestion for $spec is itself invalid",
"suggested $suggestion for $spec on $probe is itself invalid",
ContainerCapabilities.validate(suggestion, probe).isValid,
)
}