Option 3 was taken: both workers' requests are now expedited. But the issue's causal chain is
wrong, and that is the first thing to read here — setExpedited does not cover those ten lines,
and the fix that does is a different one.
The premise, and why it does not hold
#252 says the ten dead lines "get covered by the tests that already exist" once a request carries setExpedited, because getForegroundInfo() is WorkManager's expedited-work hook. That is true and
not sufficient. WorkForeground.kt:38 in work-runtime 2.11.2 opens the library's only caller of getForegroundInfoAsync() with
and minSdk is 33. So on every device this app supports, WorkManager never consults getForegroundInfo() — expedited or not. (grep -rn getForegroundInfoAsync over the sources jar
returns the declarations and overrides, a handful of comments and log strings, and exactly one
call: WorkForeground.kt:42, four lines below that early return.)
Measured rather than only cited. With setExpedited in place and doWork still building its own ForegroundInfo, tools/local-emulator/run-e2e.sh 34 under enableAndroidTestCoverage reports:
method
INSTRUCTION
BRANCH
LINE
METHOD
ConversionWorker.getForegroundInfo
7 missed / 0 covered
—
5 missed / 0 covered
1 missed / 0 covered
ConcatWorker.getForegroundInfo
31 missed / 0 covered
2 missed / 0 covered
5 missed / 0 covered
1 missed / 0 covered
Every one of the ten lines ci=0. And that leg was green, 71/71 — nothing on a device pins those
overrides at all, so a coverage-blind reading of a passing suite would have called this done.
What actually covers them
Each worker held two definitions of one notification: the override, and an identical ForegroundInfo built inline in doWork. doWork now posts the override's. That is the
duplicate-definition the issue's own text circles when it notes ConversionWorker:103 builds the
same thing — and the copy nothing executed is the one that was free to drift. It had: ConcatWorker.getForegroundInfo said "Joining files" while doWork said "Joining N files", for
one notification with one job. There is now one string, ConcatWorker.joiningTitle(count), and the
override reads the input array itself so it can still answer before doWork has parsed anything.
Same measurement, same emulator, final code:
method
INSTRUCTION
BRANCH
LINE
METHOD
ConversionWorker.getForegroundInfo
0 missed / 7 covered
—
0 missed / 5 covered
0 missed / 1 covered
ConcatWorker.getForegroundInfo
2 missed / 29 covered
1 missed / 1 covered
0 missed / 5 covered
0 missed / 1 covered
Before, on main: all ten among the 32 lines docs/e2e-read-findings.md's E8 records as reached by
neither suite. After: 0 of 10 missed. The one remaining ConcatWorker branch is the ?: 0 arm
of the input-count read, which doWork refuses several lines earlier and nothing else calls — named
in its KDoc as F4-shaped rather than covered.
Measurement scaffolding (enableAndroidTestCoverage, a temporary JacocoReport copying jacocoTestReport's classDirectories/sourceDirectories/excludes) was reverted; git diff app/build.gradle.kts is empty.
The expedited / delay decision
RUN_AS_NON_EXPEDITED_WORK_REQUEST, not DROP_WORK_REQUEST. An invisible quota is no reason
to throw away a conversion the user asked for; SystemJobScheduler:198 degrades the request to an
ordinary job instead.
No initial delay is at risk. All four setInitialDelay sites are tests, and every one builds
its own OneTimeWorkRequestBuilder rather than adding a delay to what request() returns — NotificationCancelActionTest:95 starts from request() but takes only base.workSpec.input from
it. So WorkRequest.build()'s require(workSpec.initialDelay <= 0) is a future hazard, and ExpeditedRequestTest asserts the delay and constraints that keep it one: add a delay to either
builder and that class fails on the throw rather than the app failing to enqueue on a device.
The old KDoc's objection was about a quota that does not reach this shape. It belongs to the
JobScheduler job, SystemJobInfoConverter:135 sets JobInfo.setExpedited(true) only when !isRetry && !isDelayed, and a job the system stops mid-run never gets its answer from FailureOutcome: WorkerWrapper cancels the coroutine with WorkerStoppedException, resolves it
as ResetWorkerStatus, discards whatever Result the worker returned and re-enqueues with
backoff.
What it buys is narrow, and the KDoc says so.GreedyScheduler starts unconstrained, undelayed
work in-process the instant it is enqueued and has no expedited branch at all, so a conversion
begun from the open app runs exactly when it ran before. The flag is for the job still enqueued
when the app died, which has to go back through JobScheduler.
Foreground service type is untouched and needs to be. On 31+ WorkManager runs no foreground
service for expedited work; the workers' own setForeground path, ConversionForegroundType and
the manifest are exactly as they were.
The mutations, all five red
mutation
test that reddened
drop setExpedited from ConversionWorker.request
ExpeditedRequestTest — a conversion is enqueued as expedited work
drop setExpedited from ConcatWorker.request
ExpeditedRequestTest — a join is enqueued as expedited work
ConcatWorker.getForegroundInfo stops counting its inputs (inputCount = 0)
ForegroundNotificationTest — a join's first foreground post counts the files it was given
ConversionWorker.getForegroundInfo starts at percent = 50
ForegroundNotificationTest — a conversion's first foreground post is the one getForegroundInfo builds
ConversionWorker.getForegroundInfo hard-codes the title
ForegroundNotificationTest — two conversions of differently named files post differently named notifications
The last three sit inside the override's own body, so a red there is what proves doWork
executes it rather than a copy of it — independently of any coverage report.
One assertion that bites less than it reads, written down in the test KDoc and repeated here so
nobody counts it twice: WorkSpec.outOfQuotaPolicydefaults to RUN_AS_NON_EXPEDITED_WORK_REQUEST, so asserting it is already true of a request that was never
expedited. It pins the one alternative — DROP_WORK_REQUEST — and nothing else.
Tests
ExpeditedRequestTest (new, 3 tests) — both requests expedited, the policy, and the delay and
constraints that keep build() from throwing.
ForegroundNotificationTest (new, 3 tests) — the first ForegroundInfo each worker posts,
asserted against constants rather than against worker.getForegroundInfo() (which would move with
every mutation and stay green).
RecordingForegroundUpdater moved from ProgressNotificationTest into WorkerStubs.kt, which is
what that file is for; two tests now need it.
Docs
docs/coverage-read-findings.md F9 gets an update: its reopening trigger ("the day anything calls setExpedited") was wrong for the reason above, and its table row is now closed. Row 10 of docs/e2e-read-findings.md's 32-line table says the same. CLAUDE.md's coverage figures are not
touched — the re-measure rule applies and nothing here moves the testDebugUnitTest number in a way
this PR measured.
The gate
The local gate ran the full API 33-36 sweep four times on this exact tree — once for the commit
and three times for the push, since the two SSH push attempts each passed the gate and then died in
the transport — and was green every time: 71/71, 3 skipped, on each level. API 37 is not covered
locally: no device was attached, so CI's gating leg answers for it. No instrumented test was added
or removed, so the suite count stays 71 and FAILS_ON_EMULATOR_API37_BASELINE is untouched.
Two things found while landing this, filed rather than fixed here:
#258 — tools/git-hooks/pre-commit files its sweep cache under .git/lmc-verify, and in a
worktree .git is a file, so mkdir fails, nothing is recorded, and every push re-sweeps all
four levels for a byte-identical app/src. It fails silently: the hook still prints "green on API
33, 34, 35, 36" and exits 0.
git push over SSH cannot survive this hook. Git opens the remote connection for ref
discovery before running pre-push — it needs the remote sha to feed the hook — so after a
35-minute sweep the idle SSH session is gone and the pack write takes SIGPIPE: exit 141,
hook green, nothing on the remote, no error message. Reproduced twice. This branch went up over
HTTPS, where ref discovery and the pack upload are two separate requests. Worth knowing before the
next long-hook push, and arguably worth a line in CLAUDE.md beside the gate — not written here
because it is not this PR's subject.
Closes #252.
Option 3 was taken: both workers' requests are now expedited. But **the issue's causal chain is
wrong, and that is the first thing to read here** — `setExpedited` does not cover those ten lines,
and the fix that does is a different one.
## The premise, and why it does not hold
#252 says the ten dead lines "get covered by the tests that already exist" once a request carries
`setExpedited`, because `getForegroundInfo()` is WorkManager's expedited-work hook. That is true and
not sufficient. `WorkForeground.kt:38` in work-runtime 2.11.2 opens the library's **only** caller of
`getForegroundInfoAsync()` with
```kotlin
if (!spec.expedited || Build.VERSION.SDK_INT >= 31) return
```
and `minSdk` is 33. So on every device this app supports, WorkManager never consults
`getForegroundInfo()` — expedited or not. (`grep -rn getForegroundInfoAsync` over the sources jar
returns the declarations and overrides, a handful of comments and log strings, and exactly **one**
call: `WorkForeground.kt:42`, four lines below that early return.)
**Measured rather than only cited.** With `setExpedited` in place and `doWork` still building its own
`ForegroundInfo`, `tools/local-emulator/run-e2e.sh 34` under `enableAndroidTestCoverage` reports:
| method | INSTRUCTION | BRANCH | LINE | METHOD |
|---|---|---|---|---|
| `ConversionWorker.getForegroundInfo` | 7 missed / 0 covered | — | **5 missed / 0 covered** | 1 missed / 0 covered |
| `ConcatWorker.getForegroundInfo` | 31 missed / 0 covered | 2 missed / 0 covered | **5 missed / 0 covered** | 1 missed / 0 covered |
Every one of the ten lines `ci=0`. And that leg was **green, 71/71** — nothing on a device pins those
overrides at all, so a coverage-blind reading of a passing suite would have called this done.
## What actually covers them
Each worker held **two** definitions of one notification: the override, and an identical
`ForegroundInfo` built inline in `doWork`. `doWork` now posts the override's. That is the
duplicate-definition the issue's own text circles when it notes `ConversionWorker:103` builds the
same thing — and the copy nothing executed is the one that was free to drift. It had:
`ConcatWorker.getForegroundInfo` said `"Joining files"` while `doWork` said `"Joining N files"`, for
one notification with one job. There is now one string, `ConcatWorker.joiningTitle(count)`, and the
override reads the input array itself so it can still answer before `doWork` has parsed anything.
Same measurement, same emulator, final code:
| method | INSTRUCTION | BRANCH | LINE | METHOD |
|---|---|---|---|---|
| `ConversionWorker.getForegroundInfo` | 0 missed / 7 covered | — | **0 missed / 5 covered** | 0 missed / 1 covered |
| `ConcatWorker.getForegroundInfo` | 2 missed / 29 covered | 1 missed / 1 covered | **0 missed / 5 covered** | 0 missed / 1 covered |
Before, on `main`: all ten among the 32 lines `docs/e2e-read-findings.md`'s E8 records as reached by
neither suite. After: **0 of 10 missed.** The one remaining `ConcatWorker` branch is the `?: 0` arm
of the input-count read, which `doWork` refuses several lines earlier and nothing else calls — named
in its KDoc as F4-shaped rather than covered.
Measurement scaffolding (`enableAndroidTestCoverage`, a temporary `JacocoReport` copying
`jacocoTestReport`'s `classDirectories`/`sourceDirectories`/excludes) was reverted;
`git diff app/build.gradle.kts` is empty.
## The expedited / delay decision
- **`RUN_AS_NON_EXPEDITED_WORK_REQUEST`, not `DROP_WORK_REQUEST`.** An invisible quota is no reason
to throw away a conversion the user asked for; `SystemJobScheduler:198` degrades the request to an
ordinary job instead.
- **No initial delay is at risk.** All four `setInitialDelay` sites are tests, and every one builds
its own `OneTimeWorkRequestBuilder` rather than adding a delay to what `request()` returns —
`NotificationCancelActionTest:95` starts from `request()` but takes only `base.workSpec.input` from
it. So `WorkRequest.build()`'s `require(workSpec.initialDelay <= 0)` is a *future* hazard, and
`ExpeditedRequestTest` asserts the delay and constraints that keep it one: add a delay to either
builder and that class fails on the throw rather than the app failing to enqueue on a device.
- **The old KDoc's objection was about a quota that does not reach this shape.** It belongs to the
JobScheduler job, `SystemJobInfoConverter:135` sets `JobInfo.setExpedited(true)` only when
`!isRetry && !isDelayed`, and a job the system stops mid-run never gets its answer from
`FailureOutcome`: `WorkerWrapper` cancels the coroutine with `WorkerStoppedException`, resolves it
as `ResetWorkerStatus`, discards whatever `Result` the worker returned and re-enqueues with
backoff.
- **What it buys is narrow, and the KDoc says so.** `GreedyScheduler` starts unconstrained, undelayed
work in-process the instant it is enqueued and has no `expedited` branch at all, so a conversion
begun from the open app runs exactly when it ran before. The flag is for the job still enqueued
when the app died, which has to go back through JobScheduler.
- **Foreground service type is untouched and needs to be.** On 31+ WorkManager runs no foreground
service for expedited work; the workers' own `setForeground` path, `ConversionForegroundType` and
the manifest are exactly as they were.
## The mutations, all five red
| mutation | test that reddened |
|---|---|
| drop `setExpedited` from `ConversionWorker.request` | `ExpeditedRequestTest` — a conversion is enqueued as expedited work |
| drop `setExpedited` from `ConcatWorker.request` | `ExpeditedRequestTest` — a join is enqueued as expedited work |
| `ConcatWorker.getForegroundInfo` stops counting its inputs (`inputCount = 0`) | `ForegroundNotificationTest` — a join's first foreground post counts the files it was given |
| `ConversionWorker.getForegroundInfo` starts at `percent = 50` | `ForegroundNotificationTest` — a conversion's first foreground post is the one getForegroundInfo builds |
| `ConversionWorker.getForegroundInfo` hard-codes the title | `ForegroundNotificationTest` — two conversions of differently named files post differently named notifications |
The last three sit **inside the override's own body**, so a red there is what proves `doWork`
executes it rather than a copy of it — independently of any coverage report.
**One assertion that bites less than it reads**, written down in the test KDoc and repeated here so
nobody counts it twice: `WorkSpec.outOfQuotaPolicy` *defaults* to
`RUN_AS_NON_EXPEDITED_WORK_REQUEST`, so asserting it is already true of a request that was never
expedited. It pins the one alternative — `DROP_WORK_REQUEST` — and nothing else.
## Tests
- `ExpeditedRequestTest` (new, 3 tests) — both requests expedited, the policy, and the delay and
constraints that keep `build()` from throwing.
- `ForegroundNotificationTest` (new, 3 tests) — the first `ForegroundInfo` each worker posts,
asserted against constants rather than against `worker.getForegroundInfo()` (which would move with
every mutation and stay green).
- `RecordingForegroundUpdater` moved from `ProgressNotificationTest` into `WorkerStubs.kt`, which is
what that file is for; two tests now need it.
## Docs
`docs/coverage-read-findings.md` F9 gets an update: its reopening trigger ("the day anything calls
`setExpedited`") was wrong for the reason above, and its table row is now closed. Row 10 of
`docs/e2e-read-findings.md`'s 32-line table says the same. `CLAUDE.md`'s coverage figures are not
touched — the re-measure rule applies and nothing here moves the `testDebugUnitTest` number in a way
this PR measured.
## The gate
The local gate ran the full API 33-36 sweep **four** times on this exact tree — once for the commit
and three times for the push, since the two SSH push attempts each passed the gate and then died in
the transport — and was green every time: 71/71, 3 skipped, on each level. API 37 is not covered
locally: no device was attached, so CI's gating leg answers for it. No instrumented test was added
or removed, so the suite count stays 71 and `FAILS_ON_EMULATOR_API37_BASELINE` is untouched.
Two things found while landing this, filed rather than fixed here:
- **#258** — `tools/git-hooks/pre-commit` files its sweep cache under `.git/lmc-verify`, and in a
worktree `.git` is a *file*, so `mkdir` fails, nothing is recorded, and every push re-sweeps all
four levels for a byte-identical `app/src`. It fails silently: the hook still prints "green on API
33, 34, 35, 36" and exits 0.
- **`git push` over SSH cannot survive this hook.** Git opens the remote connection for ref
discovery *before* running `pre-push` — it needs the remote sha to feed the hook — so after a
35-minute sweep the idle SSH session is gone and the pack write takes `SIGPIPE`: exit **141**,
hook green, nothing on the remote, no error message. Reproduced twice. This branch went up over
HTTPS, where ref discovery and the pack upload are two separate requests. Worth knowing before the
next long-hook push, and arguably worth a line in `CLAUDE.md` beside the gate — not written here
because it is not this PR's subject.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #252.
Option 3 was taken: both workers' requests are now expedited. But the issue's causal chain is
wrong, and that is the first thing to read here —
setExpediteddoes not cover those ten lines,and the fix that does is a different one.
The premise, and why it does not hold
#252 says the ten dead lines "get covered by the tests that already exist" once a request carries
setExpedited, becausegetForegroundInfo()is WorkManager's expedited-work hook. That is true andnot sufficient.
WorkForeground.kt:38in work-runtime 2.11.2 opens the library's only caller ofgetForegroundInfoAsync()withand
minSdkis 33. So on every device this app supports, WorkManager never consultsgetForegroundInfo()— expedited or not. (grep -rn getForegroundInfoAsyncover the sources jarreturns the declarations and overrides, a handful of comments and log strings, and exactly one
call:
WorkForeground.kt:42, four lines below that early return.)Measured rather than only cited. With
setExpeditedin place anddoWorkstill building its ownForegroundInfo,tools/local-emulator/run-e2e.sh 34underenableAndroidTestCoveragereports:ConversionWorker.getForegroundInfoConcatWorker.getForegroundInfoEvery one of the ten lines
ci=0. And that leg was green, 71/71 — nothing on a device pins thoseoverrides at all, so a coverage-blind reading of a passing suite would have called this done.
What actually covers them
Each worker held two definitions of one notification: the override, and an identical
ForegroundInfobuilt inline indoWork.doWorknow posts the override's. That is theduplicate-definition the issue's own text circles when it notes
ConversionWorker:103builds thesame thing — and the copy nothing executed is the one that was free to drift. It had:
ConcatWorker.getForegroundInfosaid"Joining files"whiledoWorksaid"Joining N files", forone notification with one job. There is now one string,
ConcatWorker.joiningTitle(count), and theoverride reads the input array itself so it can still answer before
doWorkhas parsed anything.Same measurement, same emulator, final code:
ConversionWorker.getForegroundInfoConcatWorker.getForegroundInfoBefore, on
main: all ten among the 32 linesdocs/e2e-read-findings.md's E8 records as reached byneither suite. After: 0 of 10 missed. The one remaining
ConcatWorkerbranch is the?: 0armof the input-count read, which
doWorkrefuses several lines earlier and nothing else calls — namedin its KDoc as F4-shaped rather than covered.
Measurement scaffolding (
enableAndroidTestCoverage, a temporaryJacocoReportcopyingjacocoTestReport'sclassDirectories/sourceDirectories/excludes) was reverted;git diff app/build.gradle.ktsis empty.The expedited / delay decision
RUN_AS_NON_EXPEDITED_WORK_REQUEST, notDROP_WORK_REQUEST. An invisible quota is no reasonto throw away a conversion the user asked for;
SystemJobScheduler:198degrades the request to anordinary job instead.
setInitialDelaysites are tests, and every one buildsits own
OneTimeWorkRequestBuilderrather than adding a delay to whatrequest()returns —NotificationCancelActionTest:95starts fromrequest()but takes onlybase.workSpec.inputfromit. So
WorkRequest.build()'srequire(workSpec.initialDelay <= 0)is a future hazard, andExpeditedRequestTestasserts the delay and constraints that keep it one: add a delay to eitherbuilder and that class fails on the throw rather than the app failing to enqueue on a device.
JobScheduler job,
SystemJobInfoConverter:135setsJobInfo.setExpedited(true)only when!isRetry && !isDelayed, and a job the system stops mid-run never gets its answer fromFailureOutcome:WorkerWrappercancels the coroutine withWorkerStoppedException, resolves itas
ResetWorkerStatus, discards whateverResultthe worker returned and re-enqueues withbackoff.
GreedySchedulerstarts unconstrained, undelayedwork in-process the instant it is enqueued and has no
expeditedbranch at all, so a conversionbegun from the open app runs exactly when it ran before. The flag is for the job still enqueued
when the app died, which has to go back through JobScheduler.
service for expedited work; the workers' own
setForegroundpath,ConversionForegroundTypeandthe manifest are exactly as they were.
The mutations, all five red
setExpeditedfromConversionWorker.requestExpeditedRequestTest— a conversion is enqueued as expedited worksetExpeditedfromConcatWorker.requestExpeditedRequestTest— a join is enqueued as expedited workConcatWorker.getForegroundInfostops counting its inputs (inputCount = 0)ForegroundNotificationTest— a join's first foreground post counts the files it was givenConversionWorker.getForegroundInfostarts atpercent = 50ForegroundNotificationTest— a conversion's first foreground post is the one getForegroundInfo buildsConversionWorker.getForegroundInfohard-codes the titleForegroundNotificationTest— two conversions of differently named files post differently named notificationsThe last three sit inside the override's own body, so a red there is what proves
doWorkexecutes it rather than a copy of it — independently of any coverage report.
One assertion that bites less than it reads, written down in the test KDoc and repeated here so
nobody counts it twice:
WorkSpec.outOfQuotaPolicydefaults toRUN_AS_NON_EXPEDITED_WORK_REQUEST, so asserting it is already true of a request that was neverexpedited. It pins the one alternative —
DROP_WORK_REQUEST— and nothing else.Tests
ExpeditedRequestTest(new, 3 tests) — both requests expedited, the policy, and the delay andconstraints that keep
build()from throwing.ForegroundNotificationTest(new, 3 tests) — the firstForegroundInfoeach worker posts,asserted against constants rather than against
worker.getForegroundInfo()(which would move withevery mutation and stay green).
RecordingForegroundUpdatermoved fromProgressNotificationTestintoWorkerStubs.kt, which iswhat that file is for; two tests now need it.
Docs
docs/coverage-read-findings.mdF9 gets an update: its reopening trigger ("the day anything callssetExpedited") was wrong for the reason above, and its table row is now closed. Row 10 ofdocs/e2e-read-findings.md's 32-line table says the same.CLAUDE.md's coverage figures are nottouched — the re-measure rule applies and nothing here moves the
testDebugUnitTestnumber in a waythis PR measured.
The gate
The local gate ran the full API 33-36 sweep four times on this exact tree — once for the commit
and three times for the push, since the two SSH push attempts each passed the gate and then died in
the transport — and was green every time: 71/71, 3 skipped, on each level. API 37 is not covered
locally: no device was attached, so CI's gating leg answers for it. No instrumented test was added
or removed, so the suite count stays 71 and
FAILS_ON_EMULATOR_API37_BASELINEis untouched.Two things found while landing this, filed rather than fixed here:
tools/git-hooks/pre-commitfiles its sweep cache under.git/lmc-verify, and in aworktree
.gitis a file, somkdirfails, nothing is recorded, and every push re-sweeps allfour levels for a byte-identical
app/src. It fails silently: the hook still prints "green on API33, 34, 35, 36" and exits 0.
git pushover SSH cannot survive this hook. Git opens the remote connection for refdiscovery before running
pre-push— it needs the remote sha to feed the hook — so after a35-minute sweep the idle SSH session is gone and the pack write takes
SIGPIPE: exit 141,hook green, nothing on the remote, no error message. Reproduced twice. This branch went up over
HTTPS, where ref discovery and the pack upload are two separate requests. Worth knowing before the
next long-hook push, and arguably worth a line in
CLAUDE.mdbeside the gate — not written herebecause it is not this PR's subject.
🤖 Generated with Claude Code