4de169c99bf46e18dba71cc2211486b157c0aa9b
2
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  JMR-devandClaude Opus 5
|
6992f0e783 |
Reattach to a conversion that is still running (#230)
Reattachment.rank gives RUNNING the highest rank of all -- "live work outranks a finished result because a running job is holding a foreground service" -- and no test on either source set had ever produced one. ReattachOnLaunchTest covers a job that finished, one whose staged file is gone, an ambiguous pair, one still queued, and one the user cancelled. ReattachmentTest exercises the ranking as a pure function over fabricated snapshots. What was missing is a ViewModel meeting a real running job, which is also the likeliest reattachment there is: the user starts a conversion, leaves, and comes back while it is still going. The engine is a fake, deliberately. The job has to still be running when the ViewModel is built, and every real conversion in this suite finishes in about a second -- racing that is what made the cancellation tests flaky enough to need retries. A SoftwareTranscoder that blocks until released removes the race outright. Nothing about reattachment depends on which engine is transcoding: the tag query, Reattachment.choose over live WorkManager state, and observe's mapping to Converting all run identically whatever is doing the work. This is what #230 can actually deliver, and the ticket asked for the answer either way. Process death itself stays device-manual. D3/D13 already record that am kill refuses a process holding a foreground service, and there is a more basic obstacle underneath it: instrumentation runs in the app's own process, so any route that really killed it would take the test runner with it and leave nothing to assert with. Observing a relaunch needs two instrumentation runs, which the runner does not provide. So the closest observable analogue is a fresh ViewModel, with no memory of the work, meeting a job that is genuinely mid-flight. The teardown now resets ConversionDependencies. The suite runs without Android Test Orchestrator, so a BlockingTranscoder left in place would hang the next class that converts anything. Verified on a local API 34 emulator: 67 tests, 0 failures, 3 skipped; and making RUNNING unreattachable in Reattachment.rank fails this test and nothing else -- which is also the evidence that the JVM ranking test was not already covering it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
ec969c41dc |
Reattach to conversions and joins the ViewModel did not start
The queue surviving process death is the stated reason this app uses WorkManager, and
the ViewModel was where that protection stopped. `activeWorkId` and `observer` are
plain fields, so a process reclaimed after a conversion finished came back to Idle
while the output sat in `cacheDir` with nothing in the UI able to reach it. The
realistic window is not a crash mid-transcode -- it is the job finishing, the user not
saving yet, and the process being reclaimed hours later as an ordinary background one.
Both ViewModels now query their own worker's class name on init and pick up what they
find. Nothing is persisted for it, and nothing needed to be: `WorkRequest.Builder`
seeds every request's tag set with `workerClass.name` (`tags = mutableSetOf(
workerClass.name)`, work-runtime 2.11.2), and R8 keeps those names through
work-runtime's own consumer rule, `-keepnames class * extends
androidx.work.ListenableWorker`. A UUID in a `SavedStateHandle` would have been both
more machinery and less: it cannot find work enqueued by a previous install.
What the query cannot return is the job's input. `WorkInfo` hands back id, state, tags,
progress, output and run-attempt count -- never the `Data` a request was enqueued with
-- so a ViewModel could see that a conversion existed and where its output went, but
not which file it was converting. The display name and size therefore ride on tags too,
which is the whole of the production change to the request builders. The picked `Uri`
deliberately does not: nothing in a reattached state reads it, and a `content://` grant
taken by a picker in a process that no longer exists is not something to hand back as
though it still worked.
The decision is a pure function on the JVM test stack, following `FailureOutcome`:
`Reattachment.choose` takes what WorkManager reported and answers which job, if any.
Cancelled work is excluded -- the user already said no. Failed work is excluded, which
matters more than it looks now that a device has shown an interrupted worker coming
back FAILED rather than retried, its restart's `setForeground` refused as a background
foreground-service start: nothing marks a failure as seen, so it would otherwise
reappear on every launch. A success whose staged file is gone is excluded, because a
Save button that fails on tap is worse than no button. Live work outranks a finished
result, since a running job holds a foreground notification and someone opening the app
while that notification is in the shade is looking for that conversion.
Where several jobs qualify, the newest staged file wins, and that is not a detail.
Losing a tie is not the same as waiting for the next launch: the tag query has no
`ORDER BY`, so its order is unspecified but stable, and an arbitrary winner would keep
winning every launch while the other result stayed unreachable for as long as its file
existed. It is reachable today -- dismiss one result with "Start over", which leaves its
file behind, then convert something else and do not save it. `WorkInfo` carries no
timestamp of any kind, but the edge is already stat'ing the file, so the file's own
mtime is the ordering. A clock that moves backwards makes it a heuristic; an order that
is unspecified and repeats itself is worse.
Aliases are the tie that does not resolve, and that case is not hypothetical. A tag
query on a device returned two SUCCEEDED jobs whose output paths were both
`.../conversions/input_converted.mp4`, with one file on disk -- the staging name is
derived from the input's display name, so a later job overwrites an earlier one's output
and both go on reporting it. Checking the file does not separate them, and neither does
its mtime, since they share it. So the two halves are separated instead. The file is
offered, because it is the user's file either way and losing it is the defect being
fixed; the label is not, because saying which job produced it would be a guess.
`Reattachment.Ambiguous` says so and the card falls back to a neutral name rather than
borrowing the other job's. Aliases whose tags are identical -- the ordinary case, the
same file converted twice -- stay attributed, since nothing turns on which wrote it.
Age is not filtered on, and cannot be: WorkManager keeps finished work about a week and
prunes on its own schedule, so a query in a fresh process routinely returns jobs from
earlier sessions. Whether the staged file is still there is the only signal separating a
result still worth offering from one already dealt with, which is why that check carries
the weight.
Two smaller behaviours fall out of reattaching rather than starting:
- A reattached job that is cancelled lands on Idle rather than Ready. Ready would put
a Convert button over an input URI that belongs to a dead process. `observe()` takes
the cancelled destination as a defaulted parameter, so a job started here is
unchanged.
- A FAILED job's message falls back when blank, not only when absent. A worker killed
before it can report leaves no output data at all, and an exception's message can be
the empty string; both used to reach the screen as a failure with nothing said.
Deliberately not fixed here, each being its own change: the save dialog's suggested name
and MIME still come from the current picker rather than from the job that ran, so a
reattached job in a non-default format is offered the default extension; a result
dismissed with "Start over" still keeps its staged file, so it can be offered again next
launch -- the file check closes that for free once the file is deleted; and
`setForeground` still sits outside `doWork`'s try, so its throw bypasses the retry
decision entirely.
Tested where it can be. 28 JVM tests cover the choice and the tag round trip, including
the ordering, the aliasing rules and a display name that looks like another tag. The
reattachment itself is instrumented: a ViewModel constructed against the real
WorkManager is the next launch, with no memory of the work. `WorkManagerTestInitHelper`
is deliberately not used -- its `setDelegate` replaces the singleton for the whole
process, which would quietly turn `ConversionWorkerTest` into a synchronous test double
depending on class order.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|