From 0f842243b5e8fc22833207c66f6ddb336a99e828 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Tue, 1 Sep 2026 21:48:36 -0500 Subject: [PATCH] B2 (#174): read what the progress notification actually says An assertion gap rather than a coverage one, which is the reason it survived. JaCoCo is green on build()'s `if (indeterminate)` because ProgressNotificationTest drives it through a real worker -- but that test reads the notification id and EXTRA_PROGRESS and nothing else. Nothing had ever read the text. Swapping the two branches passed the whole suite; so did replacing the caller's title with a constant. Both are red now. What it costs to get wrong is small and permanent: a conversion four minutes in still saying "Preparing", or one that has not started reporting yet claiming 0%. Neither is a crash, and nothing else here would have found it. Nothing in the suite had constructed ConversionNotifications directly, and the reason turned out to be mechanical rather than an oversight: build() reaches WorkManager.getInstance for the Cancel action's PendingIntent, so the notification cannot be built without one. installTestWorkManager in setUp is the whole fixture, and the KDoc records the coupling so the next person does not rediscover it. areEnabled() in the same file is deliberately still untested. It has no caller anywhere in app/src/main, so a test would assert that a function nobody calls returns what the platform told it -- and would imply the app handles the disabled-notification case, which it does not. That is F5 in docs/coverage-read-findings.md, and it asks for a decision rather than a test. 561 -> 563 JVM tests, 0 failures. No production code changed. Co-Authored-By: Claude Opus 5 (1M context) --- .../work/NotificationProgressTextTest.kt | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 app/src/test/java/org/libremediaconverter/work/NotificationProgressTextTest.kt diff --git a/app/src/test/java/org/libremediaconverter/work/NotificationProgressTextTest.kt b/app/src/test/java/org/libremediaconverter/work/NotificationProgressTextTest.kt new file mode 100644 index 0000000..7bbef1c --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/work/NotificationProgressTextTest.kt @@ -0,0 +1,89 @@ +package org.libremediaconverter.work + +import android.app.Notification +import androidx.media3.common.util.UnstableApi +import androidx.work.Data +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.libremediaconverter.convert.installTestWorkManager +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import java.util.UUID + +/** + * The two things a progress notification can say, and that they are not the same thing. + * + * An assertion gap rather than a coverage one, and the distinction is the reason this file exists. + * JaCoCo is green on `build`'s `if (indeterminate)`, because `ProgressNotificationTest` drives it + * through a real worker -- but that test reads only the notification id and + * `Notification.EXTRA_PROGRESS`. **Nothing had ever read the text.** Swapping the two branches, or + * collapsing them into one string, passed the entire suite. + * + * What it costs to get wrong is small and constant: a conversion that has been running for four + * minutes still saying "Preparing", or one that has not started reporting yet claiming 0%. Neither + * is a crash, and neither would be found by anything else here -- which is exactly the kind of + * thing that survives for a long time. + * + * Nothing else in the suite constructs [ConversionNotifications] directly. + */ +@UnstableApi +@RunWith(RobolectricTestRunner::class) +class NotificationProgressTextTest { + + /** + * `build` reaches `WorkManager.getInstance` for the Cancel action's PendingIntent, so the + * notification cannot be built at all without one. That coupling is why nothing had ever + * constructed this class directly and read what it produced. + */ + @Before + fun setUp() { + installTestWorkManager(RuntimeEnvironment.getApplication(), Data.EMPTY) + } + + @Test + fun `an indeterminate notification says something different from a measured one`() { + val context = RuntimeEnvironment.getApplication() + val notifications = ConversionNotifications(context) + + val preparing = notifications.build(JOB_ID, TITLE, percent = 0, indeterminate = true).text() + val measured = notifications.build(JOB_ID, TITLE, percent = 42, indeterminate = false).text() + + assertNotEquals( + "the two states have to read differently, or the text says nothing at all", + preparing, + measured, + ) + assertTrue( + "a measured notification has to carry its percentage, got \"$measured\"", + measured.contains("42"), + ) + assertTrue( + "an indeterminate one must not invent one, got \"$preparing\"", + !preparing.contains("42") && !preparing.contains("0"), + ) + } + + /** + * The title is the caller's, not the builder's -- it is the file the user picked, and it is what + * tells two simultaneous conversions apart in the shade. + */ + @Test + fun `the notification is titled with the file it is converting`() { + val context = RuntimeEnvironment.getApplication() + + val built = ConversionNotifications(context).build(JOB_ID, TITLE, percent = 10) + + assertEquals(TITLE, built.extras.getString(Notification.EXTRA_TITLE)) + } + + private fun Notification.text(): String = extras.getString(Notification.EXTRA_TEXT).orEmpty() + + private companion object { + const val TITLE = "holiday.mp4" + val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-00000000000a") + } +}