JMR-dev/LibreMediaConverter
1
0
Fork 0
Code Issues 16 Pull Requests Actions Packages Projects Releases Wiki Activity

The advisory baseline check counts a KDoc as a fourth marked test, so it deviates on every PR #120

New Issue
Closed
opened 2026-08-26 02:57:37 +00:00 by JMR-dev · 1 comment
JMR-dev commented 2026-08-26 02:57:37 +00:00 (Migrated from github.com)
Copy Link
Copy Source

The advisory job's baseline check emits a false deviation on every PR, on
main as of 62040b2:

baseline DEVIATION: the tree carries 4 tests marked `@FailsOnEmulatorApi37`
                    but the baseline says 3 — update FAILS_ON_EMULATOR_API37_BASELINE

There are three. .github/scripts/e2e-report-shape.sh:186 counts with:

grep -rn "@FailsOnEmulatorApi37" "$REPO_ROOT/app/src/androidTest" --include='*.kt' \
  | grep -v import | grep -c FailsOn

which matches the string anywhere, including inside a KDoc. The fourth "test" it
finds is a comment added by #113 saying the opposite:

Media3EngineTest.kt:72:      @FailsOnEmulatorApi37
Media3EngineTest.kt:135:     @FailsOnEmulatorApi37
Media3EngineTest.kt:195:      * Deliberately not `@FailsOnEmulatorApi37`: nothing here decodes or encodes, so no emulator
SafPickerRoundTripTest.kt:300:  @FailsOnEmulatorApi37

Real annotations: 3. FAILS_ON_EMULATOR_API37_BASELINE: 3. They agree; only the
counter disagrees with both.

Why this matters more than a wrong number. #83 added this so a new failure
could not be invisible. A deviation notice that is wrong on every PR trains
everyone to skim past deviation notices, which is exactly the signal it was built
to create. It also cannot be resolved by editing the baseline — setting it to 4
would make the real check wrong and would fire the moment someone adds or
removes a genuine marker.

Neither ingredient is at fault on its own: #111's counter and #113's KDoc are
both reasonable, and they were written days apart. This is the seam between them.

The trap in fixing it. The obvious repair — count only lines that are just
the annotation — must not also stop counting a legitimately-placed one. Kotlin
allows @FailsOnEmulatorApi37 @Test fun x() on a single line, and an annotation
indented inside a nested class still counts. Whatever the new matcher is, it
needs a case for each of those, not only for the comment it is trying to exclude.

Test: e2e-report-shape.sh is exercised against captured CI output rather
than a live emulator — that is how #111 was verified and this host cannot run API
37 at all. The check is a pure function of the working tree, so it can also be
exercised directly against fixture files: a real annotation, a same-line
@FailsOnEmulatorApi37 @Test, a KDoc mentioning it, and an import.

Nothing here should change any job's status or the pass/fail rules — the
advisory job is continue-on-error and stays red by design. This is about
whether its notices can be trusted.

Sequencing: #118 is being worked in this same script right now. This should
land after it to avoid a conflict.

The advisory job's baseline check emits a **false** deviation on every PR, on `main` as of `62040b2`: ``` baseline DEVIATION: the tree carries 4 tests marked `@FailsOnEmulatorApi37` but the baseline says 3 — update FAILS_ON_EMULATOR_API37_BASELINE ``` There are three. `.github/scripts/e2e-report-shape.sh:186` counts with: ```sh grep -rn "@FailsOnEmulatorApi37" "$REPO_ROOT/app/src/androidTest" --include='*.kt' \ | grep -v import | grep -c FailsOn ``` which matches the string anywhere, including inside a KDoc. The fourth "test" it finds is a comment added by #113 saying the opposite: ``` Media3EngineTest.kt:72: @FailsOnEmulatorApi37 Media3EngineTest.kt:135: @FailsOnEmulatorApi37 Media3EngineTest.kt:195: * Deliberately not `@FailsOnEmulatorApi37`: nothing here decodes or encodes, so no emulator SafPickerRoundTripTest.kt:300: @FailsOnEmulatorApi37 ``` Real annotations: 3. `FAILS_ON_EMULATOR_API37_BASELINE`: 3. They agree; only the counter disagrees with both. **Why this matters more than a wrong number.** #83 added this so a new failure could not be invisible. A deviation notice that is wrong on every PR trains everyone to skim past deviation notices, which is exactly the signal it was built to create. It also cannot be resolved by editing the baseline — setting it to 4 would make the *real* check wrong and would fire the moment someone adds or removes a genuine marker. Neither ingredient is at fault on its own: #111's counter and #113's KDoc are both reasonable, and they were written days apart. This is the seam between them. **The trap in fixing it.** The obvious repair — count only lines that are just the annotation — must not also stop counting a legitimately-placed one. Kotlin allows `@FailsOnEmulatorApi37 @Test fun x()` on a single line, and an annotation indented inside a nested class still counts. Whatever the new matcher is, it needs a case for each of those, not only for the comment it is trying to exclude. **Test:** `e2e-report-shape.sh` is exercised against captured CI output rather than a live emulator — that is how #111 was verified and this host cannot run API 37 at all. The check is a pure function of the working tree, so it can also be exercised directly against fixture files: a real annotation, a same-line `@FailsOnEmulatorApi37 @Test`, a KDoc mentioning it, and an import. **Nothing here should change any job's status or the pass/fail rules** — the advisory job is `continue-on-error` and stays red by design. This is about whether its notices can be trusted. **Sequencing:** #118 is being worked in this same script right now. This should land after it to avoid a conflict.
JMR-dev commented 2026-08-26 03:04:34 +00:00 (Migrated from github.com)
Copy Link
Copy Source

