Merge branch 'fix/archreview-s1-s2-ci-coverage'

.github/workflows/v2-ci.yml

@@ -1,11 +1,11 @@
 # CI for the v2 branch — runs on every push + PR to the v2-akka-fuse / master
 # branches. Layered into three jobs:
 #   build           dotnet restore + build (fast feedback on compile errors)
-#   unit-tests      every v2 unit-test project
-#   integration     2-node Host.IntegrationTests harness
+#   unit-tests      the WHOLE solution minus the env-gated tiers (self-maintaining)
+#   integration     2-node Host.IntegrationTests harness + OpcUaServer.IntegrationTests
 #
 # Skips E2E (Category=E2E) — that runs nightly via v2-e2e.yml against the full
-# four-node docker-dev stack.
+# four-node docker-dev stack — and LiveIntegration (needs a real gateway/PLC/VPN).
 #
 # Compatible with both GitHub Actions and Gitea Actions (act_runner). The .NET 10
 # SDK is pinned via global.json at the repo root; if no global.json exists, the
@@ -41,22 +41,38 @@ jobs:
   unit-tests:
     needs: build
     runs-on: ubuntu-latest
-    strategy:
-      fail-fast: false
-      matrix:
-        project:
-          - tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests
-          - tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests
-          - tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests
-          - tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests
-          - tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests
     steps:
       - uses: actions/checkout@v4
       - uses: actions/setup-dotnet@v4
         with:
           dotnet-version: 10.0.x
-      - name: dotnet test ${{ matrix.project }}
-        run: dotnet test ${{ matrix.project }} --configuration Release --filter "Category!=E2E"
+      - name: dotnet restore
+        run: dotnet restore ZB.MOM.WW.OtOpcUa.slnx
+      # 07/S-1: ONE whole-solution leg instead of a hand-maintained 5-project matrix that
+      # silently dropped Client (388 tests), Analyzers (31), and every driver + most Core suite.
+      # Self-maintaining — a new *.Tests project is covered automatically, no matrix to widen —
+      # and matches CLAUDE.md's own `dotnet test ZB.MOM.WW.OtOpcUa.slnx` guidance. Excludes only
+      # the env-gated tiers: E2E (nightly, needs the docker-dev fleet) and LiveIntegration (needs
+      # a real gateway/PLC/VPN). The skip-gated *.IntegrationTests Assert.Skip when their fixtures
+      # are unreachable, so they run harmlessly here (and are counted by the skip-gate below).
+      - name: dotnet test (whole solution, minus E2E + LiveIntegration)
+        run: >
+          dotnet test ZB.MOM.WW.OtOpcUa.slnx --configuration Release --no-restore
+          --filter "Category!=E2E&Category!=LiveIntegration"
+          --logger "trx" --results-directory artifacts/trx
+      # 07/S-2 fail-on-skip: a whole-solution run that executed ZERO tests is a masked outage
+      # (bad filter, broken discovery), not a pass — fail loudly. Runs even on test failure so
+      # the diagnostic is always emitted.
+      - name: assert not-all-skipped
+        if: always()
+        run: bash scripts/ci/assert-not-all-skipped.sh artifacts/trx/*.trx
+      - name: upload trx
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: unit-trx
+          path: artifacts/trx/*.trx
+          if-no-files-found: warn
 
   integration:
     needs: build
@@ -73,4 +89,18 @@ jobs:
         with:
           dotnet-version: 10.0.x
       - name: dotnet test ${{ matrix.project }}
-        run: dotnet test ${{ matrix.project }} --configuration Release --filter "Category!=E2E"
+        run: >
+          dotnet test ${{ matrix.project }} --configuration Release
+          --filter "Category!=E2E&Category!=LiveIntegration"
+          --logger "trx" --results-directory artifacts/trx
+      # 07/S-2: this leg has NO fixtures on a hosted runner, so its tests legitimately Assert.Skip.
+      # Report-only (MIN_EXECUTED=0 never fails) so the skip tally is VISIBLE in the log instead of
+      # a silent green. FOLLOW-UP (S-2 option #2): start the one public-image fixture as a workflow
+      # `services:` block (mcr.microsoft.com/iotedge/opc-plc; the modbus/S7/AB/FOCAS sims are
+      # locally-built `otopcua-*` images not pullable here) + set OPCUA_SIM_ENDPOINT, then flip this
+      # leg to MIN_EXECUTED=1 so a fixture outage turns the job red.
+      - name: report skips (integration has no hosted fixtures yet)
+        if: always()
+        env:
+          MIN_EXECUTED: "0"
+        run: bash scripts/ci/assert-not-all-skipped.sh artifacts/trx/*.trx

scripts/ci/assert-not-all-skipped.sh

@@ -0,0 +1,54 @@
+#!/usr/bin/env bash
+#
+# assert-not-all-skipped.sh — arch-review 07/S-2 fail-on-skip guard.
+#
+# A CI test leg that ran but executed ZERO tests reports success, so a whole tier
+# silently skipping (fixtures unreachable, a --filter that matched nothing, a probe
+# that Assert.Skip'd every case) is indistinguishable from a real pass. This gate
+# reads the trx result summary and FAILS when fewer than MIN_EXECUTED tests actually
+# ran across the supplied trx files — turning "green means skipped" into a red build.
+#
+# Usage:   scripts/ci/assert-not-all-skipped.sh <trx> [<trx> ...]
+# Env:     MIN_EXECUTED  minimum executed tests required (default 1).
+#                        Set MIN_EXECUTED=0 for a report-only pass (never fails) —
+#                        used by the fixtureless integration leg until its fixtures
+#                        run as workflow services (S-2 option #2 follow-up).
+#
+set -euo pipefail
+
+min_executed="${MIN_EXECUTED:-1}"
+
+if [ "$#" -eq 0 ]; then
+  echo "assert-not-all-skipped: no trx files given" >&2
+  exit 2
+fi
+
+total_executed=0
+total_skipped=0
+seen=0
+
+for f in "$@"; do
+  if [ ! -f "$f" ]; then
+    echo "assert-not-all-skipped: trx not found: $f" >&2
+    exit 2
+  fi
+  seen=$((seen + 1))
+  # The trx <Counters .../> element in <ResultSummary> carries executed + notExecuted counts.
+  counters="$(grep -oE '<Counters[^>]*/?>' "$f" | head -1)"
+  ex="$(printf '%s' "$counters" | grep -oE 'executed="[0-9]+"' | grep -oE '[0-9]+' || true)"
+  ne="$(printf '%s' "$counters" | grep -oE 'notExecuted="[0-9]+"' | grep -oE '[0-9]+' || true)"
+  ex="${ex:-0}"
+  ne="${ne:-0}"
+  total_executed=$((total_executed + ex))
+  total_skipped=$((total_skipped + ne))
+  echo "  $(basename "$f"): executed=${ex} skipped=${ne}"
+done
+
+echo "assert-not-all-skipped: files=${seen} executed=${total_executed} skipped=${total_skipped} (min required=${min_executed})"
+
+if [ "$total_executed" -lt "$min_executed" ]; then
+  echo "::error::CI executed ${total_executed} test(s) (< ${min_executed}) — the tier ran nothing (all skipped?). Treating as FAILURE, not a pass." >&2
+  exit 1
+fi
+
+echo "assert-not-all-skipped: OK"

tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/AlarmsCommandTests.cs

@@ -23,7 +23,7 @@ public class AlarmsCommandTests
 
         var task = Task.Run(async () => { await command.ExecuteAsync(console); });
 
-        await Task.Delay(100);
+        await fakeService.SubscribeAlarmsInvoked.WaitAsync(TimeSpan.FromSeconds(10));
         console.RequestCancellation();
         await task;
 
@@ -48,7 +48,7 @@ public class AlarmsCommandTests
 
         var task = Task.Run(async () => { await command.ExecuteAsync(console); });
 
-        await Task.Delay(100);
+        await fakeService.SubscribeAlarmsInvoked.WaitAsync(TimeSpan.FromSeconds(10));
         console.RequestCancellation();
         await task;
 
@@ -73,7 +73,7 @@ public class AlarmsCommandTests
 
         var task = Task.Run(async () => { await command.ExecuteAsync(console); });
 
-        await Task.Delay(100);
+        await fakeService.SubscribeAlarmsInvoked.WaitAsync(TimeSpan.FromSeconds(10));
         console.RequestCancellation();
         await task;
 
@@ -101,7 +101,7 @@ public class AlarmsCommandTests
 
         var task = Task.Run(async () => { await command.ExecuteAsync(console); });
 
-        await Task.Delay(100);
+        await fakeService.SubscribeAlarmsInvoked.WaitAsync(TimeSpan.FromSeconds(10));
         console.RequestCancellation();
         await task;
 
@@ -124,7 +124,7 @@ public class AlarmsCommandTests
 
         var task = Task.Run(async () => { await command.ExecuteAsync(console); });
 
-        await Task.Delay(100);
+        await fakeService.SubscribeAlarmsInvoked.WaitAsync(TimeSpan.FromSeconds(10));
         console.RequestCancellation();
         await task;
 
@@ -146,7 +146,7 @@ public class AlarmsCommandTests
 
         var task = Task.Run(async () => { await command.ExecuteAsync(console); });
 
-        await Task.Delay(100);
+        await fakeService.SubscribeAlarmsInvoked.WaitAsync(TimeSpan.FromSeconds(10));
         console.RequestCancellation();
         await task;
 

tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/EventHandlerLifecycleTests.cs

@@ -51,7 +51,7 @@ public class EventHandlerLifecycleTests
 
         var task = Task.Run(async () => await command.ExecuteAsync(console));
 
-        await Task.Delay(150);
+        await fakeService.SubscribeAlarmsInvoked.WaitAsync(TimeSpan.FromSeconds(10));
         console.RequestCancellation();
         await task;
 

tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/Fakes/FakeOpcUaClientService.cs

@@ -45,6 +45,20 @@ public sealed class FakeOpcUaClientService : IOpcUaClientService
     /// <summary>Gets a value indicating whether UnsubscribeAlarmsAsync was called.</summary>
     public bool UnsubscribeAlarmsCalled { get; private set; }
 
+    // Readiness signals (arch-review 07/S-4): tests that start a long-running command on a
+    // background task previously slept a fixed `await Task.Delay(100)` to let it reach its
+    // subscribe call before cancelling — a race that flakes under CI load. These TCS complete
+    // the instant the command actually invokes the corresponding subscribe, so tests can await
+    // the real signal (with a generous timeout) instead of guessing a delay.
+    private readonly TaskCompletionSource _subscribeInvoked = new(TaskCreationOptions.RunContinuationsAsynchronously);
+    private readonly TaskCompletionSource _subscribeAlarmsInvoked = new(TaskCreationOptions.RunContinuationsAsynchronously);
+
+    /// <summary>Completes when the command under test first calls <see cref="SubscribeAsync"/>.</summary>
+    public Task SubscribeInvoked => _subscribeInvoked.Task;
+
+    /// <summary>Completes when the command under test first calls <see cref="SubscribeAlarmsAsync"/>.</summary>
+    public Task SubscribeAlarmsInvoked => _subscribeAlarmsInvoked.Task;
+
     /// <summary>Gets a value indicating whether RequestConditionRefreshAsync was called.</summary>
     public bool RequestConditionRefreshCalled { get; private set; }
 
@@ -187,6 +201,7 @@ public sealed class FakeOpcUaClientService : IOpcUaClientService
     public Task SubscribeAsync(NodeId nodeId, int intervalMs = 1000, CancellationToken ct = default)
     {
         SubscribeCalls.Add((nodeId, intervalMs));
+        _subscribeInvoked.TrySetResult();
         if (SubscribeException != null) throw SubscribeException;
         return Task.CompletedTask;
     }
@@ -202,6 +217,7 @@ public sealed class FakeOpcUaClientService : IOpcUaClientService
     public Task SubscribeAlarmsAsync(NodeId? sourceNodeId = null, int intervalMs = 1000, CancellationToken ct = default)
     {
         SubscribeAlarmsCalls.Add((sourceNodeId, intervalMs));
+        _subscribeAlarmsInvoked.TrySetResult();
         return Task.CompletedTask;
     }
 

tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/SubscribeCommandTests.cs

@@ -26,8 +26,8 @@ public class SubscribeCommandTests
         // Use the console's cancellation to trigger stop.
         var task = Task.Run(async () => { await command.ExecuteAsync(console); });
 
-        // Give it a moment to subscribe, then cancel
-        await Task.Delay(100);
+        // Wait until the command has actually subscribed (deterministic — no fixed-sleep race), then cancel
+        await fakeService.SubscribeInvoked.WaitAsync(TimeSpan.FromSeconds(10));
         console.RequestCancellation();
         await task;
 
@@ -52,7 +52,7 @@ public class SubscribeCommandTests
 
         var task = Task.Run(async () => { await command.ExecuteAsync(console); });
 
-        await Task.Delay(100);
+        await fakeService.SubscribeInvoked.WaitAsync(TimeSpan.FromSeconds(10));
         console.RequestCancellation();
         await task;
 
@@ -75,7 +75,7 @@ public class SubscribeCommandTests
 
         var task = Task.Run(async () => { await command.ExecuteAsync(console); });
 
-        await Task.Delay(100);
+        await fakeService.SubscribeInvoked.WaitAsync(TimeSpan.FromSeconds(10));
         console.RequestCancellation();
         await task;
 
@@ -100,7 +100,7 @@ public class SubscribeCommandTests
 
         var task = Task.Run(async () => { await command.ExecuteAsync(console); });
 
-        await Task.Delay(100);
+        await fakeService.SubscribeInvoked.WaitAsync(TimeSpan.FromSeconds(10));
         console.RequestCancellation();
         await task;