Skip to content

Close the biggest test gaps: composed resource keys, commands, tool discovery - #4

Merged
Kentarohakase merged 1 commit into
mainfrom
tests/close-coverage-gaps
Aug 12, 2026
Merged

Close the biggest test gaps: composed resource keys, commands, tool discovery#4
Kentarohakase merged 1 commit into
mainfrom
tests/close-coverage-gaps

Conversation

@Kentarohakase

Copy link
Copy Markdown
Owner

Stacked on #3. Phase 3 of the improvement plan: 239 tests, up from 204.

The gap that could ship a bug to users

Several texts are looked up under a key composed from an enum value — ValidationFinding_{kind}_Title, AnalysisFinding_{kind}_Message, BatchStatus_{status}, and six more families. Adding a member to AudioComparisonFindingKind or BatchProcessingStatus produced no compiler error and no test failure; it simply shipped a missing-resource marker into the interface.

ResourceTests.VisibleModelTexts_DoNotContainMissingResourceMarkers walked a hand-maintained list of about sixty values, so it could not cover this. ComposedResourceKeys_ResolveForEveryEnumValue now enumerates every member of every enum used that way and resolves the keys it produces, in both languages.

I verified it actually bites rather than trivially passing: adding a member without a resource entry turned it red and named exactly the two missing keys.

Collection: ["ValidationFinding_TemporaryFalsificationProbe_Titl"..., "ValidationFinding_TemporaryFalsificationProbe_Mess"...]
Failed!  - Failed: 2, Passed: 0

The probe member was removed again.

New coverage

  • CommandTestsAsyncRelayCommand, RelayCommand and RelayCommand<T> had no tests at all, although AsyncRelayCommand carries the guard that keeps a failing command from ending the session (Keep failing commands, slow tools and untrusted update links from breaking the session #2). Covered: reentry is blocked while a run is in flight, the error handler receives the failure, a cancellation is a normal outcome, the command becomes executable again afterwards, and the typed parameter handling including a mismatched type.
  • ResultTests — including the case every consumer has to remember: a failure may still carry a value, which is how a critical validation report reaches the UI.
  • ToolDiscoveryServiceTests — 245 lines that had no tests. Covered: path resolution, the fallback to the bare executable name, the caching of a successful probe, and the caching of a failed one from Make long running steps cancellable and keep the queue from stalling the UI #3. The probe uses where.exe /? — always present on Windows and the same shape as <tool> -version, without needing FFmpeg on the runner. (cmd.exe /? is unusable: it prints help but exits 1.)
  • AppUpdateServiceTestsCheckAsync had no tests despite AppUpdateService having an internal constructor that takes an HttpClient specifically to make it testable. A FakeHttpMessageHandler covers a newer release, an up-to-date version, an untrusted link falling back to the releases page, a 404, a malformed payload, a missing tag, an unreachable host, and cancellation.

One production change

AppUpdateService.CheckAsync caught OperationCanceledException along with everything else, so a caller that cancelled was told there was no update. It now rethrows when the caller's own token was cancelled; its internal timeout still surfaces as "no update".

Latent flakiness removed

LocalizationService.Instance.Culture is process-wide state and its setter also writes CultureInfo.DefaultThreadCurrentUICulture. Six test methods across four classes mutate it while other classes assert on localized text, and xUnit runs test classes in parallel by default. The suite takes nine seconds, so serializing it via [assembly: CollectionBehavior(DisableTestParallelization = true)] is the cheap and complete fix.

Verification

dotnet format .\AudioQualityEnhancer.slnx --verify-no-changes   -> clean
dotnet build  .\AudioQualityEnhancer.slnx -c Release            -> 0 warnings, 0 errors
dotnet test   .\AudioQualityEnhancer.slnx -c Release            -> 239 passed, 0 failed

Still not covered

MainViewModel — about 2,100 lines across eleven partials — remains untested. Its constructor instantiates eighteen services itself and is the de-facto composition root, so there is no seam to construct it in a test. That needs a constructor refactor, which is its own change rather than a by-product of this one.

@Kentarohakase
Kentarohakase force-pushed the hardening/cancellation-and-ui-responsiveness branch from fa1684e to 56e3df3 Compare August 12, 2026 00:56
@Kentarohakase
Kentarohakase force-pushed the tests/close-coverage-gaps branch from e894726 to e6c1686 Compare August 12, 2026 00:56
@Kentarohakase
Kentarohakase force-pushed the hardening/cancellation-and-ui-responsiveness branch from 56e3df3 to b7f755d Compare August 12, 2026 01:01
Several texts are looked up under a key composed from an enum value, so adding
an enum member shipped a missing-resource marker to the user with no compiler
error and no test to stop it. The existing resource test walked a hand written
list of about sixty values and could not catch that. A new test enumerates
every member of every enum used that way and resolves the keys it produces, in
both languages. Verified by adding a member without a resource entry: the test
names exactly the two keys that are missing.

The command types had no tests at all although they carry the guard that keeps
a failing command from ending the session. Reentry, the error handler, the
cancellation path and the typed parameter handling are covered now, as is
Result, whose failure case may legitimately still carry a value.

Tool discovery had no tests either. Path resolution, the fallback to the bare
name, the caching of a successful probe and the new caching of a failed one are
covered. The update check gets its first tests through the constructor that
takes an HttpClient specifically for that purpose and was unused until now; it
also propagates a cancellation instead of reporting it as no update available.

Test classes no longer run in parallel: several change the process wide
localization culture while others assert on localized text.
@Kentarohakase
Kentarohakase changed the base branch from hardening/cancellation-and-ui-responsiveness to main August 12, 2026 01:03
@Kentarohakase
Kentarohakase force-pushed the tests/close-coverage-gaps branch from e6c1686 to 90d55cd Compare August 12, 2026 01:03
@Kentarohakase
Kentarohakase merged commit c857511 into main Aug 12, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant