test(api): raise SonarCloud coverage with 153 unit tests + surgical coverage-excludes (#243)

[thomasluizon]

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

PR Review — #398: test(api): raise SonarCloud coverage with 153 unit tests + surgical coverage-excludes

Recommendation: ✅ APPROVE

Severity	Count
Critical (incl. ⚠️ old-client breaks)	0
High	0
Medium	1
Low / Info	2 (surfaced below, non-blocking)

Summary

Pure test-addition PR: 20 new unit-test files (153 tests) across Orbit.Application.Tests and Orbit.Infrastructure.Tests, plus one CI config change (.github/workflows/sonarcloud.yml) expanding sonar.coverage.exclusions with documentation comments. No production src/ code changed. Four parallel verification passes (chat-tool tests vs. tool sources, profile/validator tests vs. handlers/validators, AI-service/AppConfig tests vs. services, and the coverage-exclusion list vs. the actual files) found the new tests to be accurate, non-tautological, and correctly exercising real branches — property names, constructor signatures, error strings, and boundary constants (e.g. MaxChallengeParticipants - 1, MaxRangeDays) all check out against the real implementations. No narration comments, Console.*, or unjustified null! were introduced.

Findings

Critical

None.

High

None.

Medium

StripeBillingService.cs excluded from coverage despite having an injectable seam

• Location: .github/workflows/sonarcloud.yml (new **/Services/StripeBillingService.cs entry in sonar.coverage.exclusions, justified in the comment as a "Stripe SDK wrapper ... sibling of StripeCouponRewardService")
• Issue: Unlike StripeCouponRewardService (which does new CouponService() inline — genuinely no seam), StripeBillingService receives its Stripe clients through an injected StripeServiceClients record (src/Orbit.Infrastructure/Services/StripeBillingService.cs:14-25: CustomerService, SessionService, SubscriptionService, etc. all passed via constructor DI). Stripe.net's Service classes expose their methods as virtual specifically to support this kind of substitution/mocking, so the class is unit-testable the same way the rest of the suite tests DI-seamed services with NSubstitute — it just doesn't have a test yet.
• Risk: Framing a genuinely testable class as "no mockable seam" and coverage-excluding it inflates the SonarCloud metric without inflating real coverage, and sets a precedent for excluding future DI-seamed services instead of testing them — the opposite of what this PR's own goal (raise real coverage) is going for.
• Fix: Either (a) remove StripeBillingService.cs from sonar.coverage.exclusions and add a StripeBillingServiceTests.cs using Substitute.For<StripeServiceClients>()/mocked service methods (mirroring the pattern already used for other DI-seamed Infrastructure services in this same PR), or (b) if there's a concrete reason the virtual-method mocking doesn't work in practice here, correct the exclusion comment to state that reason instead of "no mockable seam."

Low / Info (non-blocking)

• BackgroundJobs/HangfireRecurringJobRegistrar.cs new exclusion takes two interface dependencies (IRecurringJobManager, IEnumerable<IScheduledJob>) and is trivially mockable, but it's consistent with the file's existing precedent of excluding all IHostedService registrations.
• .github/** new exclusion is placed in sonar.coverage.exclusions (still fully Sonar-analyzed for smells) rather than sonar.exclusions (full skip, where the comment's stated "parity with load-tests/**" actually lives) — likely a no-op since there's no C# under .github/, but the stated parity claim doesn't match where the entry was added.

Subagents

Agent	Verdict
security-reviewer	N/A — gate not met, no src/ files changed in this diff
contract-aligner	N/A — gate not met, no DTO/Controller/shared-type change

Validation

Check	Result
Build (dotnet)	N/A — skipped per CI-job instruction; covered by separate required checks
Tests (dotnet)	N/A — skipped per CI-job instruction; covered by separate required checks

Deferred — N/A dimensions & files not verdicted

• Dimensions 8 (DESIGN.md/AI-slop), 9 (Parity), 10 (i18n), 14 (FEATURES.md parity) — N/A, no apps/* or user-facing feature surface touched.
• Dimension 11 (Contract drift/backward-compat) — N/A, no DTO/Zod/endpoint change in the diff.
• Cross-repo dimensions generally — not verifiable in CI (orbit-ui-mobile not checked out); none apply to this diff regardless, since it's test-only + CI config.
• Every changed file (20 test files + 1 workflow file) received an explicit verdict via the four verification passes above.

What's good

• 153 new unit tests close real coverage gaps (chat tools, profile commands, validators, AI-generation services, AppConfig) with meaningful, non-vacuous assertions verified line-by-line against the actual implementations.
• The sonarcloud.yml comment block is unusually disciplined — every exclusion carries a one-line WHY, and the PR correctly removes AppConfigService.cs from the exclusion list now that it has real tests, showing the "surgical" exclusion process is generally working as intended.
• GoogleCalendarEventFetcher.cs was correctly kept out of the new Services/Calendar/** glob (it lives one directory up), so the "stays testable" claim in the comment holds.

Recommendation

Approve as-is. The Medium finding (StripeBillingService exclusion) is a metric-integrity nit, not a merge blocker — fine to land now with a follow-up to either write the test or fix the exclusion's justification.