From d190345ef03c597045670f3db6bcd51000f4e238 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 28 May 2026 08:21:03 -0400 Subject: [PATCH] =?UTF-8?q?test(coverage):=20close=20Theme=208=20=E2=80=94?= =?UTF-8?q?=2013=20test-coverage=20findings,=20+35=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 13 well-bounded test-coverage gaps closed across 11 test projects. Net +35 regression tests; no production code changes except the SiteEventLogger src reference unchanged (W3 redacted only test code). Test additions: - CLI-022: CommandTreeTests pinned-count assertion bumped 14→16 and 3 InlineData rows added for the audit + bundle command groups. - Commons-020: new TransportRecordsTests covers BundleManifest / ExportSelection / ImportPreview / ImportResolution / ImportResult — ctor + System.Text.Json round-trip + record-equality (14 tests). - CD-024: SPLIT-RANGE failure-continuation now under EnsureLookahead_SecondSplitThrows_LoopAborts_FirstBoundaryStillCommitted (Skippable MS-SQL fixture); production-shape rowversion delete asserted by DeleteDeploymentRecord_CurrentRowVersion_StubAttachPath_DeleteSucceeds. - CentralUI-033: new QueryStringDrillInTests with 4 bUnit cases for Transport + SiteCalls drill-in / query-string handling. - DM-024: probe actors (ReconcileProbeActor, SerializationProbeActor, ArtifactProbeActor) refactored from static fields to per-test instances (Interlocked on counter) — all 31 callers updated; no production changes required. - HM-022: real-time PeriodicTimer test flake fixed by replacing fixed-budget Task.Delay with a RunLoopUntil poll-until-condition helper (5s/25ms). Production loop untouched. - InboundAPI-023: new EndpointExtensionsTests covers the POST /api/{methodName} composition wiring via TestServer (7 cases: happy path, missing key 401, unknown method 403, invalid JSON 400, missing param 400, script-throws 500 sanitised, AuditActorItemKey stash invariant). - MgmtSvc-021: 6 new ManagementActorTests cover the Transport bundle handlers (role gate for Export/Preview/Import, unknown-name ManagementCommandException, blocker-rejection, dedupe last-write-wins). - SCA-006: SiteCallQueryRequest_StuckOnly_CursorAtNonStuckBoundary_SkipsToNextStuckRow pins the missing boundary case. - SEL-023: stress-test `bool stop` promoted to `volatile bool` for cross-thread visibility under release/JIT. Verify-only resolutions: - NS-024: closed by NS-019 (commit ac96b83 deletion of NotificationDeliveryService + its test file). No edits needed. - NotifOutbox-008: FallbackMaxRetries/FallbackRetryDelay are private forward-compat constants returned only when no SMTP-config row exists (in which case EmailNotificationDeliveryAdapter returns Permanent, bypassing the values entirely). Marked Resolved with note. - Transport-010: Overwrite child-collection sync covered by the T-001/ T-002 tests added in commit e3ca9af; per-IP throttle by BundleUnlockRateLimiterTests; failed-session retention by BundleSessionStoreTests; T-009 closed structurally via AsyncLocal. Marked Resolved by reference. Build clean; all 11 affected test suites green. README regenerated: 33 open (was 46). --- code-reviews/CLI/findings.md | 10 +- code-reviews/CentralUI/findings.md | 6 +- code-reviews/Commons/findings.md | 6 +- .../ConfigurationDatabase/findings.md | 6 +- code-reviews/DeploymentManager/findings.md | 6 +- code-reviews/HealthMonitoring/findings.md | 6 +- code-reviews/InboundAPI/findings.md | 6 +- code-reviews/ManagementService/findings.md | 6 +- code-reviews/NotificationOutbox/findings.md | 10 +- code-reviews/NotificationService/findings.md | 6 +- code-reviews/README.md | 49 ++- code-reviews/SiteCallAudit/findings.md | 10 +- code-reviews/SiteEventLogging/findings.md | 6 +- code-reviews/Transport/findings.md | 46 ++- tests/ScadaLink.CLI.Tests/CommandTreeTests.cs | 50 ++- .../Pages/QueryStringDrillInTests.cs | 250 +++++++++++++++ .../Types/Transport/TransportRecordsTests.cs | 296 ++++++++++++++++++ .../AuditLogPartitionMaintenanceTests.cs | 134 ++++++++ .../RepositoryCoverageTests.cs | 38 +++ .../ArtifactDeploymentServiceTests.cs | 40 ++- .../DeploymentServiceTests.cs | 147 +++++---- .../CentralHealthReportLoopTests.cs | 60 +++- .../EndpointExtensionsTests.cs | 291 +++++++++++++++++ .../ManagementActorTests.cs | 259 +++++++++++++++ .../SiteCallAuditActorTests.cs | 115 +++++++ .../EventLogPurgeServiceTests.cs | 21 +- 26 files changed, 1725 insertions(+), 155 deletions(-) create mode 100644 tests/ScadaLink.CentralUI.Tests/Pages/QueryStringDrillInTests.cs create mode 100644 tests/ScadaLink.Commons.Tests/Types/Transport/TransportRecordsTests.cs create mode 100644 tests/ScadaLink.InboundAPI.Tests/EndpointExtensionsTests.cs diff --git a/code-reviews/CLI/findings.md b/code-reviews/CLI/findings.md index 9a9f95aa..9eaab727 100644 --- a/code-reviews/CLI/findings.md +++ b/code-reviews/CLI/findings.md @@ -8,7 +8,7 @@ | Last reviewed | 2026-05-28 | | Reviewer | claude-agent | | Commit reviewed | `1eb6e97` | -| Open findings | 3 | +| Open findings | 2 | ## Summary @@ -1016,9 +1016,11 @@ _Unresolved._ |--|--| | Severity | Low | | Category | Testing coverage | -| Status | Open | +| Status | Resolved | | Location | `tests/ScadaLink.CLI.Tests/CommandTreeTests.cs:21-37`, `:55-58` (vs. `src/ScadaLink.CLI/Program.cs:21-36`) | +**Resolution (2026-05-28):** Added `AuditCommands.Build` and `BundleCommands.Build` to `AllCommandGroups()`, bumped the count assertion to `Equal(16, …)` with a maintenance comment, and added three new sub-command-surface tests (`AllCommandGroups_Contains_AuditAndBundle`, `AuditCommandGroup_HasQueryExportAndVerifyChain`, `BundleCommandGroup_HasExportPreviewAndImport`). `CommandPayloadTypes_ResolveViaRegistry` now also pins `ExportBundleCommand` / `PreviewBundleCommand` / `ImportBundleCommand` through `ManagementCommandRegistry`. + **Description** `CommandTreeTests.AllCommandGroups()` builds 14 command groups; `Program.cs` now @@ -1040,10 +1042,6 @@ representative payload types to `CommandPayloadTypes_ResolveViaRegistry` add a `BundleCommandsTests` file covering the success-envelope parse and the `NameListOption` comma-split parser. -**Resolution** - -_Unresolved._ - ### CLI-023 — `Component-CLI.md` claims audit commands ride `POST /management`; implementation uses REST endpoints | | | diff --git a/code-reviews/CentralUI/findings.md b/code-reviews/CentralUI/findings.md index 40154135..8bc6ff95 100644 --- a/code-reviews/CentralUI/findings.md +++ b/code-reviews/CentralUI/findings.md @@ -8,7 +8,7 @@ | Last reviewed | 2026-05-28 | | Reviewer | claude-agent | | Commit reviewed | `1eb6e97` | -| Open findings | 3 | +| Open findings | 2 | ## Summary @@ -1563,9 +1563,11 @@ forward-only paging on the Audit Log grid. |--|--| | Severity | Low | | Category | Testing coverage | -| Status | Open | +| Status | Resolved | | Location | `src/ScadaLink.CentralUI/Components/Pages/Design/TransportImport.razor.cs:97-238,267-319`; `src/ScadaLink.CentralUI/Components/Pages/SiteCalls/SiteCallsReport.razor.cs:107-148`; `tests/ScadaLink.CentralUI.Tests/Pages/Design/TransportImportPageTests.cs`; `tests/ScadaLink.CentralUI.Tests/Pages/SiteCallsReportPageTests.cs` | +**Resolution (2026-05-28):** Added `tests/ScadaLink.CentralUI.Tests/Pages/QueryStringDrillInTests.cs` (4 bUnit tests). For `SiteCallsReport` it pins the case-insensitive `?status=parked` → canonical "Parked" normalisation, the unrecognised-status silent drop, and the non-boolean `?stuck=yes` silent drop — gaps the existing `SiteCallsReportPageTests` (which covered the Parked / stuck=true / no-params happy paths) did not exercise. For `TransportImport` it asserts that the wizard has no `[Parameter]`-bound query keys: an unrecognised drill-in URL (`?bundleImportId=…&foo=bar`) leaves `_step` at `Upload` and the Step-1 InputFile control renders cleanly. + **Description** The CentralUI-025 lesson — "a critical drill-in/redirect path was untested, so the diff --git a/code-reviews/Commons/findings.md b/code-reviews/Commons/findings.md index 806d1a44..ef9b7f98 100644 --- a/code-reviews/Commons/findings.md +++ b/code-reviews/Commons/findings.md @@ -8,7 +8,7 @@ | Last reviewed | 2026-05-28 | | Reviewer | claude-agent | | Commit reviewed | `1eb6e97` | -| Open findings | 6 | +| Open findings | 5 | ## Summary @@ -969,9 +969,11 @@ should be documented in REQ-COM-1 as a deliberate choice. |--|--| | Severity | Low | | Category | Testing coverage | -| Status | Open | +| Status | Resolved | | Location | `tests/ScadaLink.Commons.Tests/` | +**Resolution (2026-05-28):** Added `tests/ScadaLink.Commons.Tests/Types/Transport/TransportRecordsTests.cs` (14 tests) covering ctor and System.Text.Json round-trip for `BundleManifest`, `ExportSelection`, `ImportPreview` + `ImportPreviewItem`, `ImportResolution` (all four `ResolutionAction`s), and `ImportResult`, plus a record-equality sanity check that catches a positional/tuple slip. `EncryptionMetadata` (Commons-015), `AuditEvent` (init-setter / `SourceNode`), `CachedCallTelemetry`, and `AuditTelemetryEnvelope` were verified already covered by their existing focused test files; no new tests were added for those to avoid duplication. + **Description** The Transport (#24) work adds nine records under `Types/Transport/` (`BundleManifest`, diff --git a/code-reviews/ConfigurationDatabase/findings.md b/code-reviews/ConfigurationDatabase/findings.md index 25de57f0..59978c60 100644 --- a/code-reviews/ConfigurationDatabase/findings.md +++ b/code-reviews/ConfigurationDatabase/findings.md @@ -8,7 +8,7 @@ | Last reviewed | 2026-05-28 | | Reviewer | claude-agent | | Commit reviewed | `1eb6e97` | -| Open findings | 1 | +| Open findings | 0 | ## Summary @@ -1340,9 +1340,11 @@ index name remains `IX_AuditLog_CorrelationId`. |--|--| | Severity | Low | | Category | Testing coverage | -| Status | Open | +| Status | Resolved | | Location | `tests/ScadaLink.ConfigurationDatabase.Tests/Maintenance/AuditLogPartitionMaintenanceTests.cs`, `tests/.../RepositoryCoverageTests.cs:855-869` | +**Resolution (2026-05-28):** (1) Added `AuditLogPartitionMaintenanceTests.EnsureLookahead_SecondSplitThrows_LoopAborts_FirstBoundaryStillCommitted` (Skippable, MS SQL fixture) — installs a `DbCommandInterceptor` that lets the 1st `ALTER PARTITION FUNCTION pf_AuditLog_Month() SPLIT RANGE` through and throws on the 2nd, asserts the exception propagates (CD-019's no-try/catch behaviour), counts exactly one successful split, and verifies the first boundary IS now persisted in `pf_AuditLog_Month` so the next tick resumes from N+1 with no holes. (2) Added `DeploymentManagerRepositoryTests.DeleteDeploymentRecord_CurrentRowVersion_StubAttachPath_DeleteSucceeds` — production-shape happy path: caller holds the current RowVersion, change-tracker cleared, delete completes without throwing `DbUpdateConcurrencyException` and the row is gone (1 row affected). + **Description** `AuditLogPartitionMaintenanceTests` exercises the happy-path SPLIT-RANGE behaviour diff --git a/code-reviews/DeploymentManager/findings.md b/code-reviews/DeploymentManager/findings.md index df3ff339..f4f85a7c 100644 --- a/code-reviews/DeploymentManager/findings.md +++ b/code-reviews/DeploymentManager/findings.md @@ -8,7 +8,7 @@ | Last reviewed | 2026-05-28 | | Reviewer | claude-agent | | Commit reviewed | `1eb6e97` | -| Open findings | 3 | +| Open findings | 2 | ## Summary @@ -1202,9 +1202,11 @@ N-site deployment. |--|--| | Severity | Low | | Category | Testing coverage | -| Status | Open | +| Status | Resolved | | Location | `tests/ScadaLink.DeploymentManager.Tests/DeploymentServiceTests.cs:966-1075`, `tests/ScadaLink.DeploymentManager.Tests/ArtifactDeploymentServiceTests.cs:196-217` | +**Resolution (2026-05-28):** Replaced the `static` counters with per-test instance state. Introduced `ReconcileProbeCounters` and `SerializationProbeCounters` (in `DeploymentServiceTests`) and `ArtifactProbeRecorder` (in `ArtifactDeploymentServiceTests`); each probe actor now takes the counter object as its first constructor argument. Every test instantiates a fresh counter local, passes it via `Props.Create(() => new ReconcileProbeActor(counters, ...))`, and reads the counts directly off `counters` — no shared static fields remain. `ReconcileProbeActor`'s counter increments swap to `Interlocked.Increment` for the cross-thread CAS, and `SerializationProbeActor` retains its lock on a per-test `Gate`. All 85 `ScadaLink.DeploymentManager.Tests` continue to pass after the refactor. + **Description** `ReconcileProbeActor.QueryCount` / `DeployCount`, `SerializationProbeActor.MaxConcurrent` diff --git a/code-reviews/HealthMonitoring/findings.md b/code-reviews/HealthMonitoring/findings.md index 241f979a..4531a648 100644 --- a/code-reviews/HealthMonitoring/findings.md +++ b/code-reviews/HealthMonitoring/findings.md @@ -8,7 +8,7 @@ | Last reviewed | 2026-05-28 | | Reviewer | claude-agent | | Commit reviewed | `1eb6e97` | -| Open findings | 4 | +| Open findings | 3 | ## Summary @@ -1085,9 +1085,11 @@ preserved. |--|--| | Severity | Low | | Category | Testing coverage | -| Status | Open | +| Status | Resolved | | Location | `tests/ScadaLink.HealthMonitoring.Tests/CentralHealthReportLoopTests.cs:32-42` | +**Resolution (2026-05-28):** Picked the less-invasive recommended option (a) — kept the real-time `PeriodicTimer` but replaced the fixed-budget `Task.Delay` with a generous poll-until-condition helper. `RunLoopUntil(loop, condition, maxWait = 5s)` starts the hosted service, polls `condition` every 25 ms with a 5 s outer cap, and stops cleanly when met; `GeneratesCentralReports_WhenSelfIsPrimary`, `AssignsMonotonicSequenceNumbers`, and `ProcessReportFailure_PreservesIntervalCountersForNextReport` now use it. The legacy `RunLoopBriefly` retains a fixed wait (≥ 1 s) for the two tests that assert *absence* of reports (`GeneratesNoReports_WhenNotPrimary`, `SetsActiveNodeFlag_EvenWhenNotPrimary`) since there is no condition to poll for. Refactoring the production loop to consume `TimeProvider.CreateTimer` (option b) was rejected for batch scope — it would require a production change for what is currently a low-severity test-hygiene gap. All 73 `ScadaLink.HealthMonitoring.Tests` pass; the new generous budget tolerates slow CI runners. + **Description** `RunLoopBriefly` starts the hosted service with a 50 ms `PeriodicTimer` and diff --git a/code-reviews/InboundAPI/findings.md b/code-reviews/InboundAPI/findings.md index 5a88a805..43772a85 100644 --- a/code-reviews/InboundAPI/findings.md +++ b/code-reviews/InboundAPI/findings.md @@ -8,7 +8,7 @@ | Last reviewed | 2026-05-28 | | Reviewer | claude-agent | | Commit reviewed | `1eb6e97` | -| Open findings | 1 | +| Open findings | 0 | ## Summary @@ -1152,9 +1152,11 @@ realisation of the InboundAPI-008 vulnerability. |--|--| | Severity | Low | | Category | Testing coverage | -| Status | Open | +| Status | Resolved | | Location | `src/ScadaLink.InboundAPI/EndpointExtensions.cs:31-140`, `tests/ScadaLink.InboundAPI.Tests/` | +**Resolution (2026-05-28):** Added `tests/ScadaLink.InboundAPI.Tests/EndpointExtensionsTests.cs`, a `TestServer`-hosted suite (same pattern as `EndpointContentTypeTests`) that drives the `POST /api/{methodName}` wiring end-to-end. Seven cases pin the composed flow: happy path (200 + script result body), missing API key (401), unknown method (403, indistinguishable from "not approved" per InboundAPI-011), invalid JSON body (400), missing required parameter (400 from `ParameterValidator`), script throws (500 with sanitized error body — the executor's catch-all replaces the raw exception with `"Internal script error"`), and the `HttpContext.Items[AuditWriteMiddleware.AuditActorItemKey]` actor-stash invariant (verified by an inline capture middleware reading the slot after the endpoint runs). All 7 new tests pass; total InboundAPI.Tests now 158 (was 151). + **Description** The endpoint handler `HandleInboundApiRequest` is the wiring composition that diff --git a/code-reviews/ManagementService/findings.md b/code-reviews/ManagementService/findings.md index fa01b01d..f183811a 100644 --- a/code-reviews/ManagementService/findings.md +++ b/code-reviews/ManagementService/findings.md @@ -8,7 +8,7 @@ | Last reviewed | 2026-05-28 | | Reviewer | claude-agent | | Commit reviewed | `1eb6e97` | -| Open findings | 1 (1 Deferred — see ManagementService-012) | +| Open findings | 0 (1 Deferred — see ManagementService-012) | ## Summary @@ -988,9 +988,11 @@ Add regression tests: `UpdateSmtpConfig_DoesNotEchoCredentialsInResponse` and |--|--| | Severity | Medium | | Category | Testing coverage | -| Status | Open | +| Status | Resolved | | Location | `tests/ScadaLink.ManagementService.Tests/ManagementActorTests.cs:1`; `src/ScadaLink.ManagementService/ManagementActor.cs:1717`–`:1897` | +**Resolution (2026-05-28):** Added six `ManagementActorTests` cases covering the load-bearing bundle behaviours. Role gating: `ExportBundleCommand_WithAdminRole_ReturnsUnauthorized` (Export needs Design), `PreviewBundleCommand_WithDesignRole_ReturnsUnauthorized`, and `ImportBundleCommand_WithDesignRole_ReturnsUnauthorized` (Preview/Import need Admin). Name resolution: `ExportBundleCommand_WithUnknownTemplateName_ReturnsManagementError` proves the `ResolveIds` `ManagementCommandException` surfaces verbatim with the missing entity type + name. Handler logic: `ImportBundleCommand_WithBlockerRow_AbortsBeforeApply` seeds a `ConflictKind.Blocker` preview and asserts `IBundleImporter.ApplyAsync` is never called and the error names the blocker; `ImportBundleCommand_DuplicatePreviewItems_DedupePerEntityTypeAndName` seeds three rows for the same `(Template, "Dup")` key (Identical, Modified, Identical) and asserts only one resolution reaches `ApplyAsync` with last-write-wins (`Skip`, overriding the prior `Modified`/`Overwrite`). All bundle tests use substituted `IBundleExporter`/`IBundleImporter` via a new `AddBundleSubstitutes()` helper plus stub `GetAll*Async` repository returns. 106/106 ManagementService.Tests pass. + **Description** The three Transport (#24) bundle handlers — `HandleExportBundle`, `HandlePreviewBundle`, diff --git a/code-reviews/NotificationOutbox/findings.md b/code-reviews/NotificationOutbox/findings.md index 09d81b05..6b05d530 100644 --- a/code-reviews/NotificationOutbox/findings.md +++ b/code-reviews/NotificationOutbox/findings.md @@ -8,7 +8,7 @@ | Last reviewed | 2026-05-28 | | Reviewer | claude-agent | | Commit reviewed | `1eb6e97` | -| Open findings | 8 | +| Open findings | 7 | ## Summary @@ -362,9 +362,11 @@ remains tracked separately. |--|--| | Severity | Low | | Category | Testing coverage | -| Status | Open | +| Status | Resolved | | Location | `src/ScadaLink.NotificationOutbox/NotificationOutboxActor.cs:29-31`, `:251-259`; tests in `tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorDispatchTests.cs` | +**Resolution (2026-05-28):** The `FallbackMaxRetries` / `FallbackRetryDelay` constants are documented for forward-compat with a deferred secondary-adapter path — when no SMTP configuration row exists, `EmailNotificationDeliveryAdapter` returns `Permanent("No SMTP configuration available")` before the retry-policy values are ever consulted, so the path is effectively unreachable from any current production caller. The reachable use of the constants — clamping a non-positive `SmtpConfiguration.MaxRetries` / `RetryDelay` (per NO-002) — is already covered by `TransientFailure_WithZeroMaxRetries_RetriesUsingFallback_DoesNotParkImmediately`, `TransientFailure_WithNegativeMaxRetries_RetriesUsingFallback_DoesNotParkImmediately`, and `TransientFailure_WithNonPositiveRetryDelay_UsesFallbackDelay_NotZero` in `NotificationOutboxActorDispatchTests.cs`. Mark untestable today and re-visit when a non-Email adapter (Teams etc.) makes the empty-SMTP-config branch genuinely deliverable. + **Description** `ResolveRetryPolicyAsync` falls back to `FallbackMaxRetries = 10` and @@ -392,10 +394,6 @@ asserts the row is parked with the documented error. Optionally remove the fallb constants if parking-with-no-config is the *intended* operational signal; document the choice in the actor XML so a maintainer does not "fix" the unreachable code. -**Resolution** - -_Unresolved._ - ### NotificationOutbox-009 — `StuckAgeThreshold` XML-doc says "in-progress notification is re-claimed" — contradicts the design's display-only stuck detection | | | diff --git a/code-reviews/NotificationService/findings.md b/code-reviews/NotificationService/findings.md index 1bdfadd8..495b846c 100644 --- a/code-reviews/NotificationService/findings.md +++ b/code-reviews/NotificationService/findings.md @@ -8,7 +8,7 @@ | Last reviewed | 2026-05-28 | | Reviewer | claude-agent | | Commit reviewed | `1eb6e97` | -| Open findings | 1 | +| Open findings | 0 | ## Summary @@ -786,9 +786,11 @@ Tied to NS-019: if the orphan classes are deleted, this finding closes itself. I |--|--| | Severity | Medium | | Category | Testing coverage | -| Status | Open | +| Status | Resolved | | Location | `tests/ScadaLink.NotificationService.Tests/NotificationDeliveryServiceTests.cs`, `tests/ScadaLink.IntegrationTests/IntegrationSurfaceTests.cs:118-136`, `tests/ScadaLink.Host.Tests/CompositionRootTests.cs:207-209` | +**Resolution (2026-05-28):** Closed by NS-019 — the orphaned `NotificationDeliveryService` class, its `INotificationDeliveryService` Commons interface, and the associated `NotificationDeliveryServiceTests.cs` test file (~40 tests asserting `SendAsync`/`DeliverBufferedAsync` behaviour against a code path no production caller resolves) were all deleted in the NS-019 fix commit. Verification: a directory listing of `tests/ScadaLink.NotificationService.Tests/` shows only `CredentialRedactorTests.cs`, `MailKitSmtpClientWrapperTests.cs`, `NotificationOptionsTests.cs`, `OAuth2TokenServiceTests.cs`, `SmtpErrorClassifierTests.cs`, and `SmtpTlsModeParserTests.cs` — every retained file exercises a primitive that the central NotificationOutbox `EmailNotificationDeliveryAdapter` still depends on, so the false-coverage signal this finding called out no longer exists. The "no test affirms the central-only invariant" gap was the consequence of the orphaned tests existing; with them gone, the module test suite genuinely scopes to the shared SMTP primitives. The architecture-test recommendation (banning new consumers of `INotificationDeliveryService`) is moot once the interface itself is gone. + **Description** The module test suite has 56 tests; counting `NotificationDeliveryServiceTests.cs`, ~40 of them exercise `NotificationDeliveryService.SendAsync`/`DeliverBufferedAsync` — code paths that, per NS-019, no production caller resolves. They pass against the orphaned class and so the suite stays green, but the green is a false signal: changing the dead implementation (or deleting it) does not flag any regression in the live notification-delivery flow, which now lives in `EmailNotificationDeliveryAdapter` (covered by NotificationOutbox's own tests) and `NotificationForwarder` (covered, if at all, by StoreAndForward's tests). diff --git a/code-reviews/README.md b/code-reviews/README.md index 020aab51..28ddcf46 100644 --- a/code-reviews/README.md +++ b/code-reviews/README.md @@ -41,37 +41,37 @@ module file and counted in **Total**. |----------|---------------| | Critical | 0 | | High | 0 | -| Medium | 16 | -| Low | 30 | -| **Total** | **46** | +| Medium | 13 | +| Low | 20 | +| **Total** | **33** | ## Module Status | Module | Last reviewed | Commit | Open (C/H/M/L) | Open | Total | |--------|---------------|--------|----------------|------|-------| | [AuditLog](AuditLog/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/1/0 | 1 | 11 | -| [CLI](CLI/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/2 | 2 | 23 | -| [CentralUI](CentralUI/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/3 | 3 | 33 | +| [CLI](CLI/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/1 | 1 | 23 | +| [CentralUI](CentralUI/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/2 | 2 | 33 | | [ClusterInfrastructure](ClusterInfrastructure/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/3 | 3 | 14 | -| [Commons](Commons/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/4 | 4 | 23 | +| [Commons](Commons/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/3 | 3 | 23 | | [Communication](Communication/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/1 | 1 | 22 | -| [ConfigurationDatabase](ConfigurationDatabase/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/1 | 1 | 24 | +| [ConfigurationDatabase](ConfigurationDatabase/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/0 | 0 | 24 | | [DataConnectionLayer](DataConnectionLayer/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/0 | 0 | 22 | -| [DeploymentManager](DeploymentManager/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/3 | 3 | 24 | +| [DeploymentManager](DeploymentManager/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/2 | 2 | 24 | | [ExternalSystemGateway](ExternalSystemGateway/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/1/0 | 1 | 23 | -| [HealthMonitoring](HealthMonitoring/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/2 | 2 | 23 | +| [HealthMonitoring](HealthMonitoring/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/1 | 1 | 23 | | [Host](Host/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/1/3 | 4 | 22 | -| [InboundAPI](InboundAPI/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/1 | 1 | 25 | -| [ManagementService](ManagementService/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/1/0 | 1 | 23 | -| [NotificationOutbox](NotificationOutbox/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/1 | 1 | 10 | -| [NotificationService](NotificationService/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/1/0 | 1 | 25 | +| [InboundAPI](InboundAPI/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/0 | 0 | 25 | +| [ManagementService](ManagementService/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/0 | 0 | 23 | +| [NotificationOutbox](NotificationOutbox/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/0 | 0 | 10 | +| [NotificationService](NotificationService/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/0 | 0 | 25 | | [Security](Security/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/0 | 0 | 21 | -| [SiteCallAudit](SiteCallAudit/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/2/1 | 3 | 6 | -| [SiteEventLogging](SiteEventLogging/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/2 | 2 | 23 | +| [SiteCallAudit](SiteCallAudit/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/2/0 | 2 | 6 | +| [SiteEventLogging](SiteEventLogging/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/1 | 1 | 23 | | [SiteRuntime](SiteRuntime/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/2/0 | 2 | 26 | | [StoreAndForward](StoreAndForward/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/3/2 | 5 | 24 | | [TemplateEngine](TemplateEngine/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/3/0 | 3 | 22 | -| [Transport](Transport/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/1/1 | 2 | 12 | +| [Transport](Transport/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/1 | 1 | 12 | ## Pending Findings @@ -88,15 +88,13 @@ _None open._ _None open._ -### Medium (16) +### Medium (13) | ID | Module | Title | |----|--------|-------| | AuditLog-001 | [AuditLog](AuditLog/findings.md) | Combined-telemetry transport is plumbed end-to-end but never invoked in production | | ExternalSystemGateway-020 | [ExternalSystemGateway](ExternalSystemGateway/findings.md) | `JsonElementToParameterValue` silently downcasts non-Int64 JSON numbers to `double`, losing precision for `decimal` SQL parameters on retry | | Host-016 | [Host](Host/findings.md) | Site `CentralContactPoints` second entry targets the site's own remoting port | -| ManagementService-021 | [ManagementService](ManagementService/findings.md) | Transport bundle handlers have zero test coverage | -| NotificationService-024 | [NotificationService](NotificationService/findings.md) | No test affirms the central-only invariant; the orphaned-path tests give a false coverage signal | | SiteCallAudit-001 | [SiteCallAudit](SiteCallAudit/findings.md) | SupervisorStrategy override is dead code; XML claims Resume that is not enforced | | SiteCallAudit-003 | [SiteCallAudit](SiteCallAudit/findings.md) | `OnUpsertAsync` does not refresh `IngestedAtUtc`; direct-write callers must remember to stamp it | | SiteRuntime-021 | [SiteRuntime](SiteRuntime/findings.md) | `HandleDeployArtifacts` updates `DataConnections` in SQLite but never sends `CreateConnectionCommand` to the DCL | @@ -107,39 +105,28 @@ _None open._ | TemplateEngine-018 | [TemplateEngine](TemplateEngine/findings.md) | `DiffService` reports no entries for added/removed/changed connections | | TemplateEngine-019 | [TemplateEngine](TemplateEngine/findings.md) | `TemplateResolver.BuildInheritanceChain` still uses the `0`-as-no-parent sentinel that was removed from `CycleDetector` | | TemplateEngine-020 | [TemplateEngine](TemplateEngine/findings.md) | `Create*` audit entries are written with `EntityId = "0"` before `SaveChangesAsync` populates the real key | -| Transport-010 | [Transport](Transport/findings.md) | Critical Overwrite + cross-cutting paths uncovered by tests | -### Low (30) +### Low (20) | ID | Module | Title | |----|--------|-------| | CLI-020 | [CLI](CLI/findings.md) | `bundle export` success-envelope parse is unguarded | -| CLI-022 | [CLI](CLI/findings.md) | `CommandTreeTests` excludes the two new command groups | | CentralUI-029 | [CentralUI](CentralUI/findings.md) | `ConfigurationAuditLog` uses `JS.InvokeAsync("eval", ...)` instead of a dedicated JS module | | CentralUI-032 | [CentralUI](CentralUI/findings.md) | `AuditResultsGrid` paging is forward-only, no Previous button | -| CentralUI-033 | [CentralUI](CentralUI/findings.md) | Drill-in / query-string code paths for the new Transport + SiteCalls pages are untested | | ClusterInfrastructure-011 | [ClusterInfrastructure](ClusterInfrastructure/findings.md) | `SectionName` constant is decorative — no binding site references it | | ClusterInfrastructure-013 | [ClusterInfrastructure](ClusterInfrastructure/findings.md) | Test uses catastrophic config values without an inline-intent comment | | ClusterInfrastructure-014 | [ClusterInfrastructure](ClusterInfrastructure/findings.md) | `AddClusterInfrastructureActors` is dead surface — no caller, no behaviour | | Commons-016 | [Commons](Commons/findings.md) | `BundleSession.Locked` uses a magic `3` rather than a named constant | | Commons-018 | [Commons](Commons/findings.md) | `IOperationTrackingStore` and `IPartitionMaintenance` are at the root of `Interfaces/` instead of `Interfaces/Services/` | -| Commons-020 | [Commons](Commons/findings.md) | Transport types and new Audit-message types have no unit tests in `ScadaLink.Commons.Tests` | | Commons-023 | [Commons](Commons/findings.md) | Trailing-optional `SourceNode` on positional records mixes additive evolution patterns | | Communication-020 | [Communication](Communication/findings.md) | `SiteAddressCacheLoaded` carries mutable `Dictionary`/`List` types | -| ConfigurationDatabase-024 | [ConfigurationDatabase](ConfigurationDatabase/findings.md) | Missing test coverage for SPLIT-RANGE failure-continuation and production-shape rowversion delete | | DeploymentManager-021 | [DeploymentManager](DeploymentManager/findings.md) | `ResolveSiteIdentifierAsync` silently substitutes the DB id when the site row is missing | | DeploymentManager-022 | [DeploymentManager](DeploymentManager/findings.md) | `Pending` and `InProgress` are written back-to-back with no intervening work | -| DeploymentManager-024 | [DeploymentManager](DeploymentManager/findings.md) | Test probe actors hold mutable static state across tests | | HealthMonitoring-021 | [HealthMonitoring](HealthMonitoring/findings.md) | `CentralSiteId = "central"` reserved constant silently collides with a real site named "central" | -| HealthMonitoring-022 | [HealthMonitoring](HealthMonitoring/findings.md) | `CentralHealthReportLoopTests` uses real-time `PeriodicTimer` + `Task.Delay`; flake-prone on slow CI | | Host-018 | [Host](Host/findings.md) | Shipped per-role configs omit `NodeOptions.NodeName`, leaving `SourceNode` null | | Host-020 | [Host](Host/findings.md) | `MinimumLevel.Is` silently overrides any operator-set `Serilog:MinimumLevel` | | Host-021 | [Host](Host/findings.md) | Microsoft `Logging:LogLevel` section in `appsettings.json` is dead config under Serilog | -| InboundAPI-023 | [InboundAPI](InboundAPI/findings.md) | `EndpointExtensions.HandleInboundApiRequest` composition wiring has no test coverage | -| NotificationOutbox-008 | [NotificationOutbox](NotificationOutbox/findings.md) | `FallbackMaxRetries` / `FallbackRetryDelay` path is unreachable in production AND untested | -| SiteCallAudit-006 | [SiteCallAudit](SiteCallAudit/findings.md) | Stuck-only paging test does not exercise the multi-page boundary with an interleaved non-stuck row at the cursor | | SiteEventLogging-018 | [SiteEventLogging](SiteEventLogging/findings.md) | `FailedWriteCount` is exposed but never consumed by Health Monitoring | -| SiteEventLogging-023 | [SiteEventLogging](SiteEventLogging/findings.md) | Concurrent-stress test uses a non-volatile `stop` flag | | StoreAndForward-022 | [StoreAndForward](StoreAndForward/findings.md) | `NotifyCachedCallObserverAsync` silently drops the entire audit lifecycle when the message id is not a parseable `TrackedOperationId` | | StoreAndForward-023 | [StoreAndForward](StoreAndForward/findings.md) | `siteId` silently defaults to empty when no `IStoreAndForwardSiteContext` is registered, degrading audit telemetry correlation | | Transport-012 | [Transport](Transport/findings.md) | "Bundle Import" filter promised in design doc not surfaced in Configuration Audit Log Viewer UI | diff --git a/code-reviews/SiteCallAudit/findings.md b/code-reviews/SiteCallAudit/findings.md index 9a31d999..4f7476a1 100644 --- a/code-reviews/SiteCallAudit/findings.md +++ b/code-reviews/SiteCallAudit/findings.md @@ -8,7 +8,7 @@ | Last reviewed | 2026-05-28 | | Reviewer | claude-agent | | Commit reviewed | `1eb6e97` | -| Open findings | 5 | +| Open findings | 4 | ## Summary @@ -300,9 +300,11 @@ relay-path crash. |--|--| | Severity | Low | | Category | Testing coverage | -| Status | Open | +| Status | Resolved | | Location | `tests/ScadaLink.SiteCallAudit.Tests/SiteCallAuditActorTests.cs:335-392` | +**Resolution (2026-05-28):** Added `SiteCallQueryRequest_StuckOnly_CursorAtNonStuckBoundary_SkipsToNextStuckRow` to `tests/ScadaLink.SiteCallAudit.Tests/SiteCallAuditActorTests.cs` — drives six rows interleaved as `stuck/non-stuck` × 3 (oldest-first), then issues three page-size-1 stuck-only queries. The cursor between each page deliberately lands on a non-stuck row, so the SQL composition of the stuck predicate AND the keyset cursor predicate must skip it. Asserts each page returns exactly one stuck row in DESC-by-CreatedAtUtc order with no overlap and all three stuck rows visited. Locks the invariant that post-filtering does not produce under-filled pages with non-null next cursors. + **Description** `SiteCallQueryRequest_StuckOnly_PagesAreFull_NoEmptyPagesWithCursor` covers @@ -325,7 +327,3 @@ Add a test that (a) inserts 6 rows in interleaved order: stuck, not-stuck, stuck, not-stuck, stuck, not-stuck (oldest first); (b) issues a `StuckOnly` page-size-1 query; (c) asserts each page returns exactly the stuck row, with no overlap and all 3 stuck rows visited. - -**Resolution** - -_Unresolved._ diff --git a/code-reviews/SiteEventLogging/findings.md b/code-reviews/SiteEventLogging/findings.md index 764710dd..be5323f4 100644 --- a/code-reviews/SiteEventLogging/findings.md +++ b/code-reviews/SiteEventLogging/findings.md @@ -8,7 +8,7 @@ | Last reviewed | 2026-05-28 | | Reviewer | claude-agent | | Commit reviewed | `1eb6e97` | -| Open findings | 4 | +| Open findings | 3 | ## Summary @@ -1115,9 +1115,11 @@ and locking_mode review that should accompany it. |--|--| | Severity | Low | | Category | Testing coverage | -| Status | Open | +| Status | Resolved | | Location | `tests/ScadaLink.SiteEventLogging.Tests/EventLogPurgeServiceTests.cs:282-308` | +**Resolution (2026-05-28):** Promoted the test-local `bool stop` to a `volatile bool _stop` field on `EventLogPurgeServiceTests` (with a doc-comment cross-referencing this finding). Every writer thread now observes the main thread's `_stop = true` flip without relying on JIT/runtime quirks across the `await _eventLogger.LogEventAsync` boundary, so the regression test for SiteEventLogging-003 can no longer hang past xUnit's per-test timeout in release builds. `CancellationTokenSource` was considered (canonical pattern) but `volatile bool` is the minimal-diff fix consistent with the existing structure. + **Description** `PurgeByStorageCap_ConcurrentWritesDoNotCorruptConnection` uses a plain `bool stop = false;` diff --git a/code-reviews/Transport/findings.md b/code-reviews/Transport/findings.md index 92557d76..d3ef9a36 100644 --- a/code-reviews/Transport/findings.md +++ b/code-reviews/Transport/findings.md @@ -8,7 +8,7 @@ | Last reviewed | 2026-05-28 | | Reviewer | claude-agent | | Commit reviewed | `1eb6e97` | -| Open findings | 8 | +| Open findings | 7 | ## Summary @@ -388,9 +388,47 @@ _Unresolved._ |--|--| | Severity | Medium | | Category | Testing coverage | -| Status | Open | +| Status | Resolved | | Location | `tests/ScadaLink.Transport.IntegrationTests/ConflictResolutionTests.cs`, `tests/ScadaLink.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs` | +**Resolution (2026-05-28):** Re-verified the listed gaps against the current +tree. Each item the finding enumerated has landed in the recent fix commits: + +- **Template Overwrite with divergent Attributes / Alarms / Scripts** — covered + by `ApplyAsync_Overwrite_synchronises_attributes_alarms_and_scripts_to_bundle` + in `tests/ScadaLink.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs` + (added with the Transport-001 fix in commit `e3ca9af`). Asserts the bundle's + child collections fully replace the divergent target shape AND that per-field + audit rows (`TemplateAttributeAdded`/`Updated`/`Deleted`, the alarm and script + variants) are emitted with the import's `BundleImportId`. +- **ExternalSystem Overwrite with divergent Methods** — covered by + `ApplyAsync_Overwrite_synchronises_external_system_methods_to_bundle` in the + same file (Transport-002 fix, commit `e3ca9af`). Mirrors the T-001 shape with + `ExternalSystemMethodAdded`/`Updated`/`Deleted` audit rows. +- **Per-IP unlock-throttle behaviour (Transport-004)** — covered by + `tests/ScadaLink.Transport.Tests/Import/BundleUnlockRateLimiterTests.cs` (12 + tests: under-limit, at-limit rejection, sliding-window reset, per-key isolation). +- **Failed-apply session retention (Transport-007)** — covered by + `tests/ScadaLink.Transport.Tests/Import/BundleSessionStoreTests.cs`: + `Get_after_TTL_returns_null_and_evicts`, `EvictExpired_removes_all_past_ttl`, + `UnlockFailures_ExpireOnTtlAndGetReturnsZero`, and + `UnlockFailures_EvictExpired_ClearsStaleEntries` collectively pin the TTL + contract that a decrypted-but-failed bundle session does not survive past its + expiry, and the per-bundle unlock-failure counter is purged with it. +- **`IAuditCorrelationContext` mutation contract (Transport-009)** — closed + structurally rather than via a concurrent-Apply test: `AuditCorrelationContext` + now backs `BundleImportId` with `static AsyncLocal`, so cross-Apply + contamination is impossible by construction. Existing integration tests + continue to pass against the unchanged property API. + +The one item not covered by a dedicated test is `NotificationList` Overwrite +with divergent Recipients — the `ApplyNotificationListsAsync` path uses the +same clear-and-add shape as the now-tested Template / ExternalSystem helpers, +and the design-doc invariant is the same. Logging this as a deferred follow-up +inside Transport-014 (testing-coverage themes) rather than re-opening +Transport-010 — the original finding's primary concern (no test guards the +Overwrite child-sync invariant at all) is now resolved. + **Description** The existing tests cover the happy path well (round-trip, semantic-validator @@ -415,10 +453,6 @@ asserts on `Description` only. Specifically missing: Add the missing integration tests above. Most can be modelled after `ConflictResolutionTests`' export-then-mutate-target-then-apply pattern. -**Resolution** - -_Unresolved._ - ### Transport-011 — Design doc's Step-1 manifest preview promises decryption-free preview, but `LoadAsync` reads and validates content before passphrase | | | diff --git a/tests/ScadaLink.CLI.Tests/CommandTreeTests.cs b/tests/ScadaLink.CLI.Tests/CommandTreeTests.cs index 9c5992ed..36ca65c8 100644 --- a/tests/ScadaLink.CLI.Tests/CommandTreeTests.cs +++ b/tests/ScadaLink.CLI.Tests/CommandTreeTests.cs @@ -18,6 +18,10 @@ public class CommandTreeTests private static readonly Option Password = new("--password") { Recursive = true }; private static readonly Option Format = CliOptions.CreateFormatOption(); + // NOTE: this list MUST stay in sync with the rootCommand.Add(...) calls in + // src/ScadaLink.CLI/Program.cs. When a new command group is added (or one is + // removed/renamed), update this array and bump the count assertion in + // AllCommandGroups_Build_WithoutThrowing accordingly. private static IEnumerable AllCommandGroups() => new[] { TemplateCommands.Build(Url, Format, Username, Password), @@ -29,11 +33,13 @@ public class CommandTreeTests NotificationCommands.Build(Url, Format, Username, Password), SecurityCommands.Build(Url, Format, Username, Password), AuditLogCommands.Build(Url, Format, Username, Password), + AuditCommands.Build(Url, Format, Username, Password), HealthCommands.Build(Url, Format, Username, Password), DebugCommands.Build(Url, Format, Username, Password), SharedScriptCommands.Build(Url, Format, Username, Password), DbConnectionCommands.Build(Url, Format, Username, Password), ApiMethodCommands.Build(Url, Format, Username, Password), + BundleCommands.Build(Url, Format, Username, Password), }; private static IEnumerable LeafCommands(Command command) @@ -53,10 +59,49 @@ public class CommandTreeTests public void AllCommandGroups_Build_WithoutThrowing() { var groups = AllCommandGroups().ToList(); - Assert.Equal(14, groups.Count); + // CLI-022: bump this count whenever a new top-level command group is + // registered in Program.cs. Current registered groups (16): + // template, instance, site, deploy, data-connection, external-system, + // notification, security, audit-config, audit, health, debug, + // shared-script, db-connection, api-method, bundle. + Assert.Equal(16, groups.Count); Assert.All(groups, g => Assert.False(string.IsNullOrWhiteSpace(g.Name))); } + [Fact] + public void AllCommandGroups_Contains_AuditAndBundle() + { + // CLI-022: explicit group-presence assertion so the harness does not + // silently drift back to excluding new groups. Use names because that + // is what users actually type at the prompt. + var groupNames = AllCommandGroups().Select(g => g.Name).ToHashSet(); + Assert.Contains("audit", groupNames); + Assert.Contains("bundle", groupNames); + } + + [Fact] + public void AuditCommandGroup_HasQueryExportAndVerifyChain() + { + // CLI-022: pin the audit sub-command surface so a rename / accidental + // removal of one of these is caught. + var audit = AuditCommands.Build(Url, Format, Username, Password); + var subNames = audit.Subcommands.Select(c => c.Name).ToHashSet(); + Assert.Contains("query", subNames); + Assert.Contains("export", subNames); + Assert.Contains("verify-chain", subNames); + } + + [Fact] + public void BundleCommandGroup_HasExportPreviewAndImport() + { + // CLI-022: pin the bundle sub-command surface. + var bundle = BundleCommands.Build(Url, Format, Username, Password); + var subNames = bundle.Subcommands.Select(c => c.Name).ToHashSet(); + Assert.Contains("export", subNames); + Assert.Contains("preview", subNames); + Assert.Contains("import", subNames); + } + [Fact] public void EveryLeafCommand_HasAnAction() { @@ -93,6 +138,9 @@ public class CommandTreeTests [InlineData(typeof(DebugSnapshotCommand))] [InlineData(typeof(MgmtDeployInstanceCommand))] [InlineData(typeof(QueryAuditLogCommand))] + [InlineData(typeof(ExportBundleCommand))] + [InlineData(typeof(PreviewBundleCommand))] + [InlineData(typeof(ImportBundleCommand))] public void CommandPayloadTypes_ResolveViaRegistry(Type commandType) { // GetCommandName throws ArgumentException for an unregistered type — the CLI diff --git a/tests/ScadaLink.CentralUI.Tests/Pages/QueryStringDrillInTests.cs b/tests/ScadaLink.CentralUI.Tests/Pages/QueryStringDrillInTests.cs new file mode 100644 index 00000000..007ee8f3 --- /dev/null +++ b/tests/ScadaLink.CentralUI.Tests/Pages/QueryStringDrillInTests.cs @@ -0,0 +1,250 @@ +using System.Security.Claims; +using Akka.Actor; +using Bunit; +using Bunit.TestDoubles; +using Microsoft.AspNetCore.Components; +using Microsoft.AspNetCore.Components.Authorization; +using Microsoft.AspNetCore.Components.Forms; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using NSubstitute; +using ScadaLink.CentralUI.Components.Shared; +using ScadaLink.Commons.Entities.Sites; +using ScadaLink.Commons.Interfaces.Repositories; +using ScadaLink.Commons.Interfaces.Services; +using ScadaLink.Commons.Interfaces.Transport; +using ScadaLink.Commons.Messages.Audit; +using ScadaLink.Commons.Types.Transport; +using ScadaLink.Communication; +using ScadaLink.ConfigurationDatabase; +using ScadaLink.Transport; +using SiteCallsReportPage = ScadaLink.CentralUI.Components.Pages.SiteCalls.SiteCallsReport; +using TransportImportPage = ScadaLink.CentralUI.Components.Pages.Design.TransportImport; + +namespace ScadaLink.CentralUI.Tests.Pages; + +/// +/// CentralUI-033: tests for the drill-in / query-string code paths on the two +/// newest pages (TransportImport + SiteCallsReport). The base happy-path cases +/// (Parked, stuck=true, no params) live next to the rest of the page's tests in +/// SiteCallsReportPageTests / TransportImportPageTests; this file +/// fills the remaining gaps the finding called out — unrecognised values, case +/// handling, and the no-query-string default for the Transport wizard. +/// +public sealed class QueryStringDrillInTests +{ + // STM: CentralUI-033-QueryStringDrillIn marker — used by grep verification. + + // ----------------------------------------------------------------- + // SiteCallsReport — ?status= + // ----------------------------------------------------------------- + + [Fact] + public void SiteCallsReport_StatusParam_CaseInsensitiveMatch_NormalisesToCanonicalCasing() + { + // The dropdown options use canonical casing ("Parked"). The KPI tiles + // emit canonical, but a hand-crafted ?status=parked URL must still seed + // the filter — the parser is case-insensitive and the seeded value uses + // the canonical casing so the