Compare commits

..
Author SHA1 Message Date
Jason Ross 4d21996735 Merge branch 'main' into feat/expedited-conversion-work 2026-09-07 11:11:53 -05:00
Jason Ross 264b8027e4 Merge pull request #260 from JMR-dev/fix/gate-cache-in-worktrees
Resolve the gate's cache dir with --git-common-dir, and say when it cannot (#258)
2026-09-06 18:02:22 -05:00
JMR-devandClaude Opus 5 6a8cc01862 Expedite user-initiated work, and give both getForegroundInfo overrides a caller (#252)
`ConversionWorker.request` and `ConcatWorker.request` now carry
`setExpedited(RUN_AS_NON_EXPEDITED_WORK_REQUEST)`. Conversions and joins are
started by a tap; the jobs that have to go back through JobScheduler because no
process is left to start them should not queue behind a background chore. The
class KDoc that said expedited was "deliberately not used" is replaced with what
was actually read out of work-runtime 2.11.2: retries are never expedited
(`SystemJobInfoConverter:135`), and a job the system stops mid-run is resolved as
`ResetWorkerStatus` and re-enqueued rather than answered by `FailureOutcome`.

#252's own premise does not survive measurement, and that is the second half of
this change. `getForegroundInfo()` is WorkManager's expedited-work hook, but
`WorkForeground.kt:38` opens the library's only caller with
`if (!spec.expedited || Build.VERSION.SDK_INT >= 31) return`, and minSdk is 33 --
so `setExpedited` alone leaves both overrides exactly as cold as the first
instrumented coverage read found them. Measured on API 34 rather than argued:
with the flag set and `doWork` still building its own notification, both methods
report `missed 1 / covered 0` and all ten lines `ci=0`, and the whole
instrumented suite is green anyway at 71/71.

What makes them live is that each worker held two definitions of one
notification. `doWork` now posts the override's instead of an identical copy, so
`ConcatWorker`'s countless "Joining files" -- which nothing executed and which
was therefore free to drift from the "Joining N files" that ran -- is gone.
After: both `getForegroundInfo` report `LINE 0 missed / 5 covered`.

Five mutations were run and all five went red: dropping `setExpedited` from
either request, hard-coding the conversion title, moving its `percent` off zero,
and dropping the join's input count.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-06 17:06:00 -05:00
8 changed files with 460 additions and 43 deletions
@@ -8,6 +8,7 @@ import androidx.work.CoroutineWorker
import androidx.work.Data
import androidx.work.ForegroundInfo
import androidx.work.OneTimeWorkRequestBuilder
import androidx.work.OutOfQuotaPolicy
import androidx.work.WorkerParameters
import androidx.work.hasKeyWithValueOfType
import androidx.work.workDataOf
@@ -27,6 +28,10 @@ import org.libremediaconverter.model.OutputFormat
* Progress is not reported. FFmpeg's statistics callback gives a timestamp against a
* single input's duration, which is meaningless once several files are being
* concatenated; showing a fabricated percentage would be worse than showing none.
*
* Enqueued as **expedited** work for the same reasons, and with the same caveats, as
* [ConversionWorker] — its class KDoc carries both, and a join is user-initiated in exactly the
* way a conversion is.
*/
@UnstableApi
class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker(context, params) {
@@ -68,13 +73,10 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
// which is where a WorkManager restart after process death always begins -- used to
// throw straight past this catch, taking the retry, the error message and the delete
// with it. See ConversionWorker.doWork and FailureOutcome.
setForeground(
ForegroundInfo(
NOTIFICATION_ID,
notifications.build(id, "Joining ${uris.size} files", 0, indeterminate = true),
ConversionForegroundType.current(),
),
)
//
// Posted through getForegroundInfo() rather than built here a second time -- see that
// override, and its twin in ConversionWorker.
setForeground(getForegroundInfo())
val result = ConversionDependencies.concat(applicationContext).join(uris, staged, format)
Result.success(
@@ -129,11 +131,29 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
return publisher.hasSpaceFor(bytes)
}
override suspend fun getForegroundInfo(): ForegroundInfo = ForegroundInfo(
NOTIFICATION_ID,
notifications.build(id, "Joining files", 0, indeterminate = true),
ConversionForegroundType.current(),
)
/**
* The notification a starting join posts, and now the only definition of it.
*
* WorkManager's hook for expedited work, which **will not call this on any device this app
* supports** — see [ConversionWorker.getForegroundInfo] for the measurement and for why
* `setExpedited` alone would have left these lines exactly as cold as they were. What makes
* them live is [doWork] posting this instead of building its own copy.
*
* It counts the inputs itself rather than being handed the number, so that it is still answerable
* before [doWork] has parsed anything — which is the contract WorkManager's own caller wants.
* The count is read from the same key, so the two cannot disagree. The `?: 0` arm is
* unreachable and named rather than covered: [doWork] refuses a job with no URI array several
* lines above this call, and nothing else calls it. It is the shape `docs/coverage-read-findings.md`
* calls F4 — a second line of defence that cannot be provoked.
*/
override suspend fun getForegroundInfo(): ForegroundInfo {
val inputCount = inputData.getStringArray(KEY_INPUT_URIS)?.size ?: 0
return ForegroundInfo(
NOTIFICATION_ID,
notifications.build(id, joiningTitle(inputCount), 0, indeterminate = true),
ConversionForegroundType.current(),
)
}
companion object {
/**
@@ -204,6 +224,16 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
*/
fun outputNameFor(format: OutputFormat): String = "joined.${format.extension}"
/**
* What the progress notification says while a join runs.
*
* Named once, for the convention #158 established about strings the user can see. It was
* two strings until 2026-09-06 — `"Joining N files"` built inline in [doWork] and a
* countless `"Joining files"` in [getForegroundInfo] — for one notification that only ever
* had one job, and the copy nothing executed was free to drift from the one that did.
*/
fun joiningTitle(inputCount: Int): String = "Joining $inputCount files"
private const val NOTIFICATION_ID = 1002
private const val TAG = "ConcatWorker"
@@ -215,6 +245,10 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
*/
fun request(inputs: List<Uri>, totalBytes: Long?, format: OutputFormat = DEFAULT_FORMAT) =
OneTimeWorkRequestBuilder<ConcatWorker>()
// Expedited, exactly as ConversionWorker.request is and for the same reasons; that
// one's comment and class KDoc carry them. Nothing here sets an initial delay or a
// constraint, which is what makes it legal for `build()` to accept.
.setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST)
.addTag(JobTags.inputCount(inputs.size))
.setInputData(
Data.Builder()
@@ -8,6 +8,7 @@ import androidx.work.CoroutineWorker
import androidx.work.Data
import androidx.work.ForegroundInfo
import androidx.work.OneTimeWorkRequestBuilder
import androidx.work.OutOfQuotaPolicy
import androidx.work.WorkerParameters
import androidx.work.hasKeyWithValueOfType
import androidx.work.workDataOf
@@ -40,8 +41,28 @@ import java.io.File
* observe. That durability is what makes the six-hour foreground-service timeout
* recoverable instead of fatal.
*
* Expedited work is deliberately *not* used. It maps to JobScheduler expedited jobs
* with a short quota, which is the wrong shape for a multi-minute transcode.
* Enqueued as **expedited** work, with `RUN_AS_NON_EXPEDITED_WORK_REQUEST`. This paragraph said
* the opposite until 2026-09-06 — "deliberately *not* used… the wrong shape for a multi-minute
* transcode" — and the quota it named does not reach a transcode the way it reads:
*
* - The quota belongs to the *JobScheduler* job, and `SystemJobInfoConverter:135` in
* work-runtime 2.11.2 sets `JobInfo.setExpedited(true)` only when `!isRetry && !isDelayed`.
* A retry is therefore scheduled exactly as every job is scheduled today.
* - A job the system stops mid-run does not get its answer from [FailureOutcome].
* `WorkerWrapper.interrupt` cancels the worker's coroutine with a `WorkerStoppedException`,
* which its `launch` resolves as `ResetWorkerStatus` — the worker's own `Result` is discarded
* and the work re-enqueued with backoff, whatever it returned. So a quota stop is a retry, and
* the `CancellationException` arm in [doWork] is what deletes the partial on the way through.
*
* What it buys is narrower than "conversions start sooner", and the narrowness is the honest part:
* `GreedyScheduler` starts unconstrained, undelayed work in-process the moment it is enqueued and
* carries no `expedited` branch at all, so a conversion begun from the open app runs exactly when
* it ran before — the common case does not move. The flag is for the job that has to go *through*
* JobScheduler because no process is left to start it: one still enqueued when the app died.
* `SystemJobScheduler.schedule` re-converts the spec every time it schedules, so such a job is
* expedited on the way back in, and a retried one is not. `RUN_AS_NON_EXPEDITED_WORK_REQUEST`
* rather than `DROP_WORK_REQUEST`: an invisible quota is no reason to throw a user's conversion
* away, and `SystemJobScheduler:198` degrades it to an ordinary job instead.
*
* That durability is not free, and the queue surviving is not the same as the job surviving.
* When WorkManager recovers a job after process death the app is by definition in the background,
@@ -60,7 +81,7 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
override suspend fun doWork(): Result {
val inputUri = inputData.getString(KEY_INPUT_URI)?.let(Uri::parse)
?: return Result.failure(workDataOf(KEY_ERROR to "No input file."))
val displayName = inputData.getString(KEY_DISPLAY_NAME) ?: "input"
val displayName = displayName()
// Absent, not zero, when nobody could say -- see InputQuery. `getLong(key, 0L)` is what
// made those two the same number, and `hasSpaceFor(0)` is only "is there 128 MB free".
val declaredSize = inputData
@@ -100,7 +121,10 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
// process death is. With it above the try that throw escaped doWork() entirely: no
// retry, no error in the output Data, and no staged.delete(). MediaProbe.probe below
// was outside for the same reason and had the same problem.
setForeground(foregroundInfo(displayName, percent = 0, indeterminate = true))
//
// Posted through getForegroundInfo() rather than built here a second time -- see that
// override for what the duplicate cost.
setForeground(getForegroundInfo())
// Through the seam rather than MediaProbe directly. The seam already existed for the
// ViewModel and the worker was the last caller bypassing it, which is why nothing on
@@ -339,8 +363,32 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
return OutputSpec(container, video, audio)
}
/**
* What this job's input is called, or [InputQuery.FALLBACK_DISPLAY_NAME] when nothing named it.
*
* One read rather than the two copies of `?: "input"` that [doWork] and [getForegroundInfo]
* each carried, and against `InputQuery`'s constant rather than a third literal of the same
* string: it is the same fallback the picker uses, and it reaches the save dialog as
* `input_converted.mp4`.
*/
private fun displayName(): String = inputData.getString(KEY_DISPLAY_NAME) ?: InputQuery.FALLBACK_DISPLAY_NAME
/**
* The notification a starting conversion posts, and now the only definition of it.
*
* This is WorkManager's hook for expedited work, and **it will not be called on any device
* this app supports.** `WorkForeground.kt:38` in work-runtime 2.11.2 opens with
* `if (!spec.expedited || Build.VERSION.SDK_INT >= 31) return`, that function is the library's
* only caller of `getForegroundInfoAsync()`, and `minSdk` is 33. So #252's premise — that
* enqueueing expedited work would make these lines live — is false, and `setExpedited` alone
* would have left them exactly as cold as the first instrumented coverage read found them.
*
* What makes them live is [doWork] posting *this* instead of building its own copy. The two
* were identical — same title, `percent = 0`, `indeterminate = true` — so one was a duplicate
* that could drift, and the one nothing executed is the one that would have drifted silently.
*/
override suspend fun getForegroundInfo(): ForegroundInfo = foregroundInfo(
inputData.getString(KEY_DISPLAY_NAME) ?: "input",
displayName(),
percent = 0,
indeterminate = true,
)
@@ -416,6 +464,12 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
quality: QualityTier = QualityTier.FAST,
enginePreference: EnginePreference = EnginePreference.AUTO,
) = OneTimeWorkRequestBuilder<ConversionWorker>()
// Expedited, so the jobs that do go through JobScheduler are treated as the
// user-initiated work they are -- see the class KDoc for what that is and is not worth.
// Safe to set here and only because of what this builder does not do: `build()` refuses
// an expedited request carrying an initial delay or any constraint but network and
// storage, and none of the three is set below.
.setExpedited(OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST)
.addTag(JobTags.displayName(displayName))
// Neither the tag nor the Data entry is written for a size nobody knows. A `Data` has
// no null, so the absence of the key *is* the unknown — and a tag reading
@@ -0,0 +1,102 @@
package org.libremediaconverter.work
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import androidx.work.Constraints
import androidx.work.OutOfQuotaPolicy
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
/**
* Both workers enqueue **expedited** work, and stay legal doing it.
*
* The two questions are separate and only one of them is about the flag.
*
* - **Is it set.** `expedited` is `false` by default, so `assertTrue` here is what a deleted
* `setExpedited(...)` reddens. That mutation was run.
* - **Is it legal.** `WorkRequest.Builder.build()` refuses an expedited request that carries an
* initial delay or any constraint but network and storage — `require(workSpec.initialDelay <= 0)
* { "Expedited jobs cannot be delayed" }` in work-runtime 2.11.2. Neither `request` sets either
* today, so both `build()` calls pass and the `IllegalArgumentException` is a *future* hazard
* rather than a current one. The delay and constraints assertions below are what name it: add a
* delay to either builder and this class fails on the throw, in the same second, instead of the
* app failing to enqueue a conversion on a device.
*
* **The policy assertion bites less than it reads, and that is worth writing down rather than
* leaving to be rediscovered.** `WorkSpec.outOfQuotaPolicy` *defaults* to
* `RUN_AS_NON_EXPEDITED_WORK_REQUEST`, so it is already this value on a request that was never
* expedited at all — deleting `setExpedited` does not redden it. What it does pin is the one
* alternative: `DROP_WORK_REQUEST` throws a user's conversion away because an invisible quota ran
* out, and that mutation *is* red here.
*
* The delay is not hypothetical either. Three tests deliberately build a delayed request to hold a
* job in `ENQUEUED` — `NotificationCancelActionTest`, `ReattachOnLaunchTest` and
* `CancelReachesWorkManagerTest` — and every one of them builds its own
* `OneTimeWorkRequestBuilder` rather than adding a delay to what `request` returns. That is why
* making these expedited broke none of them; the one that starts from `request` takes only
* `base.workSpec.input` from it.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class ExpeditedRequestTest {
@Test
fun `a conversion is enqueued as expedited work`() {
val spec = ConversionWorker.request(INPUT, DISPLAY_NAME, INPUT_BYTES).workSpec
assertTrue("a conversion the user asked for has to be expedited work", spec.expedited)
assertEquals(
"a quota nobody can see is no reason to drop a conversion",
OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST,
spec.outOfQuotaPolicy,
)
}
@Test
fun `a join is enqueued as expedited work`() {
val spec = ConcatWorker.request(listOf(INPUT, SECOND_INPUT), TOTAL_BYTES).workSpec
assertTrue("a join the user asked for has to be expedited work", spec.expedited)
assertEquals(
"a quota nobody can see is no reason to drop a join",
OutOfQuotaPolicy.RUN_AS_NON_EXPEDITED_WORK_REQUEST,
spec.outOfQuotaPolicy,
)
}
/**
* The two properties that keep `build()` from throwing, asserted on both requests at once
* because the rule is WorkManager's rather than either worker's.
*/
@Test
fun `neither expedited request carries what would make it illegal`() {
val requests = listOf(
ConversionWorker.request(INPUT, DISPLAY_NAME, INPUT_BYTES).workSpec,
ConcatWorker.request(listOf(INPUT, SECOND_INPUT), TOTAL_BYTES).workSpec,
)
requests.forEach { spec ->
assertEquals(
"expedited work cannot be delayed: ${spec.workerClassName}",
0L,
spec.initialDelay,
)
assertEquals(
"expedited work takes only network and storage constraints: ${spec.workerClassName}",
Constraints.NONE,
spec.constraints,
)
}
}
private companion object {
val INPUT: Uri = Uri.parse("file:///tmp/holiday.mp4")
val SECOND_INPUT: Uri = Uri.parse("file:///tmp/holiday2.mp4")
const val DISPLAY_NAME = "holiday.mp4"
const val INPUT_BYTES = 1_024L
const val TOTAL_BYTES = 2_048L
}
}
@@ -0,0 +1,203 @@
package org.libremediaconverter.work
import android.app.Application
import android.app.Notification
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import androidx.work.testing.TestListenableWorkerBuilder
import androidx.work.workDataOf
import kotlinx.coroutines.runBlocking
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.convert.ConcatJoiner
import org.libremediaconverter.convert.ConversionDependencies
import org.libremediaconverter.convert.installTestWorkManager
import org.libremediaconverter.ffmpeg.ConcatEngine
import org.libremediaconverter.model.ConcatStrategy
import org.libremediaconverter.model.DeviceCodecs
import org.libremediaconverter.model.EnginePreference
import org.libremediaconverter.model.InputProbe
import org.libremediaconverter.model.OutputFormat
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import java.io.File
import java.util.UUID
/**
* The first thing either worker posts is what its own `getForegroundInfo()` builds.
*
* **Both overrides were dead code until 2026-09-06, and #252 is where that was found** — the first
* instrumented coverage read reported `ConversionWorker:342-346` and `ConcatWorker:132-136` among
* the 32 lines *neither* suite reaches. The ticket's premise was that enqueueing expedited work
* would make them live, since `getForegroundInfo()` is WorkManager's expedited-work hook.
*
* **That premise is false at this `minSdk`, which is the finding underneath the fix.**
* `WorkForeground.kt:38` in work-runtime 2.11.2 opens `workForeground` with
* `if (!spec.expedited || Build.VERSION.SDK_INT >= 31) return`, that function is the library's only
* caller of `getForegroundInfoAsync()`, and `minSdk` is 33. So `setExpedited` alone would have left
* both overrides exactly as cold as the read found them, and a test written to drive them through
* WorkManager would be testing a code path no device this app supports can take — E1's failure
* mode, where a test asserts and never reaches.
*
* What makes them live is a single-definition change instead. Each worker had **two** definitions
* of one notification: the override, and an identical `ForegroundInfo` built inline in `doWork`.
* `doWork` now posts the override's, so the copy nothing executed is gone and the one that remains
* runs on every job.
*
* These tests are what hold that wiring. Each asserts the notification's *contents* against
* constants rather than against `worker.getForegroundInfo()` — comparing the two would move
* together under every mutation and stay green — and the mutations that redden them are named on
* each test.
*/
@UnstableApi
@RunWith(RobolectricTestRunner::class)
class ForegroundNotificationTest {
private lateinit var app: Application
private lateinit var updater: RecordingForegroundUpdater
@Before
fun setUp() {
app = RuntimeEnvironment.getApplication()
updater = RecordingForegroundUpdater()
ConversionDependencies.publisher = { AlwaysRoomPublisher(app) }
ConversionDependencies.probe = { _, _ -> InputProbe() }
ConversionDependencies.deviceCodecs = { DeviceCodecs.PERMISSIVE }
ConversionDependencies.software = { WritingTranscoder }
ConversionDependencies.concat = { WritingJoiner }
// The notification carries a WorkManager cancel PendingIntent, so without this the worker
// fails building the notification rather than on anything these tests are about.
installTestWorkManager(app, Data.EMPTY)
}
@After
fun tearDown() {
ConversionDependencies.reset()
}
/**
* Mutation that must go red, and did: inside `ConversionWorker.getForegroundInfo`, replace
* `displayName()` with a literal, or `percent = 0` with anything else. Both are in the override's
* own body, so a red here is proof `doWork` executes it rather than a copy of it.
*/
@Test
fun `a conversion's first foreground post is the one getForegroundInfo builds`() {
runBlocking { conversionWorker().doWork() }
val first = updater.infos.first()
val extras = first.notification.extras
assertEquals(
"the notification has to name the file the user picked",
DISPLAY_NAME,
extras.getString(Notification.EXTRA_TITLE),
)
assertEquals("a conversion starts at zero", 0, extras.getInt(Notification.EXTRA_PROGRESS))
assertTrue(
"nothing is known about the length of the job yet, so the bar is indeterminate",
extras.getBoolean(Notification.EXTRA_PROGRESS_INDETERMINATE),
)
assertEquals(
"the foreground service type is the regime's, not zero",
ConversionForegroundType.current(),
first.foregroundServiceType,
)
}
/**
* The count is the point.
*
* `getForegroundInfo` said `"Joining files"` and `doWork` said `"Joining N files"` — one
* notification with two texts, and the one nothing ran was free to drift. Now there is one,
* and it reads the input array itself so it can still answer before `doWork` has parsed
* anything.
*
* Mutation that must go red, and did: replace the array read in `ConcatWorker.getForegroundInfo`
* with a constant `0`, which yields `"Joining 0 files"`. Asserting merely that the title starts
* with "Joining" would survive that, which is why the whole string is pinned.
*/
@Test
fun `a join's first foreground post counts the files it was given`() {
runBlocking { joinWorker().doWork() }
val first = updater.infos.first()
assertEquals(
"the notification has to say how many files are being joined",
ConcatWorker.joiningTitle(INPUTS.size),
first.notification.extras.getString(Notification.EXTRA_TITLE),
)
assertEquals(
"the foreground service type is the regime's, not zero",
ConversionForegroundType.current(),
first.foregroundServiceType,
)
}
/**
* And the title is really the file's name rather than any string at all.
*
* [ConcatWorker.joiningTitle] is asserted above through the constant the worker itself uses, so
* that assertion cannot catch the sentence being reworded — deliberately, since the wording is
* not what the test is about. This one can: two files, two names, one worker each.
*/
@Test
fun `two conversions of differently named files post differently named notifications`() {
runBlocking { conversionWorker(displayName = OTHER_NAME).doWork() }
assertEquals(
OTHER_NAME,
updater.infos.first().notification.extras.getString(Notification.EXTRA_TITLE),
)
}
private fun conversionWorker(displayName: String = DISPLAY_NAME) = TestListenableWorkerBuilder<ConversionWorker>(
context = app,
inputData = workDataOf(
ConversionWorker.KEY_INPUT_URI to INPUT.toString(),
ConversionWorker.KEY_DISPLAY_NAME to displayName,
ConversionWorker.KEY_SIZE_BYTES to INPUT_BYTES,
// FORCE_SOFTWARE is the one preference that decides without consulting the input,
// and a file:// URI keeps the worker out of FFmpegKit's native SAF bridge.
ConversionWorker.KEY_ENGINE_PREFERENCE to EnginePreference.FORCE_SOFTWARE.name,
),
runAttemptCount = 0,
).setId(JOB_ID)
.setForegroundUpdater(updater)
.build()
private fun joinWorker() = TestListenableWorkerBuilder<ConcatWorker>(
context = app,
inputData = workDataOf(
ConcatWorker.KEY_INPUT_URIS to INPUTS.map(Uri::toString).toTypedArray(),
ConcatWorker.KEY_TOTAL_BYTES to TOTAL_BYTES,
ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name,
),
runAttemptCount = 0,
).setId(JOB_ID)
.setForegroundUpdater(updater)
.build()
private companion object {
val INPUT: Uri = Uri.parse("file:///tmp/holiday.mp4")
val INPUTS: List<Uri> = listOf(INPUT, Uri.parse("file:///tmp/holiday2.mp4"))
const val DISPLAY_NAME = "holiday.mp4"
const val OTHER_NAME = "birthday.mkv"
const val INPUT_BYTES = 1_024L
const val TOTAL_BYTES = 2_048L
val JOB_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000252")
}
}
/** A joiner that writes an output and reports a strategy; nothing here is about the engine. */
private object WritingJoiner : ConcatJoiner {
override suspend fun join(inputs: List<Uri>, output: File, format: OutputFormat): ConcatEngine.Result {
output.writeBytes(ByteArray(OUTPUT_BYTES))
return ConcatEngine.Result(ConcatStrategy.STREAM_COPY, output)
}
private const val OUTPUT_BYTES = 512
}
@@ -3,16 +3,12 @@ package org.libremediaconverter.work
import android.app.Application
import android.app.Notification
import android.app.NotificationManager
import android.content.Context
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import androidx.work.Data
import androidx.work.ForegroundInfo
import androidx.work.WorkInfo
import androidx.work.testing.TestForegroundUpdater
import androidx.work.testing.TestListenableWorkerBuilder
import androidx.work.workDataOf
import com.google.common.util.concurrent.ListenableFuture
import kotlinx.coroutines.runBlocking
import org.junit.After
import org.junit.Assert.assertEquals
@@ -224,26 +220,6 @@ class ProgressNotificationTest {
}
}
/**
* Records every [ForegroundInfo] the worker publishes, and otherwise behaves as the test default.
*
* Delegating to [TestForegroundUpdater] rather than hand-rolling a `ListenableFuture<Void>`: the
* worker awaits what this returns, so a future that never completes would hang the initial
* `setForeground` rather than test anything.
*/
private class RecordingForegroundUpdater : TestForegroundUpdater() {
val infos = mutableListOf<ForegroundInfo>()
override fun setForegroundAsync(
context: Context,
id: UUID,
foregroundInfo: ForegroundInfo,
): ListenableFuture<Void> {
infos += foregroundInfo
return super.setForegroundAsync(context, id, foregroundInfo)
}
}
/** An engine that reports whatever [report] wants reported, then writes an output. */
private class ReportingTranscoder(private val report: ((Int) -> Unit) -> Unit) : SoftwareTranscoder {
override suspend fun run(
@@ -1,11 +1,14 @@
package org.libremediaconverter.work
import android.content.Context
import androidx.work.ForegroundInfo
import androidx.work.testing.TestForegroundUpdater
import com.google.common.util.concurrent.ListenableFuture
import org.libremediaconverter.convert.OutputPublisher
import org.libremediaconverter.convert.SoftwareTranscoder
import org.libremediaconverter.model.ConversionRequest
import java.io.File
import java.util.UUID
import java.util.concurrent.ExecutionException
import java.util.concurrent.Executor
import java.util.concurrent.TimeUnit
@@ -94,3 +97,27 @@ internal class FailedFuture(private val failure: Throwable) : ListenableFuture<V
override fun get(): Void = throw ExecutionException(failure)
override fun get(timeout: Long, unit: TimeUnit): Void = throw ExecutionException(failure)
}
/**
* Records every [ForegroundInfo] the worker publishes, and otherwise behaves as the test default.
*
* Delegating to [TestForegroundUpdater] rather than hand-rolling a `ListenableFuture<Void>`: the
* worker awaits what this returns, so a future that never completes would hang the initial
* `setForeground` rather than test anything.
*
* Shared scaffolding since #252 moved it here out of `ProgressNotificationTest`, which asks what a
* *running* worker publishes; `ForegroundNotificationTest` asks what its *first* post is, and both
* questions need the same recorder. `infos.first()` is that first post in either.
*/
internal class RecordingForegroundUpdater : TestForegroundUpdater() {
val infos = mutableListOf<ForegroundInfo>()
override fun setForegroundAsync(
context: Context,
id: UUID,
foregroundInfo: ForegroundInfo,
): ListenableFuture<Void> {
infos += foregroundInfo
return super.setForegroundAsync(context, id, foregroundInfo)
}
}
+22 -1
View File
@@ -403,6 +403,27 @@ They are still correct to keep: `ForegroundInfo` is required by the `CoroutineWo
named — a `getForegroundInfo` that starts branching — plus one more: the day anything calls
`setExpedited`.
**Updated 2026-09-06 (#252, and the sentence above is half wrong).** "WorkManager calls
`getForegroundInfoAsync()` only for expedited work" is true and *not sufficient*, and the missing
half is what made the reopening trigger wrong. `WorkForeground.kt:38` in work-runtime 2.11.2 opens
the library's only caller with
```kotlin
if (!spec.expedited || Build.VERSION.SDK_INT >= 31) return
```
and `minSdk` is 33. So calling `setExpedited` reopens nothing: on **every** device this app
supports, WorkManager does not consult `getForegroundInfo()` whether the work is expedited or not.
#252 was filed on the trigger as this entry stated it, and its acceptance criterion — "a request
now carries `setExpedited` and the existing worker tests drive them" — cannot be met that way.
What made the lines live instead was that each worker held **two** definitions of one notification:
the override, and an identical `ForegroundInfo` built inline in `doWork`. `doWork` now posts the
override's, so the duplicate is gone and what remains runs on every job. The general lesson is the
one E1 states from the other side: *check that the mechanism you are relying on actually fires on
the machine that runs it* — here the mechanism was a library early-return two source lines long,
and four waves of reading had taken the API summary's word for it.
---
## F10 — Three arms that are reachable, uncovered, and cannot be made to bite
@@ -452,7 +473,7 @@ the cheaper order.
| F6 | Four more unreachable arms; `ConversionRouter:214-217`'s KDoc is false | low | confirmed by inspection; each traced to its upstream guard | **no action**, except the one-line KDoc fix |
| F7 | `probeWithExtractor`'s catch is unreachable, as `probeForConcat`'s is | n/a | measured across four URI shapes (recorded in `CLAUDE.md`) | **no action** — device-only, now written down for both sites |
| F8 | Three more dead members and six unused defaults | low | confirmed by inspection; grep per member | delete or keep knowingly — **not** a test gap |
| F9 | Both `getForegroundInfo` overrides are dead: expedited work is never used | n/a | confirmed by inspection; `grep setExpedited` returns nothing | **no action** — sharpens #88's close |
| F9 | Both `getForegroundInfo` overrides are dead: expedited work is never used | n/a | confirmed by inspection; `grep setExpedited` returns nothing | **closed 2026-09-06 by #252** — and its stated reopening trigger was wrong; see the update on the entry |
| F10 | Three reachable arms where no mutation bites | n/a | confirmed by inspection; each mutation traced to its masking guard | **no action** — recorded to stop the next read re-picking them |
Order, if these are acted on: **F1 and F5 first, separately.** They are the two with a possible
+1 -1
View File
@@ -495,7 +495,7 @@ Every one was read. **None of them is an e2e test gap**, which is the result:
| lines | where | classification |
|---|---|---|
| 9 | `Transcoders` ×3, `ConversionViewModel`, `ConverterScreen`, `JoinViewModel`, `JoinScreen`, `MainActivity`, `Reattachment` | **compiler-generated** — default-arg `$default` bridges, coroutine completion, the synthetic `NoWhenBranchMatchedException` arm of a `when` over `Destination` |
| 10 | `ConversionWorker:342-346`, `ConcatWorker:132-136` | `getForegroundInfo()` — WorkManager's **expedited-work** hook, and nothing here enqueues expedited work. The live path is `setForeground(foregroundInfo(...))`, which is covered. **#252** |
| 10 | `ConversionWorker:342-346`, `ConcatWorker:132-136` | `getForegroundInfo()` — WorkManager's **expedited-work** hook, and nothing here enqueues expedited work. The live path is `setForeground(foregroundInfo(...))`, which is covered. **#252 — closed 2026-09-06, and not the way this row expects.** Expedited work is now enqueued, but that is *not* what covers these lines: `WorkForeground.kt:38` returns before the hook whenever `SDK_INT >= 31`, and `minSdk` is 33. What covers them is `doWork` posting the override instead of a second copy of the same notification. See `coverage-read-findings.md` F9's update |
| 3 | `ConversionNotifications:60-62` | **F5** — `areEnabled()` has no callers. Already on record |
| 3 | `CopyPlanner:28`, `OutputFormat:222-223` | public members with no callers. **#253**, with F5 |
| 3 | `MediaProbe:210-212` | `probeWithFFprobe`'s `catch` — **F7's sibling, and now measured**. See below |