A validated matcher, so this does not have to be guessed at

I built a fixture covering every shape the counter has to survive and ran three candidates against
it. Fixture (all five cases, three of which are real annotations):

import org.libremediaconverter.FailsOnEmulatorApi37   // must NOT count

@FailsOnEmulatorApi37
@Test fun ownLine() {}                                 // COUNTS

@FailsOnEmulatorApi37 @Test
fun sameLineWithTest() {}                              // COUNTS — legal Kotlin

/** Deliberately not `@FailsOnEmulatorApi37`: ... */    // must NOT count  <- today's bug
@Test fun kdocMention() {}

// @FailsOnEmulatorApi37  -- commented out              // must NOT count
@Test fun commentedOut() {}

class Nested {
    @FailsOnEmulatorApi37
    @Test fun indentedDeeper() {}                      // COUNTS
}
matcher fixture (want 3) real tree (want 3)
current: grep -v import | grep -c FailsOn 5 4
^[[:space:]]*@FailsOnEmulatorApi37[[:space:]]*$ 2 3
^[[:space:]]*@FailsOnEmulatorApi37([[:space:]]|$) 3 3

The middle one is the trap I flagged when filing, and it is a real trap. "Only the annotation on
its own line" looks like the obvious repair and silently drops
@FailsOnEmulatorApi37 @Test — it would undercount the moment someone writes that, and undercounting
is the direction that hides a genuine new marker. Anchoring at line-start and requiring whitespace or
end-of-line after the name keeps that case and still rejects the KDoc (^\s*@ does not match
* Deliberately not \@...) and the commented-out form (^\s*@does not match// @...`).

The last row is the one to implement. It gives 3 on the real tree, which matches
FAILS_ON_EMULATOR_API37_BASELINE = 3, so the false deviation goes away without touching the
baseline.

Two caveats on that recommendation, so it is not over-trusted:

  • The same-line case is hypothetical in this repo today — grep -rnE '^\s*@[A-Za-z]+\s+@[A-Za-z]'
    finds no instance in androidTest or test. It is legal Kotlin and cheap to keep working, not
    something currently relied on.
  • This is a regex over source text, not a parser. It would still count an annotation inside a
    multi-line string or a /* */ block that begins mid-line. Neither exists here; if that ever
    matters the check wants a different tool, not a longer regex.

Fixture is at scratchpad/baseline-matcher/fixtures.kt in my scratch dir — worth committing
alongside the fix as the test the ticket asks for, since the check is a pure function of the tree.

## A validated matcher, so this does not have to be guessed at I built a fixture covering every shape the counter has to survive and ran three candidates against it. Fixture (all five cases, three of which are real annotations): ```kotlin import org.libremediaconverter.FailsOnEmulatorApi37 // must NOT count @FailsOnEmulatorApi37 @Test fun ownLine() {} // COUNTS @FailsOnEmulatorApi37 @Test fun sameLineWithTest() {} // COUNTS — legal Kotlin /** Deliberately not `@FailsOnEmulatorApi37`: ... */ // must NOT count <- today's bug @Test fun kdocMention() {} // @FailsOnEmulatorApi37 -- commented out // must NOT count @Test fun commentedOut() {} class Nested { @FailsOnEmulatorApi37 @Test fun indentedDeeper() {} // COUNTS } ``` | matcher | fixture (want 3) | real tree (want 3) | |---|---|---| | current: `grep -v import \| grep -c FailsOn` | **5** | **4** | | `^[[:space:]]*@FailsOnEmulatorApi37[[:space:]]*$` | **2** | 3 | | **`^[[:space:]]*@FailsOnEmulatorApi37([[:space:]]\|$)`** | **3** | **3** | **The middle one is the trap I flagged when filing, and it is a real trap.** "Only the annotation on its own line" looks like the obvious repair and silently drops `@FailsOnEmulatorApi37 @Test` — it would undercount the moment someone writes that, and undercounting is the direction that hides a genuine new marker. Anchoring at line-start and requiring whitespace or end-of-line *after* the name keeps that case and still rejects the KDoc (`^\s*@` does not match `* Deliberately not \`@...`) and the commented-out form (`^\s*@` does not match `// @...`). The last row is the one to implement. It gives **3** on the real tree, which matches `FAILS_ON_EMULATOR_API37_BASELINE = 3`, so the false deviation goes away without touching the baseline. **Two caveats on that recommendation, so it is not over-trusted:** - The same-line case is **hypothetical in this repo today** — `grep -rnE '^\s*@[A-Za-z]+\s+@[A-Za-z]'` finds no instance in `androidTest` or `test`. It is legal Kotlin and cheap to keep working, not something currently relied on. - This is a regex over source text, not a parser. It would still count an annotation inside a multi-line string or a `/* */` block that begins mid-line. Neither exists here; if that ever matters the check wants a different tool, not a longer regex. Fixture is at `scratchpad/baseline-matcher/fixtures.kt` in my scratch dir — worth committing alongside the fix as the test the ticket asks for, since the check is a pure function of the tree.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
above-cut
Worked autonomously overnight: local, JVM-verifiable, no product decision
 accessibility
Barrier affecting people with disabilities
 backlog
Held for manual review: product/UX call, CI/workflow, hardware, or unverifiable here
 bug
Something isn't working
 confirmed
Reviewer demonstrated the defect
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 plausible
Reviewer could not fully demonstrate it; treat as unproven
 question
Further information is requested
 sev:high
High severity
 sev:low
Low severity
 sev:medium
Medium severity
 wontfix
This will not be worked on
No labels bug confirmed sev:medium
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
JMR-dev
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#120