Re-derive the API 37 carrier counts, and correct what #226 left behind

The 2026-09-06 re-check of the instrumented suite. Every drifted line it found came from #226, the last PR of the e2e read's own wave. The suite is 70 tests in 14 classes, 6 carrying @FailsOnEmulatorApi37, gating leg 64. The committed baseline says 6 and the advisory job agrees. Four places still said five carriers of 69: - CLAUDE.md, three sites - FailsOnEmulatorApi37.kt's KDoc - two comments in status_check.yml The gating figure is what hid it. 69 - 5 and 70 - 6 are both 64, so the one number a reader checks against a run had not moved -- which is exactly why CLAUDE.md says to derive these rather than remember them. Two KDoc claims in SafPickerRoundTripTest described a draft rather than the code. The save test says MP3 was chosen so the setup could not depend on device codecs; the code converts at the default MP4_H265/FAST, which routes on canEncode(H265). The negation of the stated reason was true. That is E1 and E3's failure mode committed by the wave that found it, so it is written down as such rather than quietly corrected. Neither picker test has ever reported on the advisory leg. The marker's KDoc said the picker test fails there behind the rotation test; with six carriers the rotation test truncates the run first, and both advisory runs since #226 -- 34042397320 and 34043502322 -- report expected: 6, received: 4, the four being the three Media3 tests plus the rotation. The save test is therefore marked by inheritance, not measurement, and both KDocs now say so. FixtureDocumentsProvider.deletedDocumentIds() has no callers: #226 proved D4's premise and drove only the success path, so deletePartialOutput against a real DocumentsProvider is still asserted nowhere. Filed as #250 with the forcing condition and the mutation; the accessor is kept with a KDoc naming that ticket rather than removed and re-added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-06 11:16:15 -05:00
parent 1f21557b8d
commit 54167c052f
6 changed files with 154 additions and 44 deletions
@@ -9,14 +9,22 @@ package org.libremediaconverter
 * That is the whole reason there is one annotation rather than a pair of test lists: two lists
 * drift, and the drift is silent in both directions (a test that runs nowhere reads as green).
 *
- * **"Cannot be run" covers two things, and it said only the first until 2026-09-05.** Four of the
- * five carriers simply fail: three Media3 tests die in the image's own `c2.goldfish.h264.decoder`,
- * and the SAF rotation test takes the framework down with it. The fifth —
- * `SafPickerRoundTripTest.pickingAFileThroughTheSystemPickerFillsInTheFileCard` — **passes about
- * half the time and aborts `system_server` every time**, which is worse for a gating leg than an
- * honest failure: it fails the leg from the teardown, with no failing test to point at (#108).
- * The wording was widened rather than the test excused; that test's own KDoc has the four-run
- * measurement.
+ * **"Cannot be run" covers three things now, and it covered only the first until 2026-09-05.**
+ * Four of the six carriers simply fail: three Media3 tests die in the image's own
+ * `c2.goldfish.h264.decoder`, and the SAF rotation test takes the framework down with it. The
+ * fifth — `SafPickerRoundTripTest.pickingAFileThroughTheSystemPickerFillsInTheFileCard` —
+ * **passes about half the time and aborts `system_server` every time**, which is worse for a
+ * gating leg than an honest failure: it fails the leg from the teardown, with no failing test to
+ * point at (#108). The wording was widened rather than the test excused; that test's own KDoc has
+ * the four-run measurement.
+ *
+ * **The sixth is the new third thing: it is marked by inheritance, not by measurement.**
+ * `SafPickerRoundTripTest.aSaveWritesToTheDocumentTheSystemPickerCreated` (#226) opens the same
+ * picker and then a second DocumentsUI dialog on top of it, so it sits on the same task-snapshot
+ * path its sibling was marked for. It has never been observed at API 37 either way — see the
+ * measurement under [FAILS_ON_EMULATOR_API37_BASELINE], which is why it cannot be. Marking it was
+ * the conservative choice, and **the trigger for revisiting it is the rotation test, not itself**:
+ * while that one truncates the advisory run, nothing downstream of it can report.
 *
 * It says only what has been measured: **on the emulator, at API 37.** The same tests pass on a
 * physical Pixel 10 Pro XL at API 37 and at API 33–36 on the same runner under the same renderer,
@@ -46,19 +54,21 @@ annotation class FailsOnEmulatorApi37
 * keep printing with nothing to compare to, so it announces that it could not read the baseline
 * rather than falling quiet. If you see that notice, this line is what it means.
 *
- * **One number, both checks, and that is what the marker means.** A test carrying it cannot be run
- * on this image, so the count is simultaneously how many the advisory leg runs and how many fail.
- * A *smaller* failure count is the interesting direction: it means one of them now passes, which
- * is the trigger the KDoc above names for deleting the annotation.
+ * **One number, both checks, and that is what the marker was meant to mean.** A test carrying it
+ * cannot be run on this image, so the count is meant to be simultaneously how many the advisory
+ * leg runs and how many fail. A *smaller* failure count is the interesting direction: it means one
+ * of them now passes, which is the trigger the KDoc above names for deleting the annotation.
+ * **Since 2026-09-06 the second half no longer holds in practice** — the run truncates before two
+ * of the six start, which the last paragraph below measures. `expected` still holds, and it is the
+ * field that catches a marker added without changing this number.
 *
- * **The picker test is the one to read that sentence carefully for.**
- * `pickingAFileThroughTheSystemPickerFillsInTheFileCard` was marked on 2026-09-05 for aborting
- * `system_server` rather than for failing (#108), and on the gating leg it passed two runs of
- * four. It fails on the advisory leg because the rotation test runs before it and takes the
- * framework down first — measured, `api37-debug.yml` run 34008889182, which reports
- * `expected: 4, received: 4, failed: 4` with the four in the order Media3, Media3, rotation,
- * picker. (Those dispatches predate the third Media3 marker landing on `main`, so their totals
- * are four rather than five; the ordering they establish is what matters here.)
+ * **The picker tests are the ones to read that sentence carefully for, and the reason changed
+ * on 2026-09-06.** `pickingAFileThroughTheSystemPickerFillsInTheFileCard` was marked on
+ * 2026-09-05 for aborting `system_server` rather than for failing (#108), and on the gating leg
+ * it passed two runs of four. It was recorded here as *failing* on the advisory leg, behind the
+ * rotation test — measured, `api37-debug.yml` run 34008889182, `expected: 4, received: 4,
+ * failed: 4`, in the order Media3, Media3, rotation, picker. (Those dispatches predate the third
+ * Media3 marker, so their totals are four rather than six.)
 *
 * **But a second dispatch of the identical configuration reported 4/3/3**, having lost the last
 * test to the abort rather than to anything about the test list, and that is why
@@ -68,6 +78,13 @@ annotation class FailsOnEmulatorApi37
 * reporting fewer failures than this as one of them now passing; read a truncated one as the
 * framework having died, which is this job's normal.
 *
+ * **That is no longer what happens, and the difference is that neither picker test reports at
+ * all.** With six carriers the rotation test truncates the run before them: both advisory runs
+ * since #226 landed — 34042397320 and 34043502322 — report `expected: 6, received: 4`, and the
+ * four are the three Media3 tests plus the rotation. So the advisory leg currently answers for
+ * four of its six, and the comparison below is unaffected only because `failed` is not compared
+ * on a truncated run. Read it as **unmeasured**, not as passing or failing.
+ *
 * So: adding or removing a [FailsOnEmulatorApi37] means changing this number, in this file, in
 * the same diff. The report says so on the run itself if you forget — it prints the tree's own
 * `grep` count beside this one.
@@ -245,7 +245,17 @@ public final class FixtureDocumentsProvider extends DocumentsProvider {
        destinationFile(documentId).delete();
    }

-    /** Document ids {@link #deleteDocument} was called with, newest last. */
+    /**
+     * Document ids {@link #deleteDocument} was called with, newest last.
+     *
+     * <p><b>Nothing reads this yet, and that is recorded rather than hidden (#250).</b> It was
+     * added with #226 to assert {@code OutputPublisher.deletePartialOutput} — D4's cleanup — against
+     * a real {@code DocumentsProvider}. #226 only reached the <i>success</i> path, so the
+     * {@code catch} that calls it is still asserted only against {@code FakeSafProvider} under
+     * Robolectric. It is kept because the forcing condition is one {@code openDestination} override
+     * away and #250 says exactly what to add; if that ticket is closed any other way, delete this
+     * and {@link #DELETED} with it rather than leaving an accessor implying coverage.
+     */
    public static List<String> deletedDocumentIds() {
        synchronized (DELETED) {
            return new ArrayList<>(DELETED);
@@ -515,9 +515,28 @@ class SafPickerRoundTripTest {
     *
     * ## The conversion is setup, not subject
     *
-     * Save is only offered on `Converted`, so the test converts first. MP3 is chosen because the
-     * router sends it to FFmpeg unconditionally at every API level, so the setup cannot depend on
-     * the device's codecs — #223 is what that costs.
+     * Save is only offered on `Converted`, so the test converts first, at the screen's default
+     * `MP4_H265` / `FAST`. That is **not** codec-independent, and this KDoc claimed the opposite
+     * until 2026-09-06: an earlier draft used MP3 for exactly that reason, and the format had to
+     * move for a different constraint the picker imposes — [convertToTheDefaultFormat] has it.
+     * `MP4_H265` at `FAST` reaches `ConversionRouter`'s `canEncode(H265)` gate, so it runs on
+     * FFmpeg on the emulators (no hardware H265) and on Media3 on the Pixel.
+     *
+     * **That is tolerable here, and #223 is the reason it needs saying.** There, the routing
+     * decided whether the *subject* was reached, so a route to FFmpeg made the test pass while
+     * proving nothing. Here the conversion is setup: if it goes the other way and fails, this test
+     * fails loudly on the setup rather than quietly on the assertion. The subject is what `publish`
+     * was handed, which the engine that produced the file does not touch.
+     *
+     * ## Why it carries [FailsOnEmulatorApi37]
+     *
+     * By inheritance, not measurement. It opens the same picker as
+     * [pickingAFileThroughTheSystemPickerFillsInTheFileCard], which was marked for aborting
+     * `system_server` from the task-snapshot path (#108), and then a second DocumentsUI dialog on
+     * top of it. It has never been observed at API 37 either way: the rotation test truncates the
+     * advisory run first, so both advisory runs since this landed report `expected: 6, received: 4`
+     * without reaching either picker test. Marking it was the conservative choice and it is
+     * recorded as unmeasured in `FailsOnEmulatorApi37.kt` rather than dressed up as a measurement.
     */
    @Test
    @FailsOnEmulatorApi37
@@ -1060,7 +1079,13 @@ class SafPickerRoundTripTest {
        /** Short: either the dialog is up almost immediately, or the permission was already held. */
        const val PERMISSION_DIALOG_MS = 5_000L

-        /** A 3 s clip to MP3 on an emulator is about a second; this only bounds a hang. */
+        /**
+         * Only bounds a hang, and it is two orders of magnitude clear of the real cost: the whole
+         * test — pick, convert, save — takes **11.8 s** on the API 34 CI leg (run 34043502322).
+         * Deliberately generous because the engine is not fixed: the default `MP4_H265` at `FAST`
+         * lands on FFmpeg on an emulator and on Media3 on real hardware, which is faster rather
+         * than slower — see [convertToTheDefaultFormat].
+         */
        const val CONVERSION_TIMEOUT_MS = 120_000L

        /** The copy is a few kilobytes, but it crosses a provider. */