docs/e7-second-constraint
185
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  JMR-devandClaude Opus 5
|
b23ff0f082 |
Wait for the app to come back before asking Compose about it
The save test failed an API 35 leg with "No compose hierarchies found in the app". Dismissing the POST_NOTIFICATIONS dialog presses back and waits for the permission UI to be gone, but going away and the app being in front again are not the same moment, and the next Compose query landed in the gap. Asked of UiAutomator rather than through awaitAppFocus, which is the opposite of what this class argues for elsewhere and is right here: awaitAppFocus goes through composeRule.waitUntil, so it would raise the very error it is being used to avoid. Two more local API 34 runs at 70/0/0/3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
fa10d94192 |
Save to a document stock DocumentsUI created (#226)
publish deletes a destination it could not write to -- D4's fix, so a failed save does not leave a truncated file at the name the user chose -- but only when that destination was positively zero bytes first. destinationIsKnownEmpty is careful that "I could not tell" never authorises a delete, which makes the precondition load-bearing. Until now that precondition was asserted only against a fake built to match it: OutputPublisherPublishTest writes ByteArray(0) into FakeSafProvider before each case, under a comment stating this is how CreateDocument behaves. If it were false in production, D4's fix would be inert and every existing test would still pass. It is not false. Measured on an API 34 emulator against the real dialog: the document SAF hands back is a document URI and reports a size of exactly zero before anything writes to it. RecordingPublisher reads both at the moment publish sees them, through the ConversionDependencies seam, then lets the real copy proceed so the bytes are checked too. This has to go through the picker, and through the app, and both are platform constraints rather than choices. E7 in docs/e2e-read-findings.md records the first: a DocumentsProvider is reachable only through a picker-issued grant. The second was measured here -- a host Activity in this source set owning its own CreateDocument launcher cannot be started at all, because instrumentation runs in the target app's process and ActivityScenario refuses with "Intent in process org.libremediaconverter resolved to different process org.libremediaconverter.test". So #226 has no cheap half, which is what its comment now says. Three things the flow needed, each measured rather than guessed: Both taps scroll first. On Ready the screen carries a file card, five pickers and then the button, so Convert is below the fold; performClick on an off-screen node dispatches where nothing is and throws nothing, while assertIsEnabled passes either way. The first version sat waiting for a Converted that could never come. The format stays at its default. FixtureDocumentsProvider advertises video/mp4 so the picker's MIME filter has a mutation with a shape, and DocumentsUI honours that on the save side too: choosing MP3 makes the destination audio/mpeg and the fixture root is filtered out of the save dialog entirely. The notification dialog is dismissed rather than pre-granted. Convert converts from the permission callback whichever way the answer goes, so denying is a real user's path and enough. Granting programmatically did not take -- GrantPermissionsActivity appeared anyway and swallowed the tap. The provider gains create, write and delete support, which it needs to be a save target at all. It carries @FailsOnEmulatorApi37 because anything that puts DocumentsUI on screen aborts system_server on that image, as #245 established for the other two; baseline 5 -> 6. Verified on a local API 34 emulator: three full-suite runs at 70/0/0/3, and a publish that writes no bytes fails it with "array lengths differed, expected.length=58677 actual.length=0". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
07f7ed4259 |
Merge #221, and make the two counts derived rather than remembered
#221 landed while this was in review, adding one instrumented test: 68 -> 69, and 64 on the gating leg. Its own commit was "Move CLAUDE.md's instrumented counts with the test that changes them", and it still arrived stale -- it says three markers and 61 tests, both true of the main it was branched from and neither true of the main it merged into. That is the third time these two numbers have gone stale in a day, so the paragraph now says where they come from: a grep for @Test over app/src/androidTest minus the marker count, cross-checkable against any run's shape, since a leg below 37 reports the first as `expected` and the API 37 gating leg reports the second. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
7fd95ddede |
Merge main, and re-derive every count it moved
main landed 25 commits while this branch was open, including a third @FailsOnEmulatorApi37 on Media3EngineTest.cancellingARunningExportStopsIt and a batch of new instrumented tests. Every number this branch touches moved with them. Re-derived rather than adjusted, and cross-checked against run 34020234606: the API 34 leg (no filter) reports 68 tests and the API 37 gating leg 64, which is 68 minus main's four markers. With the picker test marked that is five markers, baseline 5, and 63 on the gating leg. The conflict in FailsOnEmulatorApi37.kt is resolved main's way: it had replaced the hardcoded "grows by two" with a reference to the constant, which is the same drift this file exists to prevent and a better fix than the number I put there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Jason Ross
|
350b179c9e | Merge branch 'main' into test/join-failure-message-on-device | ||
|
JMR-devandClaude Opus 5
|
31f249ae04 |
Cancel a running Media3 export, completing #224's third engine
The two FFmpeg engines were done in |
||
|
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
|
802997439d |
Let a join read the files the user actually picked (#238, #225)
Joining files picked through the system picker failed outright whenever the strategy was stream copy -- the matched-files case the UI advertises as "joined without re-encoding, no quality loss". [ffkitsaf @ ...] Protocol 'ffkitsaf' not on whitelist 'file,crypto,data'! Error opening input file .../joined_from_content.concat_list.txt JoinScreen picks with OpenMultipleDocuments, so real inputs are always content://. ConcatEngine maps each through getSafParameterForRead and FFmpegConcatCommand writes the resulting ffkitsaf: paths into the concat list file. The demuxer applies its own protocol whitelist, defaulting to file,crypto,data, and -safe 0 does not touch it: that permits absolute paths, this permits the scheme they carry. Two separate gates, and only one was open. Nothing caught it because the two halves of the bug never met. Only STREAM_COPY feeds the demuxer a list file -- REENCODE passes each input with its own -i, where the whitelist does not apply -- so joining over SAF worked for mismatched clips. And every join test passed Uri.fromFile, which takes ConcatEngine's uri.path arm instead of the bridge, so matchingClipsAreJoinedByStreamCopy exercised stream copy with a file: path and passed. The one broken combination was the one no test produced and the only one a user can reach. That is #225's gap: FFmpegKitConfig.getSafParameterForRead is on every real conversion and join, and was on no passing test -- only on UnopenableUriTest's failure side, which proves the error message rather than the bridge. ContentUriInputTest now drives both the convert and join paths from a real content:// URI. It uses a plain ContentProvider, because the documents provider cannot be reached. Measured three ways: a DOCUMENTS_PROVIDER without MANAGE_DOCUMENTS is refused at install, instrumentation runs in the target app's process so Instrumentation.getContext() still carries the app's uid and is denied, and adoptShellPermissionIdentity(MANAGE_DOCUMENTS) is denied identically -- the denial naming ACTION_OPEN_DOCUMENT as the only way in. The bridge needs no documents provider: it opens a descriptor through the resolver, so any readable content:// URI exercises it, and an ordinary provider may be exported unprotected. The whole class stays headless. Recorded on #226, which that also settles: its cheap half does not exist. Verified on a local API 34 emulator: 66 tests, 0 failures, 3 skipped; and removing the -protocol_whitelist pair reproduces the production failure verbatim in the join test and nothing else. FFmpegConcatCommandTest pins the flag on the JVM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
bc8e67888e |
Wait for the pick the launcher test is about (#220)
onInputPicked does not reach Ready on the calling thread. It hops twice --
withContext(pickDispatcher) { InputQuery.describe(...) } and then the probe
-- and pickDispatcher defaults to Dispatchers.IO, a real background thread
Compose's idling knows nothing about. So deliver() returned with the state
still Idle, and asserting immediately was a race the test usually won.
It lost five times on CI in one day, on PRs whose diffs were instrumented
tests and documentation and could not reach it. Two of those failures came
alongside #125's deadlock and could be argued as fallout; three did not.
waitUntil polls through waitForIdle, draining the main looper each time, so
it sees the recomposition the IO hop eventually posts back.
Injecting the dispatcher would be better and is not available here.
pickDispatcher is a constructor parameter precisely so a test can pin it,
but this test composes the real ConverterScreen, which resolves its own
ViewModel through viewModel() -- the seam is one layer below the launcher
edge this class exists to cover, and reaching for it would mean not testing
that edge.
The evidence is the mutation rather than the repetition count, per the note
#218 left: transposing the two launcher callbacks at ConverterScreen.kt:70
and :83 -- the exact defect this test guards -- still fails it, so the wait
did not make it vacuous. A transposed callback leaves the screen in Idle
forever and it fails on the timeout with the meaning it had before.
Supporting evidence, eight consecutive green runs of the class.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
JMR-devandClaude Opus 5
|
163ce54b77 |
Stop the cancel tests losing their race on a loaded runner
Both cancellation tests I added in |
||
|
JMR-devandClaude Opus 5
|
5416788274 |
Cancel a running join session too (#224)
The FFmpegEngine half landed in ad2a75d; this is the same gap in ConcatEngine. Between them, a real native session being asked to stop is now covered on both FFmpeg paths. The assertion is the session's return code again, and here that is not merely the better choice but close to the only one: ConcatEngine does not delete its output on cancellation at all. Its invokeOnCancellation is FFmpegKit.cancel and nothing else, where FFmpegEngine's also deletes the partial. Whether that asymmetry is deliberate is a separate question, so this asserts what is true of both engines rather than depending on it. The cancel triggers on SessionState.RUNNING rather than on progress. ConcatWorker publishes no progress at all, so there is no callback to hang it on even in principle -- and the conversion side already measured the deeper reason, that the committed clips outrun a callback-triggered cancel. The inputs are the mismatched pair on purpose, so ConcatPlanner chooses REENCODE. A stream copy of two 2 s clips is close to instantaneous and would leave nothing to interrupt; re-encoding is also the case where a user would actually reach for Cancel. Verified on a local API 34 emulator: three runs green at 64/0/0/3, and replacing invokeOnCancellation's body with an empty block fails this test and nothing else, with state=COMPLETED rc=0. Media3Engine's transformer.cancel() is still uncovered and #224 stays open for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Jason Ross
|
ad2a75d9a0 |
Merge pull request #236 from JMR-dev/test/cancelling-a-running-session
Cancel a running FFmpeg session, which nothing had ever done |
||
|
Jason Ross
|
2b921fafe4 |
Merge pull request #235 from JMR-dev/test/notification-cancel-action
Press the Cancel button in the notification |
||
|
JMR-devandClaude Opus 5
|
98c0e4dba2 |
Cancel a running FFmpeg session, which nothing had ever done (#224)
Every cancel in app/src/androidTest is WorkManager.cancelWorkById against work that is queued or already finished: ReattachOnLaunchTest cancels a job carrying a one-hour initial delay, and another immediately after enqueue. On the JVM, WorkerCancellationTest and HardwareFallbackTest's cancellation case drive a SoftwareTranscoder double that records the call. No test on any source set had asked a real native session to stop. That is docs/defect-audit.md D10's forcing condition. It is the one path where cancelling wrong is silently expensive rather than loudly broken: a missed FFmpegKit.cancel leaves the native process encoding to completion while the UI says the job is cancelled. Two things were measured rather than assumed, and both changed the test. The output file cannot be the assertion. invokeOnCancellation deletes the path, and on POSIX unlinking a file ffmpeg still holds open leaves ffmpeg writing to the unlinked inode -- so the path stays gone whether or not the cancel reached the session, and removing FFmpegKit.cancel passes that check every time. The session's own verdict is what separates them: a cancelled session ends with the cancel return code, a completed one does not. Cancelling from the first progress callback loses the race. It was tried first and failed with state=COMPLETED rc=0: every committed fixture is 2-3 s at 320x240, and the encode finishes before the first statistics callback is delivered and acted on. FFmpegKit.listSessions shows the session RUNNING far earlier, so that is what the test waits for. QualityTier.BEST is deliberate for the same reason -- preset medium leaves more of the encode ahead of the cancel. Verified on a local API 34 emulator. Four consecutive runs green at 62/0/0/3, and removing FFmpegKit.cancel while keeping output.delete fails this test and nothing else, with state=COMPLETED rc=1. ConcatEngine and Media3Engine carry the same shape and are not covered here; #224 stays open for them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
557b3edab4 |
Compare the advisory failure count only on a run that finished
The verification dispatch of the reworked harness (34011072884) came back 4 expected / 3 received / 3 failed, where the one before it (34008889182) had been 4/4/4 on the identical configuration. Nothing about the test list changed between them: the abort landed one test earlier and the picker test never started. The baseline check would have called that "one now passes", which is the wrong reading and the kind of notice #120 is about -- a deviation that is wrong often enough to teach everyone to skim past deviation notices. So `failed` is compared only when `completed cleanly` is yes, and `expected` is compared always, because `Starting N tests` is printed before anything can abort and is what actually answers "is the marked set the size the baseline says". Two cases in e2e-report-shape-test.sh, as a pair: a truncated run short by one is not a deviation, and a CLEAN run short by one still is -- so the first cannot have bought its quiet by disabling the check. This is a consequence of adding the fourth marker rather than a pre-existing bug worth its own ticket: with three, the advisory leg had been completing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
e0412329ff |
Press the Cancel button in the notification (#227)
ConversionNotifications.build attaches one action, wired to WorkManager.createCancelPendingIntent(id). Until now createCancelPendingIntent had no references anywhere outside its own declaration -- no JVM test, no instrumented test. That is worth more than an ordinary uncovered line. A conversion runs in a foreground service and the user is invited to leave the app; once they do, this action is the only way to stop it. If the PendingIntent carries the wrong id the button does nothing, the notification stays, and the job runs to completion, with no error, no log and no screen to look at. The obvious version of this test reads NotificationManager's active notifications for id 1001 and taps what it finds. Rejected: the instrumented suite grants no runtime permissions, so POST_NOTIFICATIONS is denied throughout, and whether a suppressed foreground-service notification is returned there is a platform detail that varies. The test would be asserting something about notification visibility rather than about cancellation. The PendingIntent is the subject and where it is read from is incidental, so this builds the notification for a real live work id and fires its action -- a real dispatch reaching real WorkManager, the same way on every API level. The job carries an initial delay so it stays ENQUEUED. A conversion of the 3 s fixture finishes in well under a second on an emulator, so racing a cancel against a running job would be flaky in the direction that fails, and cancelWorkById acts on ENQUEUED identically. What is under test is whether firing the action reaches WorkManager with the right id. Verified both ways on a local API 34 emulator: 61 tests, 0 failures, 3 skipped as written; and building the PendingIntent from a random UUID instead of the request's id fails the new test and nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
948d53b67e |
Read the progress percentage FFmpeg has always been computing (#229)
FFmpegEngine derives progress as stats.time / durationMs * 100, and the statistics callback runs on every conversion in FFmpegEngineTest -- they all pass durationMs = 3_000. But every call site omits onProgress, so nothing on any source set had ever looked at the number. Replacing percent with a constant reddened nothing. What already existed covers the plumbing downstream and not this: #196 covered the worker's progress lambda with a fake engine that reports whatever the test tells it to, and ProgressNotificationTest covers the throttling the same way. The arithmetic was the one part with no reader. The new test passes 30 s as the duration for a fixture that is exactly 3.000 s, so the conversion still encodes the whole clip and the reported percentage tops out around 10 rather than 100. That is what makes it bite. A range check alone is worthless: a constant 0 satisfies both "every value is in 0..100" and "the values never go backwards", and so does a list of [0, 100]. Pinning the band rejects every constant, and because the band sits a tenth of the way up it also rejects an implementation that ignores durationMs, which would report ~100 for the same run. The bound is loose -- 5..25 for an expected 10 -- because the last statistics callback can land slightly before the final frame. Verified on a local API 34 emulator: 61 tests, 0 failures, 3 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
ba16f5a89b |
Make the fallback test say when it cannot test the fallback (#223)
HardwareFallbackTest is the only automated check of the hardware->software fallback against a real codec failure, and it passed on every CI leg without ever attempting the hardware path. Measured on run 34004304566: the API 33, 34, 35 and 37 legs each log Routing sample_h264_444.mp4 -> ... via FFMPEG (NO_HARDWARE_ENCODER) Emulators expose no hardware encoder, so the router never chooses Media3 and runMedia3OrFallBack's catch is never entered. The test's assertions -- succeeded, output non-empty -- are true of that conversion too. It finished in 448 ms, which is not long enough to fail an export and then software-encode a three-second clip. Deleting the catch reddened nothing. The ticket offered two fixes and left the choice open. Trying the first one answered it, and not the way the ticket expected. Pinning deviceCodecs to PERMISSIVE, as ForcedFailureTest does, makes the router choose Media3 -- and the export then SUCCEEDS. On a local API 34 emulator, MediaCodecInfo logs NoSupport [codec.profileLevel, avc1.F4000C, video/avc] for both c2.goldfish.h264.decoder and c2.android.avc.decoder, and ExoPlayer allocates the goldfish decoder anyway, which decodes the High 4:4:4 fixture regardless of the profile it declares. c2.android.hevc.encoder then encodes it and the job reports MEDIA3. So the class KDoc's "Media3 fails partway through the export on every device" is not true of the emulator images, and no routing pressure makes this fixture force a fallback there. Pinning would also swap in software codecs, which is not the path a real device takes -- it is what made the forced run succeed. That leaves assumeTrue on the production premise as the honest answer, now with a measurement behind it rather than a coin flip. The test skips where it cannot mean anything and runs on the Pixel, where it always could. When it does run the assertion is a pair, because KEY_ENGINE_USED is FFMPEG whether the fallback fired or the router went straight there: the router chose MEDIA3 for this request on this device, AND the worker reported FFMPEG. Together, and only together, that is the fallback. Verified on a local API 34 emulator: the test reports SKIPPED and the level reports skipped=3. ForcedFailureTest still covers the fallback wiring on every leg with a double; what needs a real encoder is two real engines disagreeing about a real file. The third permanent skip is recorded in docs/local-emulator.md and beside SafPickerRoundTripTest's run-shape note. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
e2f8ef2918 |
Cite the two measurements the last two commits asserted
The advisory-leg count (4 expected, 4 received, 4 failed with the new marker) is api37-debug run 34008889182, dispatched with the annotation as its filter. The force-stop recovery was made to go red before it was believed: on a local API 36 emulator, with the picker left open and the back presses removed, the test passes with forceStopThePicker() and fails with exactly the API 37 message without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
9f06eb9988 |
Check that FLAC and Opus are FLAC and Opus (#228)
encodesFlacLosslessAudio and encodesOpus asserted only that a non-empty file appeared. Five siblings in the same class check what is in it -- encodesWav reads RIFF two lines away, and GIF, Matroska, MP3, H.264 and H.265 all assert a container marker or a track MIME. convert() throws on a non-zero return code, so these two did prove the command ran. What they could not distinguish is the command running and producing the wrong thing, which is a failure mode this codebase has already had once: F1 in docs/coverage-read-findings.md records a live Vorbis arm in FFmpegCommandBuilder that ContainerCapabilities says cannot exist. Pointing OutputFormat.FLAC's arm at pcm_s16le left both tests green. Four bytes each, in the idiom the class already uses. OutputFormat.FLAC is Container.FLAC, whose ffmpeg format is "flac", so the file opens with the native stream marker fLaC. OutputFormat.OPUS is Container.OGG -- "ogg" -- so it is an Ogg stream and opens with OggS. Verified against both muxers directly rather than assumed from the codec name; the marker belongs to the container, so it does not vary with the ffmpeg build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
d759ef32f1 |
Take the picker test off the API 37 gating leg, and give the SystemUI disable root
The gating `E2E API 37` leg failed three of the last ten status_check runs. Four runs were read logcat-first -- 34006456986, 34001744574, 34001377499 and the green 34002313300 -- and each carries exactly two `hasReadColorBufferDma` aborts before the suite (surfaceflinger, during boot and the SystemUI disable) and exactly one during it: `system_server`, thread `TaskSnapshotPer`, always inside `pickingAFileThroughTheSystemPickerFillsInTheFileCard`'s window. Nothing else in the gating set reaches the mapper. So that test kills the framework on this image whether it passes or not, and whether the leg goes red is luck: 34001377499 passed it and lost the leg anyway with `failed: 0`, 34002313300 passed it 0.6 s after the abort and went green. That is #108. The test now carries `@FailsOnEmulatorApi37` and the baseline goes 3 -> 4; the marker's own wording widens from "does not pass on this image" to "cannot be run on this image", because this carrier passes about half the time. `docs/api-37-emulator-crash.md` had counted those aborts on 2026-08-24, put them in its table, and then read the pass/fail column alone. The correction is recorded beside the original rather than replacing it. Probed on the same image and recorded there too: there is no shell knob for task snapshots -- not in `getprop`, `settings`, `device_config` or `cmd window` -- so the marker is the available answer rather than the lazy one. Two separate defects came out of the same logcats. `disable_region_sampling` has never restarted the framework on CI. `adb shell stop` and `start` are root-only and every API 37 leg has printed `Must be root` for both, so SystemUI stayed up for the whole run -- visible directly as `WindowManagerShell ... app=com.android.systemui` minutes after "final state: SystemUI disabled". Both waits also printed their own exhaustion as an elapsed time, so "system_server down after ~40 s" is what a stop that did nothing looks like. Measured on the local android-37.0 AVD, same fingerprint as CI: `adb root` makes `stop` return 0 with `pidof system_server` empty. Root is dropped again before Gradle runs, and both waits now say whether they observed anything. And when the picker test does fail, the abort is the coda rather than the cause: `InputDispatcher: No new touched window at (539.0, 525.0)` is in both reds and absent from the green, so the tap on the root is discarded, the picker is never navigated, and all four back presses land on an activity WindowManager says has not added a window yet. `forceStopThePicker` goes around input entirely so `pickTheFixture`'s whole-picker retry -- which exists for exactly this -- becomes reachable. That one is a fix on every API level, not just 37. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
39327beea7 |
Assert the join failure message against a real FFmpeg session
#217 closed by noting the join legs had not been run locally. Running them would not have answered it: nothing on either source set drove a real join *failure*, so the message unified in #203 was asserted only against values a JVM test hands to sessionOutcome directly. CI had already run those legs green on the merged commit, so the outstanding item was a gap in coverage rather than a gap in execution. The new case forces a failure with an input that does not exist -- rejected the same way by every FFmpeg build, unlike malformed media -- and asserts the message names the operation, carries the return code, and has detail after it. That last clause is the device-only half. getReturnCode, getFailStackTrace and getAllLogsAsString are native reads; if the log tail came back empty on a device the user would see "Joining failed (1): " with nothing after the colon, and every JVM test would still pass. It deliberately does not pin which detail source wins. On an ordinary non-zero return code FFmpegKit reports no fail stack trace, so stack-trace-first and log-tail-first produce identical text and no assertion here could tell them apart. SessionOutcomeTest pins that, where both sources can be non-blank at once. Claiming it here would be a KDoc asserting more than the test checks. Measured on the local API 33 emulator: 61/61 green with the test, and with both detail sources nulled in ConcatEngine it fails on the intended assertion -- "the message stopped at the return code and told the user nothing, was: 'Joining failed (1): '" -- so the prefix and return code survive the mutation and only the device-only claim goes red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
32ab54da3c |
Wait for the rotation to rebuild the Activity, not for the composition to idle
thePickedInputSurvivesARealRotation synchronised a rotation with waitForIdle(), which waits for the compose hierarchy to settle. Right after a rotation the window manager has accepted but not yet delivered as a configuration change, the old Activity's composition is already idle -- so it returns, composeRule.activity still resolves to the old instance, and the guard reads an unchanged identity hash. Nothing waited for MainActivity to be rebuilt. That is the clean AssertionError on #217's API 33 gating leg, run 33698846104: it failed the SECOND guard, so the first had passed and the display really had rotated. The wedges this ticket opened with are the same race taken the other way -- land while the composition is being torn down and waitForIdle has nothing coherent to settle on. awaitRecreation() waits on a counter fed by the runner's lifecycle monitor, bounded at 15 s. Deliberately not polling composeRule.activity: that resolves through scenario.onActivity, which blocks on the main thread, so polling it across a recreation is a plausible reading of the very wedge being fixed. Both guards stay. The identity-hash one is now a backstop rather than the primary detector -- a configChanges attribute trips the barrier's timeout first, with a message saying what was waited for. Measured on the local API 33 emulator. With the fix, 60/60 green. With the watcher mutated so the counter never increments, the test fails in 15 s naming itself and its condition, and the run still reports received 60/60 completed cleanly -- where the same missing recreation used to cost the leg 20 minutes and name nothing. The pre-fix flake does not reproduce on this host, so that is a demonstration of the timeout path, not a before/after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
e7caeeac43 |
Finish the startup sweep before onCreate returns, in the JVM suite
Robolectric builds an Application per test class that asks for one, and each onCreate launched a staging sweep on Dispatchers.IO over the shared <cacheDir>/conversions/. Nothing joined them, so a test asserting about a staged file was racing every sweep the classes before it had left in flight (#159). It was CI-only until wave 4 added ten Robolectric classes, at which point OutputPublisherStagingTest started failing locally too. LibreMediaConverterApp gains a protected open sweepScope and publishes the Job onCreate started; the JVM suite substitutes TestLibreMediaConverterApp, whose scope is Dispatchers.Unconfined so the sweep -- a plain function that never suspends -- runs to completion inline. The SupervisorJob is kept so this differs from production in the dispatcher alone. Suite-wide rather than per-test: 27 of the 58 Robolectric classes touch that directory, so opt-in was not a real option. It costs one assertion, knowingly. AppStartSweepTest opened by asserting that the manifest's android:name is what Robolectric instantiated. An application= override replaces the manifest rather than being checked against it, and applicationInfo.className reports the override too, so that claim is now unobservable from this source set and a rewritten version would assert the override against itself. The manifest link is device-only; the cast in setUp still catches the test app ceasing to extend the real one. AppStartSweepTest also joins the published Job instead of polling for ten seconds -- a poll cannot tell "swept" from "not started yet" -- and gains a test pinning that the sweep is complete when onCreate returns, which is the property the substitution exists for and the only place it is checked. Verified by mutation rather than by repetition. Putting the test app back on Dispatchers.IO reddens that test 5 times out of 5, while running the whole suite six times per arm caught nothing either way: at the rate #159 was observed at, a clean six-run arm is roughly a coin flip, so the comparison was underpowered and is not offered as evidence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
ec2cae256f |
Give both engines one rc-to-outcome function, and unify the failure message (#203)
FFmpegEngine and ConcatEngine each carried their own copy of the same `when`, and the copies had drifted: one preferred the fail stack trace and fell back to the log tail, the other only ever read the log tail. Neither was tested -- both live inside a callback handed to FFmpegKit, which does not run on the JVM -- so nothing could see that the two disagreed about what a failed session says. sessionOutcome() now holds the rule and each engine maps Success/Cancelled/Failed onto its continuation. Verified JVM-safe rather than assumed: javap over the committed AAR shows ReturnCode(int) as a plain public constructor with pure static isSuccess/isCancel and a <clinit> that loads no native library. Per #203's decision this unifies on the stack trace, so a join failure now carries the diagnostics a conversion failure always did. The PREFIX stays per-engine: unifying the strategy must not unify the sentence, since a join reporting "FFmpeg failed" would be a worse message than the one it replaces. There is a test for exactly that. The two message sources are lambdas rather than values, and that is load-bearing. getFailStackTrace and getAllLogsAsString are calls onto a native session, and only the failure arm needs either; taking them by value would put both on the happy path of every successful conversion, which the shape this replaces did not -- it read them inside the else branch. Same reasoning as capabilitiesFrom taking a Sequence in #194: a seam should not change what runs when. There is a test that counts the reads, and the eager mutation reddens it. A null return code is a real input rather than a defensive one -- getReturnCode() is nullable and a session killed before reporting has none -- so it fails, with "null" where the number would be. Nothing asserted the old join text: `grep -rn 'Joining failed|FFmpeg failed' app/src/` returns only main, plus ConcatWorker.GENERIC_FAILURE_MESSAGE, which is a different constant this does not touch. Re-run immediately before committing, as the ticket asked. Mutations, all run and restored: swap the ifBlank operands 1 red treat cancellation as a failure 2 red read both message sources eagerly 1 red hardcode the prefix 1 red Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
4e88de3045 |
Stop a permission answer starting a second conversion (#202)
currentInput() answered for Converting, Waiting and Converted as well as Ready. Those three arms were unreachable by tapping Convert -- the button renders only in the Ready branch -- but they were reachable through the POST_NOTIFICATIONS *result*, which ConverterScreen.kt:91 wires to convert() rather than to the button. Reaching one enqueued a SECOND job over a live one: activeWorkId was overwritten, and the first job kept running with its foreground notification orphaned and nothing left holding its id to cancel it. #202 decided to narrow rather than to test it as it stood, because a test written against the old shape would have frozen the double-enqueue as intended behaviour -- the F1/F5 failure mode docs/coverage-read-findings.md names. currentInput() is now (_state.value as? ConversionState.Ready)?.input, which is what JoinViewModel.join() has been all along; the two screens are the same shape and only one of them was over-general. Four cold refusal arms come with it, all reached the same way -- a system callback arriving after the screen has moved on, which is what a result redelivered after process death does: ConversionViewModel.kt:513 currentInput() ?: return ConversionViewModel.kt:600 pendingSave() ?: return JoinViewModel.kt:316 (as? Ready)?.inputs ?: return JoinViewModel.kt:390 pendingSave() ?: return One fixture note worth keeping: the second case needs a real staged file in the worker's output Data. A SUCCEEDED job with no output path maps to Failed rather than Converted, so Data.EMPTY never reaches the state the case is about -- which cost a timed-out awaitState before it was spotted. Mutations, all run and restored: restore the over-general four-arm when 1 red <- the defect this change fixes currentInput()!! at :513 2 red pendingSave()!! in save() 1 red drop both join guards 1 red Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
2db0dc65d3 |
Open the save dialog with the type the job produced (#201)
ConverterScreen.kt:80 -- state.pendingSave()?.mimeType ?: settings.spec.mimeType -- had never taken its left-hand side. Its comment records what the line is for: a retry after a failed save must open with the type its FIRST attempt used, because the fallback beside it is the current picker, which a reattached job never set. So the untested half is the fix and the tested half is the fallback it was added to stop being used. This withdraws a named exemption rather than working around it. FailedSaveRetryTest's KDoc listed the line as not asserted because "it lives in the entry point, above the ScreenContent seam, and reaching it needs a real ViewModel inside a composition". True when written; AdaptiveShellTest (#173) then established composing the real screens with real ViewModels, and #200 added the ShadowActivity mechanics for reading what a launcher launched. The reason the exemption gave no longer holds, so it is withdrawn in the same change rather than left to be taken at face value -- the shape of #141 revising #84's boundary. The job is reattached rather than run because the screen composes its own ViewModel through viewModel() and nothing can be injected into it. That is also the case the line exists for: a reattached job's spec was never in these settings at all. The test asserts the two mime types differ as well as which one is used. Without that, the assertion would pass just as well against the fallback if the fixture ever drifted onto MP4. Mutation: collapse :80 to settings.spec.mimeType -- red. Run and restored. Unrelated, and recorded because it turned up here: #159 now reproduces on this host. The full suite failed once in six runs on OutputPublisherStagingTest:184, and the isolating experiment says it is not this change -- three runs WITH the new test all passed, and the failure occurred on a run with the file removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
6334dcba34 |
Pin the launcher layer, where two callbacks share a signature (#200)
ConversionViewModel.onInputPicked(uri: Uri) and .save(destination: Uri) are both
(Uri) -> Unit, so swapping the two launcher callbacks at ConverterScreen.kt:70 and :83
compiles, renders, and passed the entire suite. Picking a file would attempt a save to it;
choosing a destination would load it as input.
That is the defect class ScreenWiringTest exists for, on the one pair it declines to cover:
it drives converterActions directly and says the launcher-backed actions stay parameters.
Right about the actions seam, and it leaves the edge above that seam unpinned. Join's
equivalents are List<Uri> and Uri, so they are not transposable and get no such test.
Two mechanics, neither used anywhere else in the suite, so both were spiked before any
assertion was written:
shadowOf(activity).nextStartedActivityForResult reads the launched Intent, EXTRA_MIME_TYPES intact
shadowOf(activity).receiveResult(...) reaches ComponentActivity's ActivityResultRegistry
and fires the rememberLauncherForActivityResult callback
createAndroidComposeRule for AdaptiveShellTest's reason: the screens compose real ViewModels
through viewModel(), and the plain rule supplies no ViewModelStoreOwner.
The picker filter rides along, since the harness is the same. ConverterScreen.kt:65-67
records why the all-types wildcard is load-bearing -- the photo picker offers no audio and
misses mkv/flac/webm -- and narrowing it would have made every audio conversion unreachable
from the picker with nothing going red.
One incidental: a KDoc cannot contain the all-types wildcard, because its second half closes
the block comment. The literal is spelled only in the assertion, and the KDoc says why.
Mutations, all run and restored:
transpose onInputPicked and save 1 red
narrow the converter picker to video only 1 red
widen the join picker to every type 1 red
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
JMR-devandClaude Opus 5
|
7e09f010c7 |
Call the theme the way MainActivity calls it (#197)
ThemeColorSchemeTest resolves every branch of the `when` and always passes darkTheme explicitly, so the $default bridge is never entered and isSystemInDarkTheme() is never called. MainActivity.kt:79 is its only default-argument caller and does not execute on the JVM, which left the app's actual call shape -- no arguments at all -- the one nothing exercised. LibreMediaConverterTheme reported mi=21, mb=6, cb=12 at method level. Not #68. That issue is the two unreachable arms, DarkColorScheme and LightColorScheme, which cannot run because dynamicColor is always true and nothing can flip it; it is an open product decision and stays open. This is the reachable half. The assertion compares schemes rather than reading a luminance threshold, which would be a guess about the device palette. What is asserted is that the no-argument call resolves the SAME scheme an explicit darkTheme of the matching value does, and a different one from its opposite -- true whatever palette the platform hands back, and exactly the claim being made: the default reads the system rather than picking a side. The two assertions are also what stops the pair passing vacuously if all three resolutions were identical. Two @Config(qualifiers = ...) cases rather than two classes: qualifiers are settable per method, unlike the sdk pinning ForegroundTypeRegimeTest needed nested classes for. Mutations, all run and restored, and each reddening a different half -- which is also what shows the qualifiers take effect rather than both cases running in one mode: darkTheme defaulted to false night case red darkTheme defaulted to true light case red isSystemInDarkTheme() inverted both red Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
49249be280 |
Report hardware progress to WorkManager, which nothing had checked (#196)
ConversionWorker.kt:208-210 is a second onProgress lambda at a second call site -- the one handed to engine.transcode -- and it reported ci == 0. Every test in this file drives the FFmpeg path; HardwareFallbackTest reaches runMedia3OrFallBack but its recording transcoder records the call and never invokes the callback it was handed. So the two engines' progress wiring was one tested and one not, and the untested one is the default: ConversionRouter sends everything it can to Media3, which makes this the lambda most conversions actually use. Same asymmetry argument CLAUDE.md records for ContainerCapabilities:94. It goes in this file rather than beside HardwareFallbackTest because this is where progress plumbing lives and where RecordingForegroundUpdater already is -- and because the software and hardware cases now sit side by side, which is what makes the asymmetry visible rather than merely fixed. workerReporting gains an engine-preference parameter defaulted to FORCE_SOFTWARE, so no existing case changes. AUTO with a real H.264 probe, because FORCE_SOFTWARE is exactly what keeps the other tests out of this branch, and because InputProbe() reports UNPARSEABLE -- which PERMISSIVE.canDecode refuses, sending every job to FFmpeg with no test saying why. The percentage is asserted, not merely that an update happened: publishProgress takes a display name and a percent, and replacing the percent with a constant compiles fine. Mutations, both run and restored: empty the hardware onProgress lambda 1 red report a constant percent instead of the engine's 1 red Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
2125763ebf |
Read FFprobe's answer without spawning FFprobe (#195)
readMediaInformation was 114 missed instructions and 24 missed branches -- the second
largest block on the wave-4 report -- and exactly one line of it needed a device:
FFprobeKit.getMediaInformation(path).getMediaInformation()
Everything after it reads an ordinary object, so it moves into ffprobeInfoFrom and the edge
keeps the call and the null check. Verified JVM-safe rather than assumed: javap over the
committed AAR's runtime jar shows MediaInformation(JSONObject, List<StreamInformation>,
List<Chapter>) and StreamInformation(JSONObject) as plain public constructors whose <clinit>
does not load the native library, so a test builds its own without libffmpegkit present.
The decision worth reaching is containerFrom's SECOND argument. FFprobe reports
"matroska,webm" for both MKV and WebM because they share a demuxer, so the video codec is
the only thing separating them. containerFrom has thirty-three covered branches and not one
can notice that argument being dropped -- the mistake is at the call, not in the callee, so
every existing containerFrom test stays green while every VP9 WebM quietly becomes an MKV.
Two things the tests found rather than confirmed.
The format properties are NESTED under "format": getFormat() resolves through
getStringFormatProperty, not off the top-level object. The first fixture put the keys at the
top level and four cases failed with a null container. The helper says so now.
And one mutation SURVIVED on the first pass -- reading dimensions with
streams.firstNotNullOfOrNull { it.getWidth() } instead of video?.getWidth(). The fixture put
the dimensions on the chosen video stream, which is also the first stream carrying any, so
the two readings agreed and the test could not tell them apart. Separating them needs a
chosen video stream with NO dimensions and a later one that has them, which is a real shape:
FFprobe omits width/height for a stream it could not measure. That case is now its own test
and the mutation reddens it.
Mutations, each run and restored:
drop the video codec argument to containerFrom 1 red
take the LAST video stream instead of the first 1 red
read dimensions from any stream, not the chosen one 0 red -> 1 red after the new case
let an unparseable duration throw instead of zero 1 red
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
JMR-devandClaude Opus 5
|
6d700f0014 |
Cut a seam through the codec enumeration, and say what a failed one actually does (#194)
probe() was 20 never-executed lines, the biggest single block on the report. It has been looked at twice and left out twice, and both closes were right about what they closed: #86 ruled it device-bound, and #133 re-checked that with ShadowMediaCodecList in hand and still declined, because MediaCodecInfoBuilder has no setIsAlias and no setCanonicalName -- "so the alias skip and the canonical-name dedup, the two things the class's KDoc calls out as easy to get wrong, are not reachable through it." That objection is about the shadow. It does not apply to a function taking its own entry type, which #133 did not evaluate. capabilitiesFrom(enumerate: () -> Sequence<CodecEntry>) holds every rule; the edge keeps only the mapping from MediaCodecList onto CodecEntry. The parameter is a Sequence rather than a List on purpose. runCatching has always wrapped the *iteration*, so a MediaCodecInfo whose properties throw partway leaves the codecs already read in place. A List parameter would move that throw outside the loop and turn a partial answer into an empty one -- a behaviour change smuggled in as a refactor. There is now a test for the partial case, and swapping the Sequence for an eager toList() reddens it. The behaviour change this DOES make is one line of log, and it is the reason the seam was worth cutting at all. The fallback said "Codec enumeration failed; assuming permissive" and returned empty sets -- but "video/avc" in emptySet() is false, so canEncode and canDecode answer no to everything and every job routes to FFmpeg. That is the restrictive answer, and it is the right one: FFmpeg does whatever Media3 does, only slower. The code stays; the message and the class KDoc now describe it. Seven tests. One of them was wrong first and the mutation is what said so: the alias case originally listed the alias *after* the codec it aliases and passed with the skip deleted, because canonicalName is shared and the dedup catches the second entry either way. The two rules overlap, so a fixture that does not separate them tests neither. Order separates them -- an alias arriving first claims the canonical name in `seen` and gets its own types credited, and the real codec is then dropped by the dedup. That is now the test, and it also says what the rule is worth: with a Set accumulator, an alias declaring the same types as its codec changes nothing, so the skip earns its place only when the two disagree. Mutations, each run and restored, each reddening the test that owns it: drop !isSoftwareOnly from the encoder predicate 1 red remove the alias skip 1 red (0 before the fixture was fixed) remove the canonical-name dedup 1 red remove the video/ prefix filter 1 red apply the hardware predicate to decoders too 1 red make the failure fallback permissive 2 red eager toList() instead of the lazy Sequence 1 red Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
da8d53851b |
Render the container row for a video nothing could name (#199)
ConverterScreen.kt:668's null arm -- DetailRow("Container", probe.container?.label ?:
"Unknown") in the VIDEO branch -- had never rendered. Every video case in FileCardTest uses
VIDEO_PROBE, which carries container = MP4.
The argument for adding it is the asymmetry, not the coverage. FileCard renders that exact
expression twice, once in AUDIO_ONLY (:660) and once in VIDEO (:668), and "an audio-only
file nothing else could describe degrades one row at a time" drives only the first. Same
expression, same fallback, one kind covered and one not -- which is the same argument
CLAUDE.md records for including ContainerCapabilities:94.
Nor is null an edge case here. InputProbe.container's own KDoc says MediaExtractor cannot
report a container at all, so it comes from FFprobe alone: any run where FFprobe did not
answer produces exactly this shape -- real codec, real dimensions, real duration, no
container. An empty value in its place would read as a rendering bug rather than as a
probe that got half its sources.
The other three rows are asserted alongside, which is what keeps this from being a copy of
the audio-only case. There, everything is unknown at once; here one field is missing from
a probe that is otherwise complete, and the rest have to be unaffected by it.
Mutation: `?: "Unknown"` -> `?: ""` at :668 only, run and restored. The AUDIO_ONLY twin at
:660 is a separate expression, and mutating that one would redden the existing test instead
-- which would prove nothing about this one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
JMR-devandClaude Opus 5
|
cc215195ee |
Build a command for the audio the user turned off (#198)
audioArgs' Drop arm -- `AudioPlan.Drop -> listOf("-an")` -- was ci == 0. The suite's only
-an assertion lives in "gif generates a palette to avoid banding and drops audio", and that
one comes from the image path at FFmpegCommandBuilder.kt:79/:90, which emits -an directly
and never reaches audioArgs. Two sites, one string, one tested.
It is a live path rather than defensive code. AdvancedPicker renders all of
AudioCodec.entries including NONE, ContainerCapabilities.validate permits audio-off whenever
the input has video, and MKV routes the job to FFmpeg -- so "convert this and drop the
soundtrack" is something a user can do today and nothing had built the command for.
Both halves are asserted, and the second is not padding: -an alone still passes if the arm
falls through to the else and emits an AAC encoder beside the flag, which is a file that is
silent because the flag won while carrying an encoder nobody asked for.
Three mutations, all run and restored. The third is the one that justifies the second
assertion, since the first two break -an as a side effect and so cannot show it:
Drop -> emptyList() red
Drop -> the else arm's aac encoder red (loses -an as well)
Drop -> listOf("-an", "-c:a", "aac") red -- -an intact, caught by assertFalse
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
JMR-devandClaude Opus 5
|
92bcff8656 |
Make a failure that says nothing still say something (#193)
Three sites, all ci == 0 before this, and all the same rule: work/ConversionWorker.kt:316 cause.message ?: GENERIC_FAILURE_MESSAGE convert/ConversionViewModel.kt:631 e.message ?: SAVE_FAILED_MESSAGE join/JoinViewModel.kt:416 e.message ?: SAVE_FAILED_MESSAGE Every existing test throws WITH a message, so the right-hand side had never been evaluated anywhere in the suite. A Throwable carrying none is not exotic: RuntimeException(), IOException() and most platform exceptions raised without an argument all have a null message, and a native engine that dies is exactly where one comes from. The worker case needed care, and the care is the reason it survived three waves. ConversionStateMappingTest's "a failure with nothing said still says something" looks like it covers that site and does not -- it drives the READ side, map(FAILED, Data.EMPTY), and that side has a fallback of its own at ConversionViewModel.kt:147-149 which turns a blank KEY_ERROR back into the same constant. So a test asserting on the resulting Failed state stays green while the worker's fallback is broken. Measured rather than reasoned: with :316 mutated to .orEmpty(), exactly ONE of 587 tests went red, and it was the new one. Everything else, including the test that appears to cover it, stayed green. So the worker case reads KEY_ERROR off the worker's own Result, before anything downstream can repair it. The two save cases have no such second line -- both write _state.value directly -- so the state is the right thing to assert there, and both also assert that `pending` still travels: a fallback that dropped the handle would leave the file unreachable from the very screen that just said the save failed. Held in one class against the ticket's suggestion of three. They are one rule at three layers, and the masking above has to be explained once rather than three times. FailedSaveRetryTest already sets the precedent for both ViewModels in one file; this adds one worker to that shape. FORCE_SOFTWARE in the worker fixture so the failure comes straight out of runFFmpeg. AUTO would enter runMedia3OrFallBack, whose catch runs the job a second time in software: the same exception arrives, but by a path this is not about and which HardwareFallbackTest owns. Mutations, all run and restored -- each site to .orEmpty(), never to a different constant, which would only prove the test reads a constant: ConversionWorker:316 1 of 587 red (this file) ConversionViewModel:631 red JoinViewModel:416 red Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
e4867ff956 |
Connect the Cancel button to WorkManager, which nothing did (#192)
Both ViewModels' cancel() is one line -- activeWorkId?.let(workManager::cancelWorkById) -- and JaCoCo reports every line of both as covered. The only test of either was SettingsEditsTest's "cancelling with no active job does nothing rather than throwing", whose own comment names the half it drives: "the null side". The other side had never been entered, and JoinViewModel.cancel() had no test at all. So nothing in 584 tests connected the Cancel button to WorkManager. The affordance tests click TestTags.CANCEL and assert the action fires into a stub; ScreenWiringTest asserts the action calls viewModel.cancel(). Both halves were pinned and the join between them was not. No line-level filter could have found this. It takes a method-level read -- mi=11, ci=7, mb=1, cb=1 on both -- a covered method with an arm nothing takes, which is the second of the two filters #194 records and the gap that argued for adding it. The fixture is why this stayed uncovered rather than why it is hard. The test WorkManager runs on a SynchronousExecutor, so an ordinary request finishes inline: by the time a test could call cancel(), convert()'s job was already terminal, leaving only the null arm reachable. setInitialDelay is what TestScheduler honours, so the job sits in ENQUEUED and the test never releases it. Production never sets a delay, so the request is built by hand rather than through ConversionWorker.request -- but ENQUEUED at runAttemptCount 0 is a real state every job passes through, Reattachment.choose ranks it QUEUED, and conversionStateFrom maps it to Converting(input, 0). The delay changes how long the job stays in a real state, not which state it is in. Three tests, and the third is not padding: without it, cancelAllWork() in place of cancelWorkById(activeWorkId) passes the other two. It asserts the shape rather than the identity -- exactly one of two queued jobs is cancelled -- because which one the ViewModel reattached to is the query's business, and Reattachment's ordering notes say queued jobs are left tied deliberately. WorkManager's own record is asserted before the screen. The screen alone would be weaker than it looks: CANCELLED maps to Idle for a reattached job, and Idle is also where a ViewModel that did nothing whatsoever would sit. Mutations, both run and both restored: cancel() -> no-op all three red cancelWorkById(id) -> cancelAllWork() only the third red The second is what shows the third test does independent work rather than restating the first two. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
aa7e1d8b01 |
C1 (#176): give ConcatWorker the seam ConversionWorker always had, and test what was behind it
ConcatWorker constructed ConcatEngine in place while ConversionWorker reached its engines through ConversionDependencies. That asymmetry is the whole reason one worker had a tested failure path and the other had none: everything past setForeground was untested on *every* source set, JVM and device alike. The repo had already measured the cost and written it down. PerJobStagingTest's KDoc records that **reverting ConcatWorker to a constant staging name left all 257 tests green**, because nothing could reach the line that names the file. RefusedJobTest says it from the other side -- "the next thing past the count guard is ConcatEngine, which is native". That mutation is red now. The seam is `ConversionDependencies.concat: (Context) -> ConcatJoiner`, beside .hardware and .software. ConcatEngine implements the interface; its Result type stays nested in the implementation, because moving it would touch every call site to buy nothing -- what a test needs is the ability to not run FFmpeg, and that is the method, not the type. Five tests, and the mutations that hold them: the engine's own reason reaches the user replace e.message with the generic string a failure with no message still says one drop the ?: GENERIC_FAILURE_MESSAGE fallback a failed join deletes its partial drop staged.delete() from the catch no input array at all is refused swap in TOO_FEW_INPUTS_MESSAGE a join reports its own staged file revert to the constant staging name The delete test was vacuous on its first draft and the mutation caught it: it scanned the staging directory for a "join-" prefix that StagingNames.forJob does not produce -- it names files <jobId>.<ext> -- so the assertion was trivially true. Rewritten to assert against the handle the joiner was actually given. Two small fixes ride along, both the repo's own conventions rather than new opinions. "No input files." becomes NO_INPUTS_MESSAGE, per #158: a message the user can see is named once, so a test asserts the string the worker writes rather than a copy that can drift. And the JVM now covers that arm, which ran before staging and before any native code and had no business being a device test. 579 -> 584 JVM tests, 0 failures. ConcatWorker: 14 -> 5 missed lines, 2 -> 0 missed branches. Line 2173/2348 -> 2183/2352; branch 1091/1342 unchanged in the numerator. The line denominator moved 2348 -> 2352: that is the ConcatJoiner interface, not new untested code. Said plainly because CLAUDE.md's coverage entry has a history of explaining its own numbers wrongly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
794cef7b34 |
C2 (#177): cut MediaProbe's two-probe merge into a seam, and ask which probe wins
probe() runs MediaExtractor and FFprobe independently and merges the two, and every rule in that merge is a decision nothing held. The reason is structural rather than an oversight: RemuxTest drives the whole thing on a device against committed fixtures, but only ever with one probe answering and the other agreeing or also failing. Nothing on any source set can arrange for a real extractor and a real FFprobe to *disagree*, so every elvis in the merge was taken in one direction and never the other. The seam is `internal fun merge(Extracted?, FFprobeInfo?): InputProbe`, pulled out of probe() whole -- probe() now reads the two probes, merges, and keeps the log. FFprobeInfo becomes internal alongside it; Extracted already was, with a KDoc giving this exact reason, and FFprobeInfo simply never got the same treatment. Half a signature being private is what made the function unnameable from a test. Eleven tests, and the mutations that hold them: image beats a real video codec demote the isImage arm below the video arm the extractor wins on codecs flip the elvis to FFprobe-first duration is the larger reading replace maxOf with extractor-first dimensions prefer the extractor flip the width elvis no recognised stream is unreadable narrow the guard to `extracted == null && info == null` All five red, then restored. One mutation I tried first was *semantically equivalent* -- moving the image arm above the both-null arm changes nothing for any reachable input -- so it stayed green and is recorded here rather than counted: a green mutation is only evidence when the mutation is a real change. The last row is the arm the ticket was filed for: parsed, and carrying no stream either probe recognised, which is what a container holding only subtitles looks like. Its input was already being constructed elsewhere in the suite -- MediaProbeTrackWalkTest calls extractedFrom(emptyList()) and gets exactly it -- and had never been handed to the merge. 568 -> 579 JVM tests, 0 failures. MediaProbe: 35 -> 24 missed lines, 72 -> 40 missed branches. Line 2103/2348 -> 2173/2348; branch 1029/1340 -> 1091/1342. The branch denominator moved by two, and it is the seam that moved it -- worth stating separately from the numerator, because CLAUDE.md's coverage entry has a documented history of explaining its own numbers wrongly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
eded47d666 |
B1 (#173): tell the rail from the bottom bar, and the Convert tab from the Join tab
Two assertion gaps, not coverage gaps, which is why they lasted. AppRootRestorationTest already drives AppRoot at Compact and Expanded, so JaCoCo is green on useRail -- but it asserts only that the selected tab survives recreation, through a stub `content` composable. Nothing anywhere queried for a rail or a bar, and nothing composed the real screens. Measured before this file existed: - transposing the NavigationRail and NavigationBar bodies passed the entire suite - transposing Content's two arms passed it too A tablet showing phone chrome, or the Convert tab opening the Join screen, and 546 tests with nothing to say about either. AppRoot's own KDoc is why that matters more than it looks: from targetSdk 37 the app is resized and rotated whether or not it is ready, so the width class is not a preference. WindowWidthSizeClass.Medium appears in no test in either source set today. useRail is `!= Compact`, so Medium takes the rail; narrowing it to `== Expanded` is one character and breaks every tablet and unfolded foldable. That mutation is red now, and it is red only because of the Medium test -- the Compact and Expanded ones both survive it. Two things this needed: **createAndroidComposeRule rather than createComposeRule.** Rendering AppRoot with its default content reaches ConverterScreen's `viewModel = viewModel()`, which needs a ViewModelStoreOwner. It works because both ViewModels are `@JvmOverloads constructor(app: Application, ...)` so AndroidViewModelFactory can build them, and because ui-test-manifest's debugImplementation entry already puts a ComponentActivity in the merged manifest the unit tests build against -- which app/build.gradle.kts says in terms. Checked with a throwaway spike before the ticket was filed, rather than discovered here. **Two tags, applied inside main.** The only production change: TestTags.Shell, set on the rail and the bar. There is no other way to tell the two apart -- both render the same two destinations with the same labels and the same selection state, so any assertion writable without them is satisfied by either layout. In TestTags and applied by the shell rather than handed down by the test, for the reason that file's KDoc gives: a tag the test supplies proves only that the test set it. TagTableUniquenessTest covers the new group. 564 -> 568 JVM tests, 0 failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
b41341a1cb |
B3 (#175): pin the AAC arm every ordinary conversion takes
audioArgs has six arms. Five are named codecs with tests; AAC arrives through the `else`, so nothing named it -- neither "aac" nor "192k" appeared anywhere in FFmpegCommandBuilderTest. It is the audio MP4 and M4A get, which is to say the audio the picker offers first and most conversions produce. Both halves are asserted, and the bitrate is the half worth arguing for: an -b:a that quietly changed would fail nothing, look wrong in no command line, and surface only as files that sound different from the ones the app produced last month. Both mutations confirmed red -- 192k -> 128k and aac -> libfdk_aac. Asserted through MP4_H264 and M4A_AAC rather than one of them, so an AAC arm added above the `else` later has to keep answering the same way for both. **Deliberately not added here: an ENCODABLE_AUDIO-vs-audioArgs agreement test**, the obvious companion to VideoCodecMimeAgreementTest. It would freeze the answer to F1, which is open: ContainerCapabilities.kt:84 says "nothing here emits a Vorbis encoder" and FFmpegCommandBuilder.kt:188 does. docs/coverage-read-findings.md says in terms that the tempting fix there locks in the wrong answer and that the decision comes first. This is the AAC arm only. 563 -> 564 JVM tests, 0 failures. No production code changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
0f842243b5 |
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) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
2fbc957119 |
A5 (#171): fire the muxer guard that repairs "MP4 for everything", which had never fired
Media3Muxers' KDoc names the defect this guards -- "the router claimed five containers while the engine silently wrote MP4 for all of them" -- and the repair itself was untested: Media3Engine$buildTransformer$3, the requireNotNull message lambda, was four lines and four branches at 0%. Nothing had ever driven a plan whose container Media3 cannot mux, and factoryFor answers null for fourteen of them. Weakening it does not crash. The wrong output is a playable file with the wrong container, which is why a test rather than a bug report is what would catch it. Same harness and the same two disciplines as Media3EngineEmptyCompositionTest, which is the sibling this joins: assert the plan really is the one the test needs before driving the engine, and rule out CancellationException so an unresumed continuation cannot read as a pass. Three premises are asserted here rather than assumed -- that the plan is still WebM by the time the engine sees it, that Media3 really has no muxer for WebM, and that neither track was dropped, since the empty-composition refusal fires earlier and is a different test's subject. The assertion is on the exception type *and* its message, and the ticket predicted why: replacing requireNotNull with `?: DefaultMuxer.Factory()` does not make the export succeed, it lets it run on and fail some other way. Measured -- that mutation fails the type assertion, so the guard is genuinely what this test is holding, and the message assertion stands behind it. 560 -> 561 JVM tests, 0 failures. No production code changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
a645442acc |
A2 + A3 (#168, #169): the hardware fallback, the cancellation that must not take it, and the name a job may not have
runMedia3OrFallBack was eleven lines at 0% and isCancellation had never been called by any JVM test -- ci=0, not merely a missed branch. The seam to reach it has existed the whole time: ConversionDependencies.hardware, which no unit test had ever set. What kept the path cold is that every worker test uses EnginePreference.FORCE_SOFTWARE, which never enters the function, and the probe defaults to UNPARSEABLE, which PERMISSIVE.canDecode refuses -- so even AUTO would have routed straight to FFmpeg for a reason no assertion mentioned. Both are now stated in setUp rather than inherited. Four behaviours, each with the mutation that proves it: hardware failure falls back to software delete the fallback call ... on a *clean* staging file delete staged.delete() before it cancellation is rethrown, not fallen back delete `if (isCancellation(e)) throw e` engine.close() runs either way empty the finally block the display-name fallback (#169) change "input" to anything else All five confirmed red, then restored. The cancellation one is the reason this ticket was first in the group. runMedia3OrFallBack catches Throwable, so without that re-throw a user cancelling a hardware transcode has the app quietly start a *second* conversion in software -- the one thing cancelling is for. ForcedFailureTest covers the failure half on a device and does not cover this half at all. The clean-staging assertion is made where it is observable rather than by reading the file: the software fake records whether the output existed when it was entered, so a missing delete shows up as FFmpeg finding a half-written hardware output at the path it is about to write. 556 -> 560 JVM tests, 0 failures. No production code changed. ConversionWorker: 22 -> 9 missed lines, 14 -> 7 missed branches. Line 2090/2348 -> 2103/2348; branch 1022/1340 -> 1029/1340. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
5761faced6 |
A4 (#170): join the two halves of an unreadable join clip, and record why the catch arm stays device-only
The ticket asked for two things. One of them is not reachable from the JVM, and saying so is most of the value here. **probeForConcat's catch arm cannot be provoked on this runtime.** Robolectric's MediaExtractor never throws from setDataSource -- measured across four input shapes: an unregistered content:// authority, a missing file://, a file of garbage bytes, and an http:// URL. All four returned normally with trackCount = 0. So a failed read arrives as an empty track list rather than as an exception and reaches the same ConcatInput(null, null, 0, 0, 0) by the other road. The catch stays covered only by ConcatEngineTest on a device. The test file says this rather than implying the arm is handled. **What is reachable, and was genuinely missing, is the span.** Both halves were already covered and neither reached the other: MediaProbeTrackWalkTest pins what concatInputFrom makes of a track list, ConcatPlannerTest's `an unknown codec is not treated as a match` pins what the planner does with a hand-built ConcatInput(video = null). The planner's safety rests on the probe really producing that shape, and the hand-built fixture would go on passing if it stopped. Measured rather than claimed: mutating concatInputFrom's initial `video` to a non-null placeholder leaves ConcatPlannerTest green and turns this red. Dropping the planner's video null guard turns both red -- so that half was already held, and this file does not claim credit for it. The coupling itself is worth writing down: ConcatPlanner guards video against a null codec and audio not at all, and that asymmetry is correct rather than an oversight -- MediaProbe.shortName returns a non-null String, so a null audioCodec means the track is absent and two clips with no audio really do match, while a null videoCodec means absent *or* unreadable. The audio check is safe because the video guard fires first on a clip nothing could read. Nothing held that. 555 -> 556 JVM tests, 0 failures. No production code changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
c2c0bfa848 |
A6 (#172): six one-branch outcomes nothing produced, and one that cannot be produced
Each of these is a site where JaCoCo reported mi > 0 -- a concrete instruction no test
runs -- rather than a partial branch on a compound condition, which is how the group was
filtered in the first place. Six closed, one moved to the exclusions.
AndroidDeviceCodecs:35 the UNPARSEABLE sentinel, refused where an unknown name is not
ContainerCapabilities:94 accepts(container, VideoCodec.NONE, mode) -- the audio twin has
had a test since #136; the asymmetry is the argument
ContainerCapabilities:323 repairVideo's keep-the-requested-codec arm
ContainerCapabilities:348 firstContainerHolding's fallback container
OutputPublisher:230 the resolver call that throws -- the third case the KDoc names
and the one the shape list was missing
JobSnapshots:31 a job that recorded no output path at all
Every one was mutated and confirmed red, then restored. Two are worth stating because
they did not go red first time or would not have:
**The firstContainerHolding test was vacuous on its first draft.** It asserted the
refusal still offered *something*, and deleting the fallback left it green: the source
container is a candidate in its own right, so the list stays non-empty and only its
contents change. Rewritten around AVI, which has no mapping for H.265, and asserting the
codec survives -- without the fallback the app silently offers H.264 instead, which is
the actual loss. This is the failure mode CLAUDE.md records from the mutation review, met
head on rather than in the abstract.
**OutputPublisher:230 needed one line.** `a destination whose size cannot be determined is
never deleted` already walked three RowShapes; QUERY_THROWS was the fourth case its own
KDoc names -- "a resolver call that throws" -- and the only one that reaches `?: false`
through runCatching rather than through a cursor answer.
ContainerCapabilities:282's `.filter { it != exclude }` is **not** closed here and is not
a gap: nothing can make it drop anything. On the shared container `repair` always changes
at least one codec, because a codec it left alone is one validate would not have refused;
every other candidate differs by container; and the single call site passing a non-default
exclude (validateVideo:186) excludes a spec carrying VideoCodec.COPY while every repaired
candidate carries NONE. F4-shaped -- recorded rather than covered, and 550 tests agree.
550 -> 555 JVM tests, 0 failures. Five new tests rather than six: OutputPublisher:230
is one line inside a test that already existed. No production code changed.
Line 2087/2348 -> 2090/2348; branch 1011/1340 -> 1022/1340.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
JMR-devandClaude Opus 5
|
016030f3e4 |
A1 (#167): pin all three foreground-service regimes, and the boundary between two of them
`ConversionForegroundType.current()` has three arms and the JVM suite executed one. `robolectric.properties` pins everything to `sdk=36`, and `@Config` appears nowhere in `app/src/test`, so 3 lines and 3 of 4 branches were cold. The instrumented test is not a substitute, and the reason is specific rather than general. `ConversionWorkerTest.foregroundTypeMatchesTheRunningApiLevel` asserts against whichever API the leg is, so it covers one arm per leg and never the other two -- and the legs that would cover 33 and 34 are the ones #122 wedges. From docs/coverage-read-findings.md, an API 33 run reported `received: 60` with `failed: unknown`: the regime was exercised and that leg could not have said so if it had broken. This runs all three deterministically in the same ./gradlew invocation. Four classes, not three. 35 shares its answer with 36 and looks redundant; it is the whole point. Relaxing `>= VANILLA_ICE_CREAM` to `>` is invisible at every level except exactly 35 -- measured, not assumed: that mutation failed ForegroundTypeApi35Test alone, while swapping DATA_SYNC and MEDIA_PROCESSING failed 34, 35 and 36. Without the 35 class the first mutation survives the suite. 546 -> 550 JVM tests, 0 failures. No production code changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Jason Ross
|
c6e9a480e1 |
Merge pull request #165 from JMR-dev/test/screen-wiring
W3: the screen wiring, and a narrower hazard than the ticket claimed |
||
|
Jason Ross
|
d8110d7a62 |
Merge pull request #164 from JMR-dev/test/viewmodel-setters
W4: the seven settings edits, and three tests that did not bite until they did |
||
|
JMR-devandClaude Opus 5
|
6f3966cc69 |
W3 (#156): the screen wiring, and a narrower hazard than the ticket claimed
The stateful outer composables hand `ConverterScreenContent` and `JoinScreenContent` a list of `viewModel::` references. No test in the suite had ever seen that list: the content tests build their own `ConverterActions`, so they drive the stateless inner and never touch the wiring. THE TICKET'S PREMISE WAS HALF WRONG, AND CHECKING BEAT ASSUMING. #156 was filed claiming a transposition of any two of seventeen bindings would survive the suite. Measured instead of trusted: onVideoCodec <-> onAudioCodec -> REJECTED: "Inapplicable candidate(s): fun setAudioCodec(codec: AudioCodec)" onCancel <-> onReset -> COMPILES Every typed binding -- container, both codecs, preset, suggestion, quality, engine preference -- takes a distinct parameter type, so the compiler is already the test. Writing assertions against those transpositions would have been theatre, and this file says so rather than quietly including them. WHAT IS ACTUALLY AT RISK is the `() -> Unit` bindings, which are interchangeable to the compiler: two on the converter screen (onCancel, onReset) and *three* on the join screen (onJoin, onCancel, onReset). A Cancel button that discards the finished file, a Start-over that leaves it on screen, or a Join button that cancels -- each is one wrong word and each ships. I got that wrong in the first check too: an early run reported the onCancel/ onReset swap as rejected, from a grep-and-exit-code test that misread a stale build. Re-running it properly printed BUILD SUCCESSFUL with the swap in place. THE SEAM. `converterActions(viewModel, onPickInput, onConvert, onSave)` and `joinActions(viewModel, onPickInputs, onSave)`. The launcher-backed actions stay parameters -- they need an ActivityResultLauncher, which is the part that genuinely needs a composition, and keeping them out means the rest needs none. Told apart by effect rather than by a recording double: `reset()` sets the state to Idle, `cancel()` with no active job leaves it alone (`activeWorkId?.let`, which SettingsEditsTest pins). Mutations -- every transposition caught, each by two tests: converter onCancel <-> onReset | 2 tests join onJoin <-> onCancel | 2 tests join onReset <-> onCancel | 2 tests a typed binding dropped to {} | 1 test a launcher action rerouted | 1 test The two-test symmetry is deliberate: one direction alone passes against a wiring with BOTH actions bound to the same method, which is what a copy-pasted line produces. Three guard assertions earned their place during writing -- the picks land through an injected dispatcher, and without `ParkedPickDispatcher.runAll()` all three state-based tests sat on Idle and would have asserted nothing. They failed loudly instead of passing quietly. `@UnstableApi` on both builders, per CLAUDE.md; lint caught their absence, as it did in W1. 525 -> 537 tests, 88.0% -> 88.9% line, 70.5% -> 75.4% branch. Gate green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
a507736d3d |
W4 (#157): the seven settings edits, and three tests that did not bite until they did
`setPreset` was covered; the six beside it and `cancel()` had no coverage at all.
That asymmetry is the tell -- they are reachable from the JVM suite by exactly
the route `setPreset` already takes, and nothing had asked.
WHAT IS ASSERTED. Not "the setter sets something". Each of these copies into a
nested `OutputSpec`, so the failure worth catching is a setter that writes the
right value into the wrong field, or that rebuilds the spec and quietly discards
the other two. Every test asserts the field it changed AND that the rest survived.
THREE OF THEM DID NOT BITE, AND THE REASON IS WORTH KEEPING. The first run of the
mutations came back with two green:
setContainer rebuilding from OutputFormat.MP4_H265.spec -> GREEN
setQuality also resetting enginePreference to AUTO -> GREEN
Both for one mistake of mine: I asserted "the rest survived" against values that
were still at their defaults. `ConversionSettings` starts at `MP4_H265.spec`,
`QualityTier.FAST` and `EnginePreference.AUTO` -- so a mutation that RESET a
neighbouring field to its default was indistinguishable from one that left it
alone. The tests were checking a value, not a behaviour.
Fixed by moving each neighbour off its default before the call under test. A
third test had the same latent hazard -- it asserted `quality == FAST` -- and was
corrected with the others rather than left to fail later.
That is precisely the shape CLAUDE.md warns about ("five of them passing the
whole suite over a completely unguarded code path"), and it is the second time in
this wave the mutation pass has earned its place: green was not evidence.
Eight mutations, all red after the fix:
setContainer rebuilds from a preset | 1 test
setQuality resets the engine preference | 1 test
setEnginePreference resets the quality | 2 tests
applySuggestion resets quality | 1 test
setVideoCodec writes nothing | 1 test
setAudioCodec writes nothing | 2 tests
setEnginePreference writes nothing | 2 tests
cancel() dereferences a null activeWorkId | 1 test
516 -> 525 tests, 87.7% -> 88.0% line, 70.4% -> 70.5% branch. Gate green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|