chore(localdb): phase-2 DoD sweep
Build: 0 errors solution-wide, and 0 warnings from every project this branch
touches. The ~816 solution-wide warnings are pre-existing xUnit1051 /
OTOPCUA0001 / CS86xx in untouched driver + client test projects.
Tests: full solution run compared against a full run on a detached worktree at
the pre-branch baseline 2e46d054. The two failure SETS are identical -- all 13
tests, same names, zero new failures. Net +26 tests: +3 Core.AlarmHistorian
(drain gate), +6 Runtime (role view), +17 Host.IntegrationTests (migrator +
convergence). Set comparison rather than counts, because the suite carries
standing environment- and load-dependent failures a count would hide.
The greps found real drift Task 6 missed -- eight live sites still naming the
deleted SqliteStoreAndForwardSink, including the AdminUI /alarms/historian
panel text, which is user-visible, and a <see cref> in HistorianAdapterActor
that resolved to nothing without warning. All repointed at
LocalDbStoreAndForwardSink; CLAUDE.md's alarm-history paragraph now also
records the LocalDb buffer and the primary-gated drain, and drops DatabasePath
from the knob list. docs/AlarmTracking.md still promised an
AlarmHistorianOptions.Validate() startup warning for a relative DatabasePath
and an empty SharedSecret; both branches are gone, so it now says so.
Code references to AlarmHistorian:DatabasePath reduce to exactly two intentional
ones: AlarmSfLegacyMigrator.LegacyPathKey and its test. No `new SqliteConnection`
remains anywhere in Core.AlarmHistorian.
Recon doc gains the durable verification record: guard-deletion evidence for
both vacuous passes, the two exact-set replicated-table pins (both assert set
equality, so an added or a dropped registration fails), and the baseline test
comparison with a per-failure account of why each of the 13 is not this
branch's.
Stops here per the plan. Task 8's live gate needs explicit go-ahead; nothing on
this branch is to be merged.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
@@ -64,12 +64,13 @@
|
||||
{
|
||||
"id": 7,
|
||||
"subject": "Task 7: DoD sweep (offline) \u2014 STOP and report after this",
|
||||
"status": "pending",
|
||||
"status": "completed",
|
||||
"blockedBy": [
|
||||
4,
|
||||
5,
|
||||
6
|
||||
]
|
||||
],
|
||||
"note": "Build: 0 errors solution-wide; 0 warnings from every project this branch touches (the ~816 solution-wide warnings are pre-existing xUnit1051/OTOPCUA0001/CS86xx in untouched driver + client test projects). GREPS FOUND REAL DRIFT Task 6 missed: 8 live sites still named the deleted SqliteStoreAndForwardSink - CLAUDE.md, the AdminUI /alarms/historian panel text (user-visible), HistorianAdapterActor (a <see cref> that did NOT warn), Runtime + Host + 2 Driver.Historian.Gateway doc comments, 1 gateway test comment, docs/drivers/Historian.Wonderware.md; plus docs/AlarmTracking.md still claimed a Validate() startup warning for a relative DatabasePath (that branch is gone). All fixed. Code refs to AlarmHistorian:DatabasePath now reduce to exactly two intentional ones: AlarmSfLegacyMigrator.LegacyPathKey and its test. No 'new SqliteConnection' in Core.AlarmHistorian. Both exact-set pins assert the 3-table set, both directions. Guard-deletion evidence (both vacuous passes + the pin inventory) consolidated into the recon doc."
|
||||
},
|
||||
{
|
||||
"id": 8,
|
||||
|
||||
@@ -300,3 +300,72 @@ Related: `Program.cs:170-173` shows an `AlarmHistorian:Enabled=true` /
|
||||
`ServerHistorian:Enabled=false` deployment is explicitly supported and warned about — which is
|
||||
exactly the rig's shape. Task 6 must confirm `ServerHistorianOptionsValidator` does not fail host
|
||||
start for a disabled section with an empty `Endpoint`.
|
||||
|
||||
---
|
||||
|
||||
## Guard-deletion evidence (recorded at the Task 7 DoD sweep)
|
||||
|
||||
Every guard this phase adds was proved by deleting it and watching the tests go red. Two of those
|
||||
runs found tests that would have passed over a broken implementation. Both are recorded here because
|
||||
the *shape* of the vacuity is reusable, not just the fix.
|
||||
|
||||
### Control 1 — remove `IRedundancyRoleView.Publish` from `DriverHostActor`
|
||||
|
||||
**3 of 6** `DriverHostActorRoleViewTests` went red. The other three assert
|
||||
`ShouldServiceAsPrimary == true`, which is exactly the value the view is **seeded** with — so they
|
||||
cannot distinguish "published true" from "never published at all". That split is expected and
|
||||
acceptable *given* the three that do go red pin the false cases, which only a real publish can
|
||||
produce. Worth knowing before someone reads a future 3/6 result as a regression.
|
||||
|
||||
### Control 2 — remove `RegisterReplicated("alarm_sf_events")` from `LocalDbSetup.OnReady`
|
||||
|
||||
**3 of 4** `AlarmSfConvergenceTests` went red immediately. The fourth,
|
||||
`TheSameEventOnBothNodes_ConvergesToOneRow`, **passed vacuously**: with replication switched off
|
||||
each node trivially held its own single row, and "count == 1 on both nodes" was satisfied by two
|
||||
completely unconverged databases. The assertion could not tell convergence from isolation.
|
||||
|
||||
**Fix:** enqueue a second, *distinct* event on node B and require **both** nodes to hold two rows.
|
||||
Isolation now yields 1 and 2; only convergence yields 2 and 2. Re-run under the control: red.
|
||||
Control restored: all 4 green.
|
||||
|
||||
The general trap — an assertion whose expected value is also what a *disabled* system produces —
|
||||
is the same one recorded in the `#485`/`#486` unreadable-artifact defect class.
|
||||
|
||||
### Exact-set replicated-table pins
|
||||
|
||||
Two, not one, and both assert set equality (ordered `ShouldBe` against the literal three-table list,
|
||||
so an added *or* dropped registration fails):
|
||||
|
||||
- `LocalDbSetupTests` — drives the production `OnReady` callback directly.
|
||||
- `LocalDbWiringTests` — resolves `ILocalDb` from the full driver DI graph, which is what catches a
|
||||
registration that never reaches a running host.
|
||||
|
||||
## Test-suite evidence (Task 7 DoD sweep)
|
||||
|
||||
Full solution run on the branch, compared against a **full solution run on a detached worktree at
|
||||
the pre-branch baseline `2e46d054`**. Comparing failure *sets*, not counts, because the suite has a
|
||||
standing set of environment- and load-dependent failures that a raw count would hide.
|
||||
|
||||
**The two failure sets are identical — all 13 tests, same names, both runs. Zero new failures.**
|
||||
|
||||
| Assembly | Baseline | Branch | Delta |
|
||||
|---|---|---|---|
|
||||
| `Core.AlarmHistorian.Tests` | 0 failed / 29 passed | 0 failed / **32** passed | **+3** (the drain-gate tests; 29 ported from the old sink's suite) |
|
||||
| `Runtime.Tests` | 1 failed / 418 passed | 1 failed / **424** passed | **+6** (`DriverHostActorRoleViewTests`) |
|
||||
| `Host.IntegrationTests` | 2 failed / 169 passed | 2 failed / **186** passed | **+17** (migrator + convergence + pin updates) |
|
||||
|
||||
The 13 standing failures, and why each is not this branch's:
|
||||
|
||||
- **3 × `DriverTypeNamesGuardTests`** — `ArgumentNullException (secretResolver)` out of
|
||||
`GalaxyDriverFactoryExtensions.Register`, reached by reflection. A real pre-existing defect in a
|
||||
guard test, reproduced exactly on the baseline worktree. Unrelated to this phase.
|
||||
- **4 × `AbLegacy.IntegrationTests`**, **3 × `OpcUaClient.IntegrationTests`**,
|
||||
**1 × `DriverProbeHandshakeE2eTests.AbCip_Green_AgainstSim`** — driver fixtures on the
|
||||
`10.100.0.35` docker host are not running.
|
||||
- **1 × `RoslynVirtualTagEvaluatorTests.Evaluate_racing_ClearCompiledScripts_never_fails_with_disposed`**
|
||||
— times out under full-suite load; passes in isolation.
|
||||
- **1 × `ContinuousHistorizationRecorderTests.Retry_after_writer_failure_eventually_acks`** — fails
|
||||
under full-suite load ("writer must have been called at least twice"), and `Runtime.Tests` passes
|
||||
**425/425** when run alone. Also fails on the baseline. Worth noting for anyone reading a future
|
||||
run: this one is *newly observed* here only because the previous sweep's log was truncated, not
|
||||
because the branch introduced it.
|
||||
|
||||
Reference in New Issue
Block a user