test: close the reachable coverage gaps and align SonarQube's denominator

[HandyS11]

Why

SonarQube reported 91.7% coverage against a local gate already at ~97%. Most of that delta was not a testing gap.

SonarQube does not take its "lines to cover" denominator from the coverage report — each language analyzer computes executable lines itself, and anything the report is silent about counts as uncovered. Two files were absorbing the difference:

File	Lines	Why it read as 0%
wwwroot/app.js	197	Browser JavaScript. Sonar's JS analyzer indexes it; coverlet, being a .NET tool, cannot cover it at any TFM.
Program.cs	36	SonarC# honours [ExcludeFromCodeCoverage] (hence PairingListener/FcmRegistration at 100%), but does not associate it with top-level statements — they aren't syntactically inside the partial class Program; part carrying the attribute. Coverlet excludes them correctly; the file never appears in the local web gap list.

Both are now in sonar.coverage.exclusions. Neither weakens the local gate — tools/coverage/report.sh never saw either file.

⚠️ app.js is excluded, not fixed. It has no tests. Excluding it stops 400 lines of untested JS from dominating a .NET coverage metric, but the gap is real and is documented as such. Closing it means adding a Node toolchain and sonar.javascript.lcov.reportPaths.

Real coverage added

	before	after
libraries	96.91 / 93.55	97.22–97.45 / 94.22–94.33 (see caveat)
web app	99.26 / 97.09	99.56 / 98.54
CameraController	94.70 / 91.30	100 / 100
ClientAddress	100 / 50	100 / 100
RequestMode	94.87 / 97.22	100 / 100
RustPlusSocket	see caveat below	see caveat below

• FakeRustPlus — an IRustPlus double that can throw where the mock server can only return a failed Response. Non-camera members throw NotSupportedException, so a controller change that starts calling one fails loudly rather than passing against a permissive stub.
• CameraControllerSeamTests (10 tests) — zero/negative renewal intervals, a throwing unsubscribe on dispose, the disconnected-client skip, failed renewals with and without a subscriber, cancellation inside a renewal, the loop-condition exit, frame drop with no handler, empty-bitmask MoveAsync.
• RustPlusSocketBaseTests — a bare subclass reaching Dispose(bool)'s finalizer arm and the default ParseNotification.

Warning

Correction to an earlier version of this description. It claimed RustPlusSocket went 89.17 → 90.68 line. That was one good run, not a reproducible result. Three runs of the same commit gave 89.17%, 89.92% and 90.68% — its teardown and concurrent-dispose arms are covered or not depending on how the integration tests' real WebSocket teardown interleaves. RustPlusSocketBaseTests deterministically adds three lines plus one branch; the rest was noise. The library aggregate moves with it (97.22–97.45%), so the "before/after" rows above should be read per-class, and only for the classes that are stable. testing.md now documents this.

• ClientAddressTests + one RequestModeTests case — addressless-request fallbacks that IsLocal short-circuits past.

RustPlusSocket audit

I checked all 42 uncovered lines rather than inheriting testing.md's claim. It holds: the remainder are catch (ObjectDisposedException) arms around concurrent dispose, close-handshake failures, and Task.WhenAny teardown-bound races — each needs a dispose to land inside a specific window, or a real socket to break mid-close. Only two were cleanly reachable, and both are now covered.

Deliberately not chased

Enumerated in testing.md rather than contrived around, matching the standard the doc already sets for SessionSweeper:

• CredentialsStore.Save's !OperatingSystem.IsWindows() arm — needs a Windows CI leg, not a better test.
• SessionStore's two CAS-contention arms — need a seam that exists only to be tested; the actual cap contract is pinned by SessionStoreCapsTests.
• SessionSweeper — already documented as unreachable.

Verification

tools/coverage/report.sh — full suite, all projects, both TFM hosts. Both gates green (min 95.0/90.0). ReSharper formatter applied, so the pre-push hook is satisfied.

Projected SonarQube: 91.7% → ~97%. That figure is an estimate; the real number lands when the Sonar workflow next runs on develop.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

The new async tests rely on fixed wall-clock delays that can be timing-sensitive and lead to flaky CI failures under contention.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR improves coverage fidelity between the repo’s local Coverlet/ReportGenerator gate and SonarQube’s “lines to cover” calculation by (1) excluding non-.NET or syntactically un-excludable files from Sonar’s coverage denominator, and (2) adding targeted unit tests to cover previously reachable-but-untested branches in the core socket and camera controller logic.

Changes:

• Add new unit tests (including a purpose-built IRustPlus test double) to close reachable coverage gaps in CameraController, ClientAddress, RequestMode, and RustPlusSocket.
• Document why SonarQube’s coverage denominator can diverge from the coverage report and why sonar.coverage.exclusions needs entries beyond samples/tests/tools.
• Update the Sonar workflow to exclude wwwroot assets and apps/**/Program.cs from Sonar coverage calculations.

File summaries

File	Description
tests/RustPlusApi.UnitTests/RustPlusSocketBaseTests.cs	Adds tests covering RustPlusSocket.Dispose(bool) finalizer arm and default ParseNotification no-op.
tests/RustPlusApi.CredentialsWeb.UnitTests/RequestModeTests.cs	Adds a direct assertion for IsLoopbackAddress(null) to cover the documented “no connection address” case.
tests/RustPlusApi.CredentialsWeb.UnitTests/ClientAddressTests.cs	Adds coverage for the "unknown" fallback when RemoteIpAddress is absent.
tests/RustPlusApi.Camera.UnitTests/FakeRustPlus.cs	Introduces an IRustPlus test double enabling throw/fault paths not reachable via the mock server.
tests/RustPlusApi.Camera.UnitTests/CameraControllerSeamTests.cs	Adds seam tests for keep-alive/dispose edge cases and guard rails (incl. empty movement buttons).
docs/development/testing.md	Updates achieved coverage figures and documents SonarQube denominator alignment + accepted residual gaps rationale.
.github/workflows/Sonar.yml	Extends sonar.coverage.exclusions to avoid Sonar counting un-coverable wwwroot JS and top-level Program.cs statements as uncovered.

Review details

Suppressed comments (1)

tests/RustPlusApi.Camera.UnitTests/CameraControllerSeamTests.cs:210

• Using a fixed 200ms delay to "wait for repeated renewals" can be timing-sensitive and lead to flaky failures if the keep-alive loop doesn't get scheduled promptly. Consider waiting (bounded) until SubscribeCount exceeds the threshold instead of assuming a specific wall-clock delay is sufficient.

        // Nothing is attached to OnKeepAliveFailed: reporting the failure must be a no-op rather
        // than a null dereference, and the loop must keep retrying past it.
        await Task.Delay(TimeSpan.FromMilliseconds(200));

        Assert.True(client.SubscribeCount > 2, $"expected repeated renewals, saw {client.SubscribeCount}");

• Files reviewed: 7/7 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[HandyS11]

Correct for these two, fixed in 29f9de7.

Both asserted a renewal count had reached a value inside the window (== 2 at line 142, > 2 at line 210), so a contended runner under-delivers and the test fails. Both now poll via a bounded WaitUntilAsync against the existing 10s Timeout, so contention slows the test rather than failing it.

I did not change the two Task.Delay calls in SubscribeAsync_WithZeroInterval_NeverRenews / WithNegativeInterval_NeverRenews. Those assert the count stayed at 1, so contention can only make the assertion more true — and "wait until the count reaches the expected value" can't express absence. The delay is now a named SettleWindow constant documenting why that direction is contention-safe.

Chasing this did surface a real problem, though: RustPlusSocket's coverage is itself not reproducible. Three runs of the parent commit gave 89.17%, 89.92% and 90.68% line, because its teardown and concurrent-dispose arms are covered or not depending on how the integration tests' WebSocket teardown interleaves. The 89.17 → 90.68 improvement claimed in the PR description was one good run, not a result — the new tests add exactly three lines plus one branch. testing.md now quotes the library figure as a range and warns against reading small movements as regressions.