diff --git a/src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/AuditLogIngestActor.cs b/src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/AuditLogIngestActor.cs
index 1ca8e692..c4c57525 100644
--- a/src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/AuditLogIngestActor.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.AuditLog/Central/AuditLogIngestActor.cs
@@ -42,13 +42,14 @@ namespace ZB.MOM.WW.ScadaBridge.AuditLog.Central;
/// placeholder.
///
///
-/// Two constructors exist for a deliberate reason: the test ctor injects a
-/// concrete against a per-test MSSQL fixture
-/// (the only way to verify the ingest-timestamp stamp + duplicate-key
-/// idempotency end to end), while the production host wiring registers the
-/// actor as a cluster singleton and must therefore resolve the repository —
-/// which is a scoped EF Core service — from a fresh DI scope per message.
-/// Mirroring the Notification Outbox actor's pattern.
+/// There is exactly ONE constructor, taking an :
+/// the actor is a cluster singleton and the repository is a scoped EF Core
+/// service, so it must be resolved from a fresh DI scope per message and never
+/// held on a field. Mirroring the Notification Outbox actor's pattern. Tests
+/// that need a concrete repository (e.g. the MSSQL-backed fixture verifying the
+/// ingest-timestamp stamp and duplicate-key idempotency) supply a stub provider
+/// that hands one out, rather than a second constructor that would cache it —
+/// see register row 61.
///
///
public class AuditLogIngestActor : ReceiveActor
@@ -100,41 +101,25 @@ public class AuditLogIngestActor : ReceiveActor
///
internal static readonly TimeSpan IngestFallbackBudget = TimeSpan.FromSeconds(5);
- private readonly IServiceProvider? _serviceProvider;
- private readonly IAuditLogRepository? _injectedRepository;
+ private readonly IServiceProvider _serviceProvider;
private readonly ILogger _logger;
///
- /// Test-mode constructor — injects a concrete repository instance whose
- /// lifetime exceeds the test, so the actor reuses the same instance across
- /// every message. Used by the MSSQL-backed TestKit fixture.
- ///
- /// Audit log repository instance shared across all messages.
- /// Logger for ingest diagnostics.
- public AuditLogIngestActor(
- IAuditLogRepository repository,
- ILogger logger)
- {
- ArgumentNullException.ThrowIfNull(repository);
- ArgumentNullException.ThrowIfNull(logger);
-
- _injectedRepository = repository;
- _logger = logger;
-
- ReceiveAsync(OnIngestAsync);
- // The single-repository test ctor cannot service the dual-write —
- // it has no SiteCalls repo and no DbContext. The handler still
- // registers (so callers don't dead-letter) but replies empty so the
- // test surface stays explicit about what this ctor supports.
- ReceiveAsync(OnCachedTelemetryWithoutDualWriteAsync);
- }
-
- ///
- /// Production constructor — resolves from
+ /// The only constructor — resolves from
/// a fresh DI scope per message because the repository is a scoped EF Core
/// service registered by AddConfigurationDatabase. The actor itself
/// is a long-lived cluster singleton, so it cannot hold a scope across
/// messages.
+ ///
+ ///
+ /// Register row 61: a second, repository-taking "test-mode" constructor used
+ /// to cache a scoped repository on a field for the actor's whole lifetime.
+ /// Nothing in production selected it, but it left the captive-dependency bug
+ /// one call site away and had to be allowlisted in the Host tests'
+ /// CaptiveDependencyGuard. It is gone; test fixtures build a stub
+ /// instead, which keeps the actor's own
+ /// contract "resolve per scope, hold nothing".
+ ///
///
/// Root service provider used to open a fresh scope per message.
/// Logger for ingest diagnostics.
@@ -169,59 +154,51 @@ public class AuditLogIngestActor : ReceiveActor
var accepted = new List(cmd.Events.Count);
// Resolve the repository for the whole batch — one DbContext per
- // message, mirroring NotificationOutboxActor. The injected-repository
- // mode (test ctor) skips the scope entirely.
+ // message, mirroring NotificationOutboxActor.
// The IAuditRedactor is also resolved from the per-message scope when
// one is available so the row is truncated + redacted before
- // InsertIfNotExistsAsync. The single-repository test ctor has no
- // service provider — it falls through with no redactor, which preserves
- // the small-payload assumptions baked into the existing fixtures.
+ // InsertIfNotExistsAsync. A composition root that registers no redactor
+ // (test fixtures) falls back to the SafeDefault inside
+ // IngestWithRepositoryAsync rather than to pass-through.
// Use CreateAsyncScope + await using so scoped EF Core
// services (IAsyncDisposable DbContexts) dispose asynchronously
// without blocking on sync Dispose() of pending connection cleanup.
- if (_injectedRepository is not null)
+ //
+ // Guard scope-creation + repository resolution in a
+ // try/catch, mirroring OnCachedTelemetryAsync. A transient DI /
+ // DbContext-factory fault (pooled-context init, SQL-connection
+ // exhaustion, a resolution race during host churn) would otherwise
+ // propagate out of the ReceiveAsync handler, trip the parent's
+ // supervision, and RESTART this central singleton over a transient
+ // fault — dropping the captured reply so the site's Ask times out.
+ // Best-effort audit must never wedge the singleton: log, optionally
+ // bump the failure counter, and still reply with whatever was
+ // accepted (empty on an up-front scope-resolution throw) so the
+ // site keeps its rows Pending and retries on the next drain.
+ try
{
- await IngestWithRepositoryAsync(_injectedRepository, redactor: null, failureCounter: null, cmd, nowUtc, accepted)
+ await using var scope = _serviceProvider.CreateAsyncScope();
+ var repository = scope.ServiceProvider.GetRequiredService();
+ var redactor = scope.ServiceProvider.GetService();
+ // Central health counter is best-effort —
+ // unregistered (test composition roots) means the per-row catch
+ // simply logs without surfacing on the health dashboard.
+ var failureCounter = scope.ServiceProvider.GetService();
+ await IngestWithRepositoryAsync(repository, redactor, failureCounter, cmd, nowUtc, accepted)
.ConfigureAwait(false);
}
- else
+ catch (Exception ex)
{
- // Guard scope-creation + repository resolution in a
- // try/catch, mirroring OnCachedTelemetryAsync. A transient DI /
- // DbContext-factory fault (pooled-context init, SQL-connection
- // exhaustion, a resolution race during host churn) would otherwise
- // propagate out of the ReceiveAsync handler, trip the parent's
- // supervision, and RESTART this central singleton over a transient
- // fault — dropping the captured reply so the site's Ask times out.
- // Best-effort audit must never wedge the singleton: log, optionally
- // bump the failure counter, and still reply with whatever was
- // accepted (empty on an up-front scope-resolution throw) so the
- // site keeps its rows Pending and retries on the next drain.
- try
- {
- await using var scope = _serviceProvider!.CreateAsyncScope();
- var repository = scope.ServiceProvider.GetRequiredService();
- var redactor = scope.ServiceProvider.GetService();
- // Central health counter is best-effort —
- // unregistered (test composition roots) means the per-row catch
- // simply logs without surfacing on the health dashboard.
- var failureCounter = scope.ServiceProvider.GetService();
- await IngestWithRepositoryAsync(repository, redactor, failureCounter, cmd, nowUtc, accepted)
- .ConfigureAwait(false);
- }
- catch (Exception ex)
- {
- // Scope creation or a required-service resolution threw before
- // (or while) processing the batch. Surface a sustained fault on
- // the dashboard if the counter is registered, but never let the
- // throw escape the handler and restart the singleton.
- try { _serviceProvider!.GetService()?.Increment(); }
- catch { /* counter must never throw — defence in depth */ }
- _logger.LogError(
- ex,
- "Audit event batch ingest failed before/while resolving the repository scope; replying with {Accepted} accepted row(s). The site keeps unaccepted rows Pending and retries on the next drain.",
- accepted.Count);
- }
+ // Scope creation or a required-service resolution threw before
+ // (or while) processing the batch. Surface a sustained fault on
+ // the dashboard if the counter is registered, but never let the
+ // throw escape the handler and restart the singleton.
+ try { _serviceProvider.GetService()?.Increment(); }
+ catch { /* counter must never throw — defence in depth */ }
+ _logger.LogError(
+ ex,
+ "Audit event batch ingest failed before/while resolving the repository scope; replying with {Accepted} accepted row(s). The site keeps unaccepted rows Pending and retries on the next drain.",
+ accepted.Count);
}
replyTo.Tell(new IngestAuditEventsReply(accepted));
@@ -357,7 +334,7 @@ public class AuditLogIngestActor : ReceiveActor
try
{
- await using var scope = _serviceProvider!.CreateAsyncScope();
+ await using var scope = _serviceProvider.CreateAsyncScope();
var auditRepo = scope.ServiceProvider.GetRequiredService();
var siteCallRepo = scope.ServiceProvider.GetRequiredService();
var dbContext = scope.ServiceProvider.GetRequiredService();
@@ -544,18 +521,4 @@ public class AuditLogIngestActor : ReceiveActor
return true;
}
- ///
- /// Fallback handler installed on the single-repository test ctor — that
- /// ctor has no DbContext and no , so
- /// it cannot service the dual-write. Logs a warning and replies with an
- /// empty ack so callers fall through to their retry path.
- ///
- private Task OnCachedTelemetryWithoutDualWriteAsync(IngestCachedTelemetryCommand cmd)
- {
- _logger.LogWarning(
- "AuditLogIngestActor received IngestCachedTelemetryCommand on the single-repository ctor; dual-write requires the IServiceProvider ctor. Replying with empty ack ({Count} entries).",
- cmd.Entries.Count);
- Sender.Tell(new IngestCachedTelemetryReply(Array.Empty()));
- return Task.CompletedTask;
- }
}
diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteCallAudit/SiteCallAuditActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteCallAudit/SiteCallAuditActor.cs
index aee226b5..ad2a7078 100644
--- a/src/ZB.MOM.WW.ScadaBridge.SiteCallAudit/SiteCallAuditActor.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.SiteCallAudit/SiteCallAuditActor.cs
@@ -87,8 +87,7 @@ public class SiteCallAuditActor : ReceiveActor
///
private const int MaxReconciliationPagesPerTick = 50;
- private readonly IServiceProvider? _serviceProvider;
- private readonly ISiteCallAuditRepository? _injectedRepository;
+ private readonly IServiceProvider _serviceProvider;
private readonly SiteCallAuditOptions _options;
private readonly ILogger _logger;
@@ -108,28 +107,17 @@ public class SiteCallAuditActor : ReceiveActor
/// () and the site list
/// (). On the production path these are
/// resolved once from the root (central
- /// singletons registered by AddAuditLogCentralReconciliationClient);
- /// in the test path they are injected directly. They are null when
- /// the actor was built via the repo-only test ctor — in that case the
- /// reconciliation tick is NOT started (see ).
- /// The purge tick, by contrast, does NOT depend on these collaborators:
- /// it needs only the repository, so it is armed by
- /// alone (see ).
+ /// singletons registered by AddAuditLogCentralReconciliationClient).
+ /// They are null when the composition root did not register them (a
+ /// minimal host, or a test fixture that deliberately omits them) — in that
+ /// case the reconciliation tick is NOT started (see
+ /// ). The purge tick, by contrast, does
+ /// NOT depend on these collaborators: it needs only the repository, so it is
+ /// always armed (see ).
///
private readonly IPullSiteCallsClient? _pullClient;
private readonly ISiteEnumerator? _siteEnumerator;
- ///
- /// Master switch for the two background schedulers (reconciliation + purge),
- /// set true by the production and reconciliation ctors and false
- /// by the repo-only MSSQL test ctor. The purge timer is
- /// gated on THIS flag rather than on the reconciliation collaborators, so a
- /// host that omits the reconciliation client still purges (no unbounded
- /// central SiteCalls growth) while the MSSQL read/upsert tests stay
- /// free of any scheduled side effects.
- ///
- private readonly bool _backgroundTimersEnabled;
-
///
/// Per-site reconciliation watermark — the composite
/// (UpdatedAtUtc, TrackedOperationId) keyset of the highest row seen
@@ -195,98 +183,23 @@ public class SiteCallAuditActor : ReceiveActor
private IActorRef? _centralCommunication;
///
- /// Test-mode constructor — injects a concrete repository instance whose
- /// lifetime exceeds the test, so the actor reuses the same instance
- /// across every message. Used by the MSSQL-backed TestKit fixture.
- /// An optional lets a test pin the stuck/KPI
- /// windows; when omitted the production defaults apply.
- ///
- /// This ctor disables BOTH background timers (sets
- /// to false) and injects no
- /// reconciliation client/enumerator, so neither the reconciliation tick nor
- /// the purge tick fires — the MSSQL-backed read/upsert tests must see no
- /// scheduled side effects (no phantom pulls, no background purge).
- ///
- ///
- /// Concrete repository instance to use for all messages.
- /// Logger for diagnostics and error reporting.
- /// Optional configuration overrides; production defaults apply when null.
- /// Optional central direct-write audit sink for operator relay actions; null disables the relay audit row.
- public SiteCallAuditActor(
- ISiteCallAuditRepository repository,
- ILogger logger,
- SiteCallAuditOptions? options = null,
- ICentralAuditWriter? auditWriter = null)
- {
- ArgumentNullException.ThrowIfNull(repository);
- ArgumentNullException.ThrowIfNull(logger);
-
- _injectedRepository = new SerializedRepository(repository);
- _logger = logger;
- _options = options ?? new SiteCallAuditOptions();
- _auditWriter = auditWriter;
-
- // Repo-only MSSQL test ctor: keep BOTH background timers off so the
- // read/upsert tests see no scheduled side effects.
- _backgroundTimersEnabled = false;
-
- RegisterHandlers();
- }
-
- ///
- /// Test-mode constructor for the reconciliation tick (Piece A) — injects a
- /// concrete repository PLUS the two reconciliation collaborators directly,
- /// so the per-site self-heal pull is unit-testable in-memory without a DI
- /// container or a live gRPC channel. Background timers are enabled, so the
- /// purge tick starts (it needs only the repository) and the reconciliation
- /// tick starts too because the client + enumerator are present — see
- /// / .
- ///
- /// Concrete repository instance used for upserts and purges.
- /// Enumerates the sites to reconcile each tick.
- /// Pull client used to fetch changed rows from each site.
- /// Logger for diagnostics and error reporting.
- /// Optional configuration overrides; production defaults apply when null.
- ///
- /// Public (not internal) because Akka's default ActivatorProducer
- /// instantiates the actor via reflection with public-only binding flags —
- /// an internal ctor yields a MissingMethodException at actor
- /// creation. Distinguished from the production
- /// ctor by its concrete-collaborator parameter list; only the test project
- /// (or a host that hand-resolves the collaborators) constructs it this way.
- ///
- public SiteCallAuditActor(
- ISiteCallAuditRepository repository,
- ISiteEnumerator siteEnumerator,
- IPullSiteCallsClient pullClient,
- ILogger logger,
- SiteCallAuditOptions? options = null)
- {
- ArgumentNullException.ThrowIfNull(repository);
- ArgumentNullException.ThrowIfNull(siteEnumerator);
- ArgumentNullException.ThrowIfNull(pullClient);
- ArgumentNullException.ThrowIfNull(logger);
-
- _injectedRepository = new SerializedRepository(repository);
- _siteEnumerator = siteEnumerator;
- _pullClient = pullClient;
- _logger = logger;
- _options = options ?? new SiteCallAuditOptions();
-
- // Reconciliation test ctor: collaborators present, so both timers arm
- // (the reconciliation tick uses the collaborators; the purge tick needs
- // only the repository).
- _backgroundTimersEnabled = true;
-
- RegisterHandlers();
- }
-
- ///
- /// Production constructor — resolves
+ /// The only constructor — resolves
/// from a fresh DI scope per message because the repository is a scoped EF
/// Core service registered by AddConfigurationDatabase. The actor
/// itself is a long-lived cluster singleton, so it cannot hold a scope
/// across messages.
+ ///
+ ///
+ /// Register row 61: two further "test-mode" constructors used to take a
+ /// concrete repository and cache it (wrapped in a serializing decorator) on
+ /// a field for the actor's whole lifetime. Nothing in production selected
+ /// them, but they left the captive-dependency bug one call site away and had
+ /// to be allowlisted in the Host tests' CaptiveDependencyGuard. They
+ /// are gone; test fixtures build a stub
+ /// instead — and own the serializing decorator themselves, since it exists
+ /// only to compensate for a fixture handing the SAME repository instance to
+ /// every scope. Production never needed it: each scope gets its own.
+ ///
///
/// DI service provider used to create a scoped repository per message.
/// Actor configuration (stuck threshold, KPI interval, relay timeout).
@@ -321,12 +234,6 @@ public class SiteCallAuditActor : ReceiveActor
_pullClient = serviceProvider.GetService();
_siteEnumerator = serviceProvider.GetService();
- // Production path: background timers run. The purge tick is armed
- // unconditionally here (it needs only the repository); the reconciliation
- // tick additionally requires its collaborators and logs a Warning if
- // they were not registered.
- _backgroundTimersEnabled = true;
-
RegisterHandlers();
}
@@ -462,11 +369,6 @@ public class SiteCallAuditActor : ReceiveActor
///
private void StartReconciliationTimer()
{
- if (!_backgroundTimersEnabled)
- {
- return;
- }
-
if (_pullClient is null || _siteEnumerator is null)
{
_logger.LogWarning(
@@ -488,23 +390,16 @@ public class SiteCallAuditActor : ReceiveActor
///
/// Starts the daily purge tick. The purge needs ONLY the
- /// repository — never the reconciliation collaborators — so it is gated on
- /// alone, NOT on
+ /// repository — never the reconciliation collaborators — so it is armed
+ /// unconditionally, NOT gated on
/// / . This decouples
/// the daily terminal-row purge from the reconciliation client: a host that
/// registers Site Call Audit without the reconciliation client still purges,
/// so the central SiteCalls table can never grow unbounded just
- /// because the self-heal puller is absent. Only the repo-only MSSQL test ctor
- /// (background timers off) skips it, keeping the read/upsert tests free of
- /// scheduled side effects.
+ /// because the self-heal puller is absent.
///
private void StartPurgeTimer()
{
- if (!_backgroundTimersEnabled)
- {
- return;
- }
-
var interval = _options.ResolvedPurgeInterval;
_purgeTimer = Context.System.Scheduler.ScheduleTellRepeatedlyCancelable(
// Short first tick (min(interval, 5min)): purges are idempotent, and a full-interval
@@ -532,20 +427,11 @@ public class SiteCallAuditActor : ReceiveActor
var replyTo = Sender;
var id = cmd.SiteCall.TrackedOperationId;
- // Scope-per-message mirrors AuditLogIngestActor — production EF
- // repository is scoped; the injected-repository mode (tests) skips
- // the scope entirely.
- IServiceScope? scope = null;
- ISiteCallAuditRepository repository;
- if (_injectedRepository is not null)
- {
- repository = _injectedRepository;
- }
- else
- {
- scope = _serviceProvider!.CreateScope();
- repository = scope.ServiceProvider.GetRequiredService();
- }
+ // Scope-per-message mirrors AuditLogIngestActor — the EF repository is
+ // a scoped service, so it is resolved fresh here and never held.
+ IServiceScope? scope = _serviceProvider.CreateScope();
+ ISiteCallAuditRepository repository =
+ scope.ServiceProvider.GetRequiredService();
try
{
@@ -624,15 +510,8 @@ public class SiteCallAuditActor : ReceiveActor
// thread on a synchronous Dispose() of pending connection cleanup — the tick
// holds the scope across many awaited UpsertAsync calls. Mirrors the sibling
// SiteAuditReconciliationActor.OnTickAsync. ResolveRepository() (sync Dispose)
- // is retained for the synchronous message-handler paths. In the injected-
- // repository test path there is no scope to open and the test repo is reused.
- if (_injectedRepository is not null)
- {
- await ReconcileSitesAsync(sites, client, _injectedRepository).ConfigureAwait(false);
- return;
- }
-
- await using var scope = _serviceProvider!.CreateAsyncScope();
+ // is retained for the synchronous message-handler paths.
+ await using var scope = _serviceProvider.CreateAsyncScope();
var repository = scope.ServiceProvider.GetRequiredService();
await ReconcileSitesAsync(sites, client, repository).ConfigureAwait(false);
}
@@ -860,15 +739,8 @@ public class SiteCallAuditActor : ReceiveActor
// Open the scope INLINE with CreateAsyncScope + await using
// so the scoped EF Core repository (an IAsyncDisposable DbContext) disposes
// asynchronously rather than blocking the Akka dispatcher thread on a
- // synchronous Dispose(). Mirrors SiteAuditReconciliationActor; the
- // injected-repository test path reuses the test repo with no scope.
- if (_injectedRepository is not null)
- {
- await PurgeWithRepositoryAsync(_injectedRepository, threshold).ConfigureAwait(false);
- return;
- }
-
- await using var scope = _serviceProvider!.CreateAsyncScope();
+ // synchronous Dispose(). Mirrors SiteAuditReconciliationActor.
+ await using var scope = _serviceProvider.CreateAsyncScope();
var repository = scope.ServiceProvider.GetRequiredService();
await PurgeWithRepositoryAsync(repository, threshold).ConfigureAwait(false);
}
@@ -1454,19 +1326,13 @@ public class SiteCallAuditActor : ReceiveActor
///
/// Resolves an for one read message.
- /// In test mode the injected instance is returned with a null scope; in
- /// production a fresh DI scope is created and returned so the caller can
- /// dispose it once the read completes — the same scope-per-message pattern
- /// as .
+ /// A fresh DI scope is created and returned so the caller can dispose it
+ /// once the read completes — the same scope-per-message pattern as
+ /// .
///
private (IServiceScope? Scope, ISiteCallAuditRepository Repository) ResolveRepository()
{
- if (_injectedRepository is not null)
- {
- return (null, _injectedRepository);
- }
-
- var scope = _serviceProvider!.CreateScope();
+ var scope = _serviceProvider.CreateScope();
return (scope, scope.ServiceProvider.GetRequiredService());
}
@@ -1565,91 +1431,6 @@ public class SiteCallAuditActor : ReceiveActor
private PurgeComplete() { }
}
- ///
- /// Serializing wrapper applied to a repository handed in through the
- /// test constructors. One call at a time, in arrival order.
- ///
- ///
- ///
- /// In production every consumer of this actor's repository gets its OWN DI
- /// scope — one per message, one per background pass — so the off-mailbox
- /// reconciliation and purge passes can safely run alongside mailbox handlers:
- /// separate scopes mean separate DbContexts. The injected-repository
- /// constructors break that assumption: ONE instance (typically wrapping one
- /// DbContext) is shared by the mailbox handlers AND the background
- /// passes, and DbContext forbids concurrent operations — "A second
- /// operation was started on this context instance" is a hard fault, and the
- /// interleaving is nondeterministic, so it surfaces as a flaky test rather
- /// than a reliable one.
- ///
- ///
- /// Serializing at the CALL, not around the whole pass, is deliberate: it
- /// removes the concurrency hazard while preserving the property the
- /// off-mailbox passes exist for — a long drain (blocked in a network pull,
- /// holding no repository call) still lets ingest, query and KPI messages be
- /// answered. Suspending the mailbox for the duration of a pass would have
- /// been simpler and would have invalidated exactly those regression tests.
- /// Production is untouched: it never constructs this type.
- ///
- ///
- private sealed class SerializedRepository : ISiteCallAuditRepository
- {
- private readonly ISiteCallAuditRepository _inner;
- private readonly SemaphoreSlim _gate = new(1, 1);
-
- public SerializedRepository(ISiteCallAuditRepository inner) => _inner = inner;
-
- public Task UpsertAsync(SiteCall siteCall, CancellationToken ct = default) =>
- RunAsync(() => _inner.UpsertAsync(siteCall, ct));
-
- public Task GetAsync(TrackedOperationId id, CancellationToken ct = default) =>
- RunAsync(() => _inner.GetAsync(id, ct));
-
- public Task> QueryAsync(
- SiteCallQueryFilter filter, SiteCallPaging paging, CancellationToken ct = default) =>
- RunAsync(() => _inner.QueryAsync(filter, paging, ct));
-
- public Task PurgeTerminalAsync(DateTime olderThanUtc, CancellationToken ct = default) =>
- RunAsync(() => _inner.PurgeTerminalAsync(olderThanUtc, ct));
-
- public Task ComputeKpisAsync(
- DateTime stuckCutoffUtc, DateTime deliveredSinceUtc, CancellationToken ct = default) =>
- RunAsync(() => _inner.ComputeKpisAsync(stuckCutoffUtc, deliveredSinceUtc, ct));
-
- public Task> ComputePerSiteKpisAsync(
- DateTime stuckCutoffUtc, DateTime deliveredSinceUtc, CancellationToken ct = default) =>
- RunAsync(() => _inner.ComputePerSiteKpisAsync(stuckCutoffUtc, deliveredSinceUtc, ct));
-
- public Task> ComputePerNodeKpisAsync(
- DateTime stuckCutoffUtc, DateTime deliveredSinceUtc, CancellationToken ct = default) =>
- RunAsync(() => _inner.ComputePerNodeKpisAsync(stuckCutoffUtc, deliveredSinceUtc, ct));
-
- private async Task RunAsync(Func call)
- {
- await _gate.WaitAsync().ConfigureAwait(false);
- try
- {
- await call().ConfigureAwait(false);
- }
- finally
- {
- _gate.Release();
- }
- }
-
- private async Task RunAsync(Func> call)
- {
- await _gate.WaitAsync().ConfigureAwait(false);
- try
- {
- return await call().ConfigureAwait(false);
- }
- finally
- {
- _gate.Release();
- }
- }
- }
}
///
diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogIngestActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogIngestActorTests.cs
index 61fdc239..aa6f70e5 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogIngestActorTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogIngestActorTests.cs
@@ -51,10 +51,15 @@ public class AuditLogIngestActorTests : TestKit, IClassFixture
- Sys.ActorOf(Props.Create(() => new AuditLogIngestActor(
- repository,
+ private IActorRef CreateActor(IAuditLogRepository repository)
+ {
+ // Register row 61: the actor's repository-taking test ctor is gone; the fixture
+ // supplies a stub provider that hands this repository out of every scope.
+ var provider = TestSupport.StubAuditRepositoryProvider.Build(repository);
+ return Sys.ActorOf(Props.Create(() => new AuditLogIngestActor(
+ provider,
NullLogger.Instance)));
+ }
[SkippableFact]
public async Task Receive_BatchOf5_Calls_Repo_5Times_Acks_All_5()
diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/DatabaseSyncEmissionEndToEndTests.cs b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/DatabaseSyncEmissionEndToEndTests.cs
index c6490b4d..0a7997b8 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/DatabaseSyncEmissionEndToEndTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/DatabaseSyncEmissionEndToEndTests.cs
@@ -130,10 +130,15 @@ public class DatabaseSyncEmissionEndToEndTests : TestKit, IClassFixture
- Sys.ActorOf(Props.Create(() => new AuditLogIngestActor(
- repo,
+ private IActorRef CreateIngestActor(IAuditLogRepository repo)
+ {
+ // Register row 61: the actor's repository-taking test ctor is gone; the fixture
+ // supplies a stub provider that hands this repository out of every scope.
+ var provider = TestSupport.StubAuditRepositoryProvider.Build(repo);
+ return Sys.ActorOf(Props.Create(() => new AuditLogIngestActor(
+ provider,
NullLogger.Instance)));
+ }
private IActorRef CreateTelemetryActor(
ISiteAuditQueue queue,
diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/ExecutionIdCorrelationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/ExecutionIdCorrelationTests.cs
index 02ab43b3..a936f0ee 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/ExecutionIdCorrelationTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/ExecutionIdCorrelationTests.cs
@@ -127,10 +127,15 @@ public class ExecutionIdCorrelationTests : TestKit, IClassFixture
- Sys.ActorOf(Props.Create(() => new AuditLogIngestActor(
- repo,
+ private IActorRef CreateIngestActor(IAuditLogRepository repo)
+ {
+ // Register row 61: the actor's repository-taking test ctor is gone; the fixture
+ // supplies a stub provider that hands this repository out of every scope.
+ var provider = TestSupport.StubAuditRepositoryProvider.Build(repo);
+ return Sys.ActorOf(Props.Create(() => new AuditLogIngestActor(
+ provider,
NullLogger.Instance)));
+ }
private IActorRef CreateTelemetryActor(
ISiteAuditQueue queue,
diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/SyncCallEmissionEndToEndTests.cs b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/SyncCallEmissionEndToEndTests.cs
index 2e7633ad..409610fa 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/SyncCallEmissionEndToEndTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/SyncCallEmissionEndToEndTests.cs
@@ -110,10 +110,15 @@ public class SyncCallEmissionEndToEndTests : TestKit, IClassFixture
- Sys.ActorOf(Props.Create(() => new AuditLogIngestActor(
- repo,
+ private IActorRef CreateIngestActor(IAuditLogRepository repo)
+ {
+ // Register row 61: the actor's repository-taking test ctor is gone; the fixture
+ // supplies a stub provider that hands this repository out of every scope.
+ var provider = TestSupport.StubAuditRepositoryProvider.Build(repo);
+ return Sys.ActorOf(Props.Create(() => new AuditLogIngestActor(
+ provider,
NullLogger.Instance)));
+ }
private IActorRef CreateTelemetryActor(
ISiteAuditQueue queue,
diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/TestSupport/StubAuditRepositoryProvider.cs b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/TestSupport/StubAuditRepositoryProvider.cs
new file mode 100644
index 00000000..fb3d27da
--- /dev/null
+++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/TestSupport/StubAuditRepositoryProvider.cs
@@ -0,0 +1,41 @@
+using Microsoft.Extensions.DependencyInjection;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
+
+namespace ZB.MOM.WW.ScadaBridge.AuditLog.Tests.TestSupport;
+
+///
+/// Deferred-work register row 61 — the replacement for AuditLogIngestActor's deleted
+/// repository-taking "test-mode" constructor.
+///
+///
+/// That constructor cached a scoped repository on an actor field for the actor's whole
+/// lifetime. Nothing in production selected it, but the shape was one call site away from
+/// re-introducing a captive-dependency bug, so it had to be allowlisted in the Host tests'
+/// CaptiveDependencyGuard. The actor now has a single
+/// constructor; a fixture that wants a concrete repository builds one of these instead.
+///
+///
+///
+/// Behaviour is unchanged for the ingest path: the old ctor passed redactor: null and
+/// failureCounter: null, and a provider with neither registered resolves both to null
+/// too — so the actor still falls back to SafeDefaultAuditRedactor and still logs
+/// without a health counter. The one deliberate difference is that
+/// IngestCachedTelemetryCommand now takes the real dual-write handler rather than the
+/// deleted "no dual write" stub; no fixture built on this helper sends that message.
+///
+///
+internal static class StubAuditRepositoryProvider
+{
+ ///
+ /// Builds a provider whose every scope yields the same audit-log repository instance.
+ ///
+ /// Repository instance shared by every scope.
+ public static ServiceProvider Build(IAuditLogRepository repository)
+ {
+ ArgumentNullException.ThrowIfNull(repository);
+
+ return new ServiceCollection()
+ .AddScoped(_ => repository)
+ .BuildServiceProvider();
+ }
+}
diff --git a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteAlarmLiveCacheServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteAlarmLiveCacheServiceTests.cs
index 8735009e..0aead6e8 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteAlarmLiveCacheServiceTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteAlarmLiveCacheServiceTests.cs
@@ -41,6 +41,17 @@ public class SiteAlarmLiveCacheServiceTests : TestKit
/// Graceful-completion callbacks captured from every subscribe.
public List Completions { get { lock (_lock) { return _completions.ToList(); } } }
+ private int _streamEnds;
+
+ ///
+ /// Register row 40: how many site-wide subscriptions have been torn down (their
+ /// cancellation token fired). This is the aggregator-stop observable — the linger
+ /// stop cancels the aggregator's token, which ends the hanging subscribe below.
+ /// Tests that must show an aggregator was NOT stopped assert on this rather than
+ /// sampling IsLive once after a fixed sleep.
+ ///
+ public int StreamEndCount => Volatile.Read(ref _streamEnds);
+
public override Task SubscribeSiteAsync(
string correlationId, Action onAlarmEvent,
Action onError, Action onCompleted, CancellationToken ct,
@@ -55,7 +66,11 @@ public class SiteAlarmLiveCacheServiceTests : TestKit
// subscribe), which is what makes the aggregator report the stream live.
if (AutoConnect) onConnected?.Invoke();
var tcs = new TaskCompletionSource();
- ct.Register(() => tcs.TrySetResult());
+ ct.Register(() =>
+ {
+ Interlocked.Increment(ref _streamEnds);
+ tcs.TrySetResult();
+ });
return tcs.Task;
}
public override void Unsubscribe(string correlationId) { }
@@ -184,27 +199,47 @@ public class SiteAlarmLiveCacheServiceTests : TestKit
[Fact]
public void Last_Viewer_Leaving_Stops_Aggregator_After_Linger()
{
- var service = CreateService(TimeSpan.FromMilliseconds(200), out _);
+ var linger = TimeSpan.FromMilliseconds(200);
+ var service = CreateService(linger, out var factory);
var sub1 = service.Subscribe(SiteId, () => { });
var sub2 = service.Subscribe(SiteId, () => { });
AwaitCondition(() => service.IsLive(SiteId), TimeSpan.FromSeconds(5));
sub1.Dispose();
- // One viewer remains — still live, not stopped.
- Thread.Sleep(300);
- Assert.True(service.IsLive(SiteId));
+
+ // Register row 40: one viewer remains, so the aggregator must NOT be stopped. The old
+ // test slept 300 ms and sampled IsLive once — a teardown that happened at any other
+ // instant (or one that stopped and restarted the stream) was invisible to it. Poll
+ // CONTINUOUSLY across three linger windows instead, bailing out the moment either
+ // signal says the aggregator went away.
+ var held = System.Diagnostics.Stopwatch.StartNew();
+ AwaitCondition(
+ () => held.Elapsed > linger * 3
+ || !service.IsLive(SiteId)
+ || factory.Client.StreamEndCount > 0,
+ TimeSpan.FromSeconds(5), TimeSpan.FromMilliseconds(10));
+ Assert.True(service.IsLive(SiteId),
+ "the aggregator was reported not-live while a viewer was still subscribed");
+ Assert.Equal(0, factory.Client.StreamEndCount);
+ Assert.Equal(1, factory.GetOrCreateCount);
sub2.Dispose();
// Last viewer left → after the linger the aggregator is torn down and the entry cleared.
AwaitCondition(() => !service.IsLive(SiteId), TimeSpan.FromSeconds(3));
+
+ // Positive control on the SAME observable the negative above rests on: the site-wide
+ // stream really is torn down now, so the zero above was a live signal rather than a
+ // counter that never moves.
+ AwaitCondition(() => factory.Client.StreamEndCount > 0, TimeSpan.FromSeconds(3));
Assert.Empty(service.GetCurrentAlarms(SiteId));
}
[Fact]
public void Resubscribe_Within_Linger_Cancels_The_Pending_Stop()
{
- var service = CreateService(TimeSpan.FromMilliseconds(400), out var factory);
+ var linger = TimeSpan.FromMilliseconds(400);
+ var service = CreateService(linger, out var factory);
var sub1 = service.Subscribe(SiteId, () => { });
AwaitCondition(() => service.IsLive(SiteId), TimeSpan.FromSeconds(5));
@@ -212,13 +247,28 @@ public class SiteAlarmLiveCacheServiceTests : TestKit
sub1.Dispose(); // schedules linger stop
var sub2 = service.Subscribe(SiteId, () => { }); // returns within the linger window → cancels it
- // Wait comfortably past the original linger; the aggregator must still be alive.
- Thread.Sleep(700);
- Assert.True(service.IsLive(SiteId));
+ // Register row 40: the pending stop must be CANCELLED, not merely outrun. The old test
+ // slept 700 ms and sampled once — it could not see a stream that was torn down and
+ // silently replaced inside that window, because GetOrCreateCount is checked only at the
+ // end and the mock hands back the same client object either way. Poll continuously past
+ // the original linger on the teardown observable itself.
+ var kept = System.Diagnostics.Stopwatch.StartNew();
+ AwaitCondition(
+ () => kept.Elapsed > linger * 3
+ || !service.IsLive(SiteId)
+ || factory.Client.StreamEndCount > 0,
+ TimeSpan.FromSeconds(5), TimeSpan.FromMilliseconds(10));
+ Assert.True(service.IsLive(SiteId),
+ "the re-subscribe did not cancel the pending linger stop");
+ Assert.Equal(0, factory.Client.StreamEndCount);
// No second aggregator/stream was opened — the same one was kept warm.
Assert.Equal(1, factory.GetOrCreateCount);
sub2.Dispose();
+
+ // Positive control: with the last viewer gone the linger stop is allowed to run, so the
+ // teardown counter this test's zero rests on does in fact move.
+ AwaitCondition(() => factory.Client.StreamEndCount > 0, TimeSpan.FromSeconds(5));
}
[Fact]
diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorTests.cs
index 6bd888e5..c1358446 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorTests.cs
@@ -76,6 +76,23 @@ public class DataConnectionActorTests : TestKit
adapter.Disconnected += Raise.Event();
}
+ ///
+ /// Register row 40: true once the actor has actually ENTERED the Connected state for
+ /// . BecomeConnected publishes
+ /// to the health collector immediately before
+ /// its Become, so this is the state transition's own observable edge.
+ ///
+ /// An adapter-side connect counter is NOT that edge: it only proves
+ /// ConnectAsync was called, while the actor is still in Connecting until it
+ /// processes the piped ConnectResult. Tests used to bridge that gap with a
+ /// fixed Task.Delay.
+ ///
+ private bool HasEnteredConnectedState(string connectionName) =>
+ _mockHealthCollector.ReceivedCalls().Any(c =>
+ c.GetMethodInfo().Name == nameof(ISiteHealthCollector.UpdateConnectionHealth) &&
+ c.GetArguments() is [string name, ConnectionHealth.Connected, ..] &&
+ name == connectionName);
+
[Fact]
public void WP6_StartsInConnectingState_AttemptsConnect()
{
@@ -430,9 +447,14 @@ public class DataConnectionActorTests : TestKit
var actor = CreateFailoverActor(primaryAdapter, "resub-test", primaryConfig, backupConfig, failoverRetryCount: 1);
- // Wait for initial connect
+ // Wait for the actor to reach Connected — not merely for ConnectAsync to have been
+ // called, and not for a fixed 200 ms to elapse (register row 40). The subscribe below
+ // is only meaningful against the Connected state: issued during Connecting it is
+ // STASHED, so a too-short settle would silently test the stash path instead, and a
+ // too-long one just slows the suite.
AwaitCondition(() => primaryConnectCount >= 1, TimeSpan.FromSeconds(2));
- await Task.Delay(200);
+ AwaitCondition(() => HasEnteredConnectedState("resub-test"), TimeSpan.FromSeconds(5),
+ TimeSpan.FromMilliseconds(10));
// Subscribe to tags while connected on primary
actor.Tell(new SubscribeTagsRequest("corr1", "inst1", "resub-test", ["sensor/temp"], DateTimeOffset.UtcNow));
diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CaptiveDependencyGuard.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CaptiveDependencyGuard.cs
index 8e323481..f95e9c7b 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CaptiveDependencyGuard.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CaptiveDependencyGuard.cs
@@ -72,28 +72,25 @@ internal static class CaptiveDependencyGuard
///
///
///
- /// Both entries below are the same shape: a nullable repository field assigned ONLY by an
- /// explicitly test-mode constructor, so the production wiring
- /// (AkkaHostedService constructs both actors through their
- /// overload) leaves the field null and every handler takes the create-a-scope branch. The
- /// test fixtures that do use those constructors are MSSQL-backed TestKit fixtures that
- /// drive the actor one message at a time, and SiteCallAuditActor additionally wraps
- /// the injected instance in its own SerializedRepository — i.e. the test path
- /// already carries a serialization guard for precisely the concurrency hazard row 45 hit.
+ /// EMPTY, and pinned empty by CaptiveDependencyGuardTests.Allowlist_IsEmpty.
///
///
- /// They are allowlisted rather than removed because deleting a test-mode constructor is a
- /// larger change than this row justifies, and rather than left unflagged because the shape
- /// is genuinely one constructor call away from being row 45 again: if a production call
- /// site ever switched to the repository overload, the actor would hold a scoped EF-backed
- /// repository for its whole lifetime. That residual risk is recorded, not dismissed.
+ /// It previously held two entries — AuditLogIngestActor._injectedRepository and
+ /// SiteCallAuditActor._injectedRepository — nullable repository fields assigned only
+ /// by explicitly test-mode constructors. They were safe solely because no production call
+ /// site picked those overloads, which left the row-45 captive-dependency bug one
+ /// constructor call away. Register row 61 deleted the constructors instead: both actors now
+ /// have a single constructor, the fields are gone, and the
+ /// test fixtures build stub providers that hand a repository out of a scope. With the shape
+ /// removed there is nothing left to exempt.
+ ///
+ ///
+ /// Keep it empty. A new entry means someone reintroduced a scoped service cached on an
+ /// actor field; prefer fixing the actor.
///
///
- internal static readonly IReadOnlySet Allowlist = new HashSet(StringComparer.Ordinal)
- {
- "ZB.MOM.WW.ScadaBridge.AuditLog.Central.AuditLogIngestActor._injectedRepository",
- "ZB.MOM.WW.ScadaBridge.SiteCallAudit.SiteCallAuditActor._injectedRepository",
- };
+ internal static readonly IReadOnlySet Allowlist =
+ new HashSet(StringComparer.Ordinal);
///
/// Scans the given actor types for instance fields holding captive scope state.
diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CaptiveDependencyGuardTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CaptiveDependencyGuardTests.cs
index 1c8419dc..57ddb5ec 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CaptiveDependencyGuardTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CaptiveDependencyGuardTests.cs
@@ -166,38 +166,65 @@ public class CaptiveDependencyGuardSelfTests
}
///
- /// The allowlist is an escape hatch, so it is itself pinned: every entry must name a
- /// field that still exists and would still be flagged. An entry that has stopped matching
- /// is either a stale exemption or — worse — a typo silently covering nothing.
+ /// Register row 61: the allowlist is now EMPTY and must stay that way.
+ ///
+ ///
+ /// This replaces the former staleness self-test, which checked that each entry still named
+ /// a field the scanner would flag. Its two entries — AuditLogIngestActor and
+ /// SiteCallAuditActor's _injectedRepository — covered repository fields
+ /// assigned only by test-mode constructors; those constructors have been deleted and the
+ /// fixtures fold onto the IServiceProvider overload, so there is no longer a field
+ /// to exempt. An empty allowlist is a strictly stronger statement than a correct one:
+ /// nothing is exempt, so nothing can rot.
+ ///
///
[Fact]
- public void Allowlist_EntriesAllStillMatchARealFlaggedField()
+ public void Allowlist_IsEmpty_NoActorExemptFromTheCaptiveDependencyScan()
{
- var actorTypes = CaptiveDependencyGuard.ProductionActorTypes();
-
- // Scan with the allowlist disabled against a scoped set wide enough to include the
- // repositories the allowlisted fields hold.
- var everyScopedType = CaptiveDependencyGuard.ScopedTypes(BuildCentralishScopedProbe());
- var unfiltered = CaptiveDependencyGuard.Scan(actorTypes, everyScopedType, applyAllowlist: false);
-
- var flagged = unfiltered
- .Select(v => $"{v.ActorType.FullName}.{v.FieldName}")
- .ToHashSet(StringComparer.Ordinal);
-
- foreach (var entry in CaptiveDependencyGuard.Allowlist)
- {
- Assert.True(
- flagged.Contains(entry),
- $"Allowlist entry '{entry}' no longer matches a flagged field. Remove it — a stale " +
- "exemption is indistinguishable from an exemption that never worked.");
- }
+ Assert.True(
+ CaptiveDependencyGuard.Allowlist.Count == 0,
+ "The captive-dependency allowlist must stay empty (register row 61). Entries present: " +
+ string.Join(", ", CaptiveDependencyGuard.Allowlist) +
+ ". An entry means an actor caches a scoped service on a field — fix the actor " +
+ "(fold test fixtures onto the IServiceProvider overload) rather than exempting it.");
}
///
- /// A minimal descriptor set naming the two repository types the allowlisted fields hold,
- /// so does not need to boot a
- /// host. Both are registered Scoped by AddConfigurationDatabase; the
- /// role-scan tests below verify that against the real container.
+ /// Register row 61 (the other half of the pin): with the allowlist empty, the two actors
+ /// that used to need exemptions must now scan CLEAN against the scoped repository types
+ /// they resolve. This is what makes the empty allowlist meaningful — it proves the entries
+ /// were removed because the fields are gone, not because the scan stopped looking.
+ ///
+ [Fact]
+ public void FormerlyAllowlistedActors_NowScanClean()
+ {
+ var actorTypes = CaptiveDependencyGuard.ProductionActorTypes();
+ var everyScopedType = CaptiveDependencyGuard.ScopedTypes(BuildCentralishScopedProbe());
+
+ // Allowlist disabled, so a surviving _injectedRepository-shaped field would still show.
+ var unfiltered = CaptiveDependencyGuard.Scan(actorTypes, everyScopedType, applyAllowlist: false);
+
+ var offenders = unfiltered
+ .Where(v => v.ActorType.Name is "AuditLogIngestActor" or "SiteCallAuditActor")
+ .Select(v => v.ToString())
+ .ToList();
+
+ Assert.True(offenders.Count == 0,
+ "AuditLogIngestActor / SiteCallAuditActor must hold no scoped repository field " +
+ "(register row 61). Found: " + string.Join(" | ", offenders));
+
+ // Sanity: the scan really did examine those two actors, so the clean result above is
+ // evidence rather than an empty walk.
+ Assert.Contains(actorTypes, t => t.Name == "AuditLogIngestActor");
+ Assert.Contains(actorTypes, t => t.Name == "SiteCallAuditActor");
+ }
+
+ ///
+ /// A minimal descriptor set naming the two repository types the deleted test-mode
+ /// constructors used to cache, so does
+ /// not need to boot a host. Both are registered Scoped by
+ /// AddConfigurationDatabase; the role-scan tests below verify that against the real
+ /// container.
///
/// The probe descriptors.
private static IServiceCollection BuildCentralishScopedProbe()
diff --git a/tests/ZB.MOM.WW.ScadaBridge.IntegrationTests/AuditLog/SiteAuditPushFlowTests.cs b/tests/ZB.MOM.WW.ScadaBridge.IntegrationTests/AuditLog/SiteAuditPushFlowTests.cs
index abd62283..8b9cc57b 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.IntegrationTests/AuditLog/SiteAuditPushFlowTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.IntegrationTests/AuditLog/SiteAuditPushFlowTests.cs
@@ -149,11 +149,16 @@ public class SiteAuditPushFlowTests : TestKit
public async Task SiteAuditEvent_DrainsToCentral_AndFlipsSiteRowToForwarded()
{
// ── Central side ──────────────────────────────────────────────────
- // Real AuditLogIngestActor over an in-memory repository (test-mode ctor).
+ // Real AuditLogIngestActor over an in-memory repository. Register row 61: the actor's
+ // repository-taking test ctor is gone, so the repository is handed out of a stub
+ // provider's scope instead of being cached on an actor field.
var centralRepo = new InMemoryAuditLogRepository();
+ var ingestProvider = new ServiceCollection()
+ .AddScoped(_ => centralRepo)
+ .BuildServiceProvider();
var ingestActor = Sys.ActorOf(Props.Create(() =>
new ZB.MOM.WW.ScadaBridge.AuditLog.Central.AuditLogIngestActor(
- centralRepo,
+ (IServiceProvider)ingestProvider,
NullLogger.Instance)));
// Real CentralCommunicationActor. It is NOT on the audit path any more (the
diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallAuditActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallAuditActorTests.cs
index dc401484..e08ef198 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallAuditActorTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallAuditActorTests.cs
@@ -73,11 +73,19 @@ public class SiteCallAuditActorTests : TestKit, IClassFixture
- Sys.ActorOf(Props.Create(() => new SiteCallAuditActor(
- repository,
- NullLogger.Instance,
- options)));
+ ISiteCallAuditRepository repository, SiteCallAuditOptions? options = null)
+ {
+ // Register row 61: the actor's repository-taking test ctor is gone. This MSSQL-backed
+ // fixture supplies a stub provider that yields the same repository from every scope,
+ // serialized (one DbContext cannot service concurrent operations) — see
+ // StubRepositoryProvider. No reconciliation collaborators are registered, so the
+ // reconciliation tick stays disarmed exactly as it did under the old ctor.
+ var provider = TestSupport.StubRepositoryProvider.Build(repository);
+ return Sys.ActorOf(Props.Create(() => new SiteCallAuditActor(
+ provider,
+ options ?? new SiteCallAuditOptions(),
+ NullLogger.Instance)));
+ }
[SkippableFact]
public async Task Receive_UpsertSiteCallCommand_Persists_Replies_Accepted()
diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallAuditPurgeTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallAuditPurgeTests.cs
index d6114fe0..d4e8e524 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallAuditPurgeTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallAuditPurgeTests.cs
@@ -102,13 +102,17 @@ public class SiteCallAuditPurgeTests : TestKit
public Task> ComputePerNodeKpisAsync(DateTime a, DateTime b, CancellationToken ct = default) => Task.FromResult>(Array.Empty());
}
- private IActorRef CreateActor(ISiteCallAuditRepository repo, SiteCallAuditOptions options) =>
- Sys.ActorOf(Props.Create(() => new SiteCallAuditActor(
- repo,
- new EmptyEnumerator(),
- new NoOpPullClient(),
- NullLogger.Instance,
- options)));
+ private IActorRef CreateActor(ISiteCallAuditRepository repo, SiteCallAuditOptions options)
+ {
+ // Register row 61: the actor's repository-taking test ctor is gone. The fixture now
+ // supplies a stub provider that hands the recording repo out of every scope.
+ var provider = TestSupport.StubRepositoryProvider.Build(
+ repo, new EmptyEnumerator(), new NoOpPullClient());
+ return Sys.ActorOf(Props.Create(() => new SiteCallAuditActor(
+ provider,
+ options,
+ NullLogger.Instance)));
+ }
// ---------------------------------------------------------------------
// 1. PurgeTick_CallsPurgeTerminal_WithRetentionThreshold
diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallAuditReconciliationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallAuditReconciliationTests.cs
index e131c40a..b94910fc 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallAuditReconciliationTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallAuditReconciliationTests.cs
@@ -238,13 +238,18 @@ public class SiteCallAuditReconciliationTests : TestKit
ISiteEnumerator sites,
IPullSiteCallsClient client,
ISiteCallAuditRepository repo,
- SiteCallAuditOptions options) =>
- Sys.ActorOf(Props.Create(() => new SiteCallAuditActor(
- repo,
- sites,
- client,
- NullLogger.Instance,
- options)));
+ SiteCallAuditOptions options)
+ {
+ // Register row 61: the actor's repository+collaborators test ctor is gone. The
+ // collaborators now reach it the way production supplies them — resolved from the
+ // provider — and the repository comes out of a scope, serialized because this fixture
+ // deliberately shares one instance across every scope.
+ var provider = TestSupport.StubRepositoryProvider.Build(repo, sites, client);
+ return Sys.ActorOf(Props.Create(() => new SiteCallAuditActor(
+ provider,
+ options,
+ NullLogger.Instance)));
+ }
// ---------------------------------------------------------------------
// 1. AbsentRow_PulledFromSite_IsUpserted
@@ -348,28 +353,30 @@ public class SiteCallAuditReconciliationTests : TestKit
}
// ---------------------------------------------------------------------
- // 4. RepoOnly test ctor does NOT start the reconciliation tick
+ // 4. A composition root without the reconciliation collaborators does NOT
+ // start the reconciliation tick
// ---------------------------------------------------------------------
[Fact]
- public void TestCtor_RepositoryOnly_DoesNotStartReconciliationTick()
+ public void NoReconciliationCollaboratorsRegistered_DoesNotStartReconciliationTick()
{
- // The repo-only test ctor (used by the MSSQL-backed actor tests) injects
- // no client/enumerator, so the tick must be gated OFF — otherwise those
- // tests would fire phantom pulls. Build the actor via that ctor and
- // confirm no pull ever happens. We can't observe a non-event directly,
- // so we share a ScriptedPullClient with an isolated actor that DOES run
- // the tick to bound the wait, then assert the repo-only actor's client
- // (a separate instance) recorded nothing.
+ // Register row 61: this used to be phrased as "the repo-only test ctor gates the tick
+ // off". With that ctor deleted, the same guarantee is expressed the way production
+ // actually reaches it — a composition root that registers NEITHER IPullSiteCallsClient
+ // NOR ISiteEnumerator leaves the tick disarmed (GetService returns null for both), so
+ // the MSSQL-backed actor tests fire no phantom pulls. We can't observe a non-event
+ // directly, so we run an isolated actor that DOES tick to bound the wait, then assert
+ // this actor upserted nothing on its own.
var repo = new RecordingRepo();
+ var bareProvider = TestSupport.StubRepositoryProvider.Build(repo);
Sys.ActorOf(Props.Create(() => new SiteCallAuditActor(
- repo,
- NullLogger.Instance,
- FastTickOptions())));
+ bareProvider,
+ FastTickOptions(),
+ NullLogger.Instance)));
- // Run a parallel actor with the full reconciliation ctor and a fast
+ // Run a parallel actor whose provider DOES carry the collaborators, with a fast
// tick; once IT has pulled we know enough wall-clock elapsed that the
- // repo-only actor would have ticked too, had it been wired.
+ // collaborator-less actor would have ticked too, had it been wired.
var liveClient = new ScriptedPullClient();
var liveRepo = new RecordingRepo();
CreateActor(
@@ -383,8 +390,8 @@ public class SiteCallAuditReconciliationTests : TestKit
duration: TimeSpan.FromSeconds(3),
interval: TimeSpan.FromMilliseconds(50));
- // The repo-only actor never reconciles: it has no client to pull with,
- // so it upserts nothing on its own.
+ // The collaborator-less actor never reconciles: it has no client to pull
+ // with, so it upserts nothing on its own.
Assert.Equal(0, repo.UpsertCallCount);
}
diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallRelayTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallRelayTests.cs
index ddea1ce7..a5fe773f 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallRelayTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/SiteCallRelayTests.cs
@@ -64,10 +64,12 @@ public class SiteCallRelayTests : TestKit
private IActorRef CreateActor(IActorRef centralComm)
{
var options = new SiteCallAuditOptions { RelayTimeout = TimeSpan.FromMilliseconds(500) };
+ // Register row 61: stub provider in place of the deleted repository ctor.
+ var provider = TestSupport.StubRepositoryProvider.Build(new ThrowingRepository());
var actor = Sys.ActorOf(Props.Create(() => new SiteCallAuditActor(
- new ThrowingRepository(),
- NullLogger.Instance,
- options)));
+ provider,
+ options,
+ NullLogger.Instance)));
actor.Tell(new RegisterCentralCommunication(centralComm));
return actor;
}
@@ -202,10 +204,11 @@ public class SiteCallRelayTests : TestKit
// No RegisterCentralCommunication — the actor has no transport to reach
// any site, so the only honest answer is "unreachable".
var options = new SiteCallAuditOptions { RelayTimeout = TimeSpan.FromMilliseconds(500) };
+ var provider = TestSupport.StubRepositoryProvider.Build(new ThrowingRepository());
var actor = Sys.ActorOf(Props.Create(() => new SiteCallAuditActor(
- new ThrowingRepository(),
- NullLogger.Instance,
- options)));
+ provider,
+ options,
+ NullLogger.Instance)));
actor.Tell(new RetrySiteCallRequest("corr-7", Guid.NewGuid(), "site-north"));
@@ -286,11 +289,15 @@ public class SiteCallRelayTests : TestKit
{
var writer = new RecordingCentralAuditWriter();
var options = new SiteCallAuditOptions { RelayTimeout = TimeSpan.FromMilliseconds(500) };
- var actor = Sys.ActorOf(Props.Create(() => new SiteCallAuditActor(
+ // The audit writer now reaches the actor through the provider (production resolves it
+ // the same way, from the root provider) rather than through a ctor parameter.
+ var provider = TestSupport.StubRepositoryProvider.Build(
new RowReturningRepository(row),
- NullLogger.Instance,
+ auditWriter: (ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services.ICentralAuditWriter)writer);
+ var actor = Sys.ActorOf(Props.Create(() => new SiteCallAuditActor(
+ provider,
options,
- (ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services.ICentralAuditWriter)writer)));
+ NullLogger.Instance)));
actor.Tell(new RegisterCentralCommunication(centralComm));
return (actor, writer);
}
diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/TestSupport/StubRepositoryProvider.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/TestSupport/StubRepositoryProvider.cs
new file mode 100644
index 00000000..83e1269e
--- /dev/null
+++ b/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/TestSupport/StubRepositoryProvider.cs
@@ -0,0 +1,162 @@
+using Microsoft.Extensions.DependencyInjection;
+using ZB.MOM.WW.ScadaBridge.AuditLog.Central;
+using ZB.MOM.WW.ScadaBridge.Commons.Entities.Audit;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
+using ZB.MOM.WW.ScadaBridge.Commons.Types;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit;
+
+namespace ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests.TestSupport;
+
+///
+/// Deferred-work register row 61 — the replacement for SiteCallAuditActor's deleted
+/// repository-taking "test-mode" constructors.
+///
+///
+/// Those constructors cached a scoped repository on an actor field for the actor's whole
+/// lifetime. Nothing in production selected them, but the shape was one call site away from
+/// re-introducing a captive-dependency bug, so it had to be allowlisted in the Host tests'
+/// CaptiveDependencyGuard. The actor now has a single
+/// constructor; a fixture that wants a concrete repository builds one of these instead.
+///
+///
+///
+/// The actor's contract is unchanged and unweakened — it still resolves per scope and holds
+/// nothing. Handing the SAME instance out of every scope is now the FIXTURE's explicit
+/// choice, which is also why the serializing decorator below lives here rather than in
+/// product code: it exists solely to compensate for that choice.
+///
+///
+internal static class StubRepositoryProvider
+{
+ ///
+ /// Builds a provider whose every scope yields the same repository instance, wrapped in
+ /// , plus any collaborators supplied.
+ ///
+ /// Repository instance shared by every scope.
+ /// Optional site enumerator; omit to leave the reconciliation tick disarmed.
+ /// Optional pull client; omit to leave the reconciliation tick disarmed.
+ /// Optional central direct-write audit sink for relay actions.
+ ///
+ /// False registers the repository unwrapped — for fixtures asserting on call ordering or
+ /// identity where the decorator would get in the way.
+ ///
+ public static ServiceProvider Build(
+ ISiteCallAuditRepository repository,
+ ISiteEnumerator? siteEnumerator = null,
+ IPullSiteCallsClient? pullClient = null,
+ ICentralAuditWriter? auditWriter = null,
+ bool serialize = true)
+ {
+ ArgumentNullException.ThrowIfNull(repository);
+
+ var shared = serialize ? new SerializingSiteCallAuditRepository(repository) : repository;
+
+ var services = new ServiceCollection();
+ services.AddScoped(_ => shared);
+
+ if (siteEnumerator is not null)
+ {
+ services.AddSingleton(siteEnumerator);
+ }
+
+ if (pullClient is not null)
+ {
+ services.AddSingleton(pullClient);
+ }
+
+ if (auditWriter is not null)
+ {
+ services.AddSingleton(auditWriter);
+ }
+
+ return services.BuildServiceProvider();
+ }
+}
+
+///
+/// Serializing wrapper for a repository instance shared across every DI scope in a test.
+/// One call at a time, in arrival order.
+///
+///
+///
+/// In production every consumer of SiteCallAuditActor's repository gets its OWN DI
+/// scope — one per message, one per background pass — so the off-mailbox reconciliation and
+/// purge passes can safely run alongside mailbox handlers: separate scopes mean separate
+/// DbContexts. A fixture that hands ONE instance (typically wrapping one
+/// DbContext) to every scope breaks that assumption, and DbContext forbids
+/// concurrent operations — "A second operation was started on this context instance" is a
+/// hard fault, and the interleaving is nondeterministic, so it surfaces as a flaky test
+/// rather than a reliable one.
+///
+///
+/// Serializing at the CALL, not around the whole pass, is deliberate: it removes the
+/// concurrency hazard while preserving the property the off-mailbox passes exist for — a long
+/// drain (blocked in a network pull, holding no repository call) still lets ingest, query and
+/// KPI messages be answered. Suspending the mailbox for the duration of a pass would have
+/// been simpler and would have invalidated exactly those regression tests.
+///
+///
+/// This type used to live inside the product actor, reachable only from its test
+/// constructors. It moved here with register row 61: production never constructed it, and the
+/// hazard it guards is created by the fixture, not by the actor.
+///
+///
+internal sealed class SerializingSiteCallAuditRepository : ISiteCallAuditRepository
+{
+ private readonly ISiteCallAuditRepository _inner;
+ private readonly SemaphoreSlim _gate = new(1, 1);
+
+ public SerializingSiteCallAuditRepository(ISiteCallAuditRepository inner) => _inner = inner;
+
+ public Task UpsertAsync(SiteCall siteCall, CancellationToken ct = default) =>
+ RunAsync(() => _inner.UpsertAsync(siteCall, ct));
+
+ public Task GetAsync(TrackedOperationId id, CancellationToken ct = default) =>
+ RunAsync(() => _inner.GetAsync(id, ct));
+
+ public Task> QueryAsync(
+ SiteCallQueryFilter filter, SiteCallPaging paging, CancellationToken ct = default) =>
+ RunAsync(() => _inner.QueryAsync(filter, paging, ct));
+
+ public Task PurgeTerminalAsync(DateTime olderThanUtc, CancellationToken ct = default) =>
+ RunAsync(() => _inner.PurgeTerminalAsync(olderThanUtc, ct));
+
+ public Task ComputeKpisAsync(
+ DateTime stuckCutoffUtc, DateTime deliveredSinceUtc, CancellationToken ct = default) =>
+ RunAsync(() => _inner.ComputeKpisAsync(stuckCutoffUtc, deliveredSinceUtc, ct));
+
+ public Task> ComputePerSiteKpisAsync(
+ DateTime stuckCutoffUtc, DateTime deliveredSinceUtc, CancellationToken ct = default) =>
+ RunAsync(() => _inner.ComputePerSiteKpisAsync(stuckCutoffUtc, deliveredSinceUtc, ct));
+
+ public Task> ComputePerNodeKpisAsync(
+ DateTime stuckCutoffUtc, DateTime deliveredSinceUtc, CancellationToken ct = default) =>
+ RunAsync(() => _inner.ComputePerNodeKpisAsync(stuckCutoffUtc, deliveredSinceUtc, ct));
+
+ private async Task RunAsync(Func call)
+ {
+ await _gate.WaitAsync().ConfigureAwait(false);
+ try
+ {
+ await call().ConfigureAwait(false);
+ }
+ finally
+ {
+ _gate.Release();
+ }
+ }
+
+ private async Task RunAsync(Func> call)
+ {
+ await _gate.WaitAsync().ConfigureAwait(false);
+ try
+ {
+ return await call().ConfigureAwait(false);
+ }
+ finally
+ {
+ _gate.Release();
+ }
+ }
+}
diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentWarmThenGateTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentWarmThenGateTests.cs
index d8cdaa8d..58f23c41 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentWarmThenGateTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentWarmThenGateTests.cs
@@ -65,6 +65,21 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable
new SiteRuntimeOptions(), NullLogger.Instance, null,
healthCollector)));
+ ///
+ /// Register row 40: starts a Deployment Manager and blocks until it has PROVABLY finished
+ /// its startup config load, by waiting for its first instance-count report. Replaces a bare
+ /// await Task.Delay(500), which merely hoped 500 ms was enough and — for the empty-site
+ /// assertion — could not tell "startup ran and found nothing" from "startup had not run yet".
+ ///
+ private (IActorRef Dm, DeployedCountCollector Health) CreateStartedDeploymentManager()
+ {
+ var health = new DeployedCountCollector();
+ var dm = CreateDeploymentManager(health);
+ AwaitCondition(() => health.ReportCount > 0, TimeSpan.FromSeconds(20),
+ TimeSpan.FromMilliseconds(25));
+ return (dm, health);
+ }
+
///
/// Captures the deployed-instance count the Deployment Manager reports. The count is
/// mutated only on the actor thread — HandleDeploy adds the instance name,
@@ -73,7 +88,20 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable
///
private sealed class DeployedCountCollector : ISiteHealthCollector
{
+ private int _reports;
+
public int LastDeployedCount { get; private set; }
+
+ ///
+ /// Register row 40: how many times the Deployment Manager has reported instance
+ /// counts. The manager calls unconditionally the
+ /// moment its startup config load completes (HandleStartupConfigsLoaded) —
+ /// including for an empty site — so a non-zero value here is the explicit
+ /// observable edge for "startup has finished", replacing a wall-clock sleep.
+ /// Without it, LastDeployedCount == 0 is satisfied by the field's initial
+ /// value and proves nothing about the manager having started at all.
+ ///
+ public int ReportCount => Volatile.Read(ref _reports);
public void IncrementScriptError() { }
public void IncrementAlarmError() { }
public void IncrementDeadLetter() { }
@@ -86,7 +114,12 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable
public void UpdateConnectionEndpoint(string connectionName, string endpoint) { }
public void UpdateTagQuality(string connectionName, int good, int bad, int uncertain) { }
public void SetStoreAndForwardDepths(IReadOnlyDictionary depths) { }
- public void SetInstanceCounts(int deployed, int enabled, int disabled) => LastDeployedCount = deployed;
+ public void SetInstanceCounts(int deployed, int enabled, int disabled)
+ {
+ LastDeployedCount = deployed;
+ Interlocked.Increment(ref _reports);
+ }
+
public void SetParkedMessageCount(int count) { }
public void SetNodeHostname(string hostname) { }
public void SetClusterNodes(IReadOnlyList nodes) { }
@@ -112,9 +145,10 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable
[Fact]
public async Task DeleteArrivingDuringTheCompileWarm_IsQueuedAndAppliedAfterTheDeploy()
{
- var health = new DeployedCountCollector();
- var dm = CreateDeploymentManager(health);
- await Task.Delay(500); // empty startup
+ // The startup load has demonstrably completed (it reported counts at least once), so
+ // the zero below is the manager's own observation of an empty site rather than an
+ // untouched field.
+ var (dm, health) = CreateStartedDeploymentManager();
Assert.Equal(0, health.LastDeployedCount);
var deployProbe = CreateTestProbe();
@@ -145,8 +179,7 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable
[Fact]
public async Task SecondDeployDuringTheWarm_SupersedesTheFirst_AndAnswersItsDeployer()
{
- var dm = CreateDeploymentManager();
- await Task.Delay(500);
+ var (dm, _) = CreateStartedDeploymentManager();
var first = CreateTestProbe();
var second = CreateTestProbe();
@@ -177,8 +210,7 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable
[Fact]
public async Task DeploysForDifferentInstances_DoNotBlockEachOther()
{
- var dm = CreateDeploymentManager();
- await Task.Delay(500);
+ var (dm, _) = CreateStartedDeploymentManager();
var probeX = CreateTestProbe();
var probeY = CreateTestProbe();
diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorChildAttributeRaceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorChildAttributeRaceTests.cs
index 437b653b..0f12b6e1 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorChildAttributeRaceTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorChildAttributeRaceTests.cs
@@ -107,16 +107,54 @@ public class InstanceActorChildAttributeRaceTests : TestKit, IDisposable
]
};
- /// Resolves the live actor instance behind a local .
- private static object GetActorInstance(IActorRef actorRef)
+ ///
+ /// Resolves a named child and the live actor instance behind it, retrying until BOTH
+ /// exist (register row 40 — absence/settle timing pins).
+ ///
+ /// Two distinct publication races sit between "the parent reported N children" and
+ /// "the child's actor object is reachable by reflection", and neither is ordered against
+ /// the test thread: throws
+ /// until the child's cell is published on the parent,
+ /// and the cell's internal Actor property stays null until the child's constructor
+ /// has run on the child's own mailbox thread. A single-shot resolve therefore fails
+ /// intermittently under load — observed both ways in full-suite runs ("subject was null"
+ /// from ResolveOne, and a null instance from the reflection read). Retrying to a bounded
+ /// deadline removes the race without weakening the assertion: what the test proves is
+ /// still the seed-dictionary isolation contract, checked once both objects exist.
+ ///
+ private async Task ResolveChildInstanceAsync(IActorRef parent, string childName)
+ where T : class
{
- var cell = ((ActorRefWithCell)actorRef).Underlying;
- // ActorCell exposes the actor instance via its internal Actor property.
- var actorProp = cell.GetType().GetProperty(
- "Actor", BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public);
- var instance = actorProp!.GetValue(cell);
- Assert.NotNull(instance);
- return instance!;
+ var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(15);
+ while (true)
+ {
+ try
+ {
+ var child = await Sys.ActorSelection(parent.Path / childName)
+ .ResolveOne(TimeSpan.FromSeconds(1));
+ var cell = ((ActorRefWithCell)child).Underlying;
+ // ActorCell exposes the actor instance via its internal Actor property.
+ var actorProp = cell.GetType().GetProperty(
+ "Actor", BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public);
+ if (actorProp!.GetValue(cell) is T typed)
+ {
+ return typed;
+ }
+ }
+ catch (ActorNotFoundException)
+ {
+ // The child's cell is not published on the parent yet — retry below.
+ }
+
+ if (DateTime.UtcNow > deadline)
+ {
+ throw new TimeoutException(
+ $"Child actor '{childName}' (or its {typeof(T).Name} instance) never " +
+ "materialised within the resolve deadline.");
+ }
+
+ await Task.Delay(25);
+ }
}
private static Dictionary GetPrivateAttributes(InstanceActor instance)
@@ -157,9 +195,7 @@ public class InstanceActorChildAttributeRaceTests : TestKit, IDisposable
// mutates on another thread (SiteRuntime-017).
foreach (var name in new[] { "WorkerA", "WorkerB" })
{
- var child = await Sys.ActorSelection(testRef.Path / $"script-{name}")
- .ResolveOne(TimeSpan.FromSeconds(5));
- var scriptActor = (ScriptActor)GetActorInstance(child);
+ var scriptActor = await ResolveChildInstanceAsync(testRef, $"script-{name}");
Assert.NotNull(scriptActor.SeedAttributesReference);
Assert.False(
@@ -177,9 +213,7 @@ public class InstanceActorChildAttributeRaceTests : TestKit, IDisposable
}
// The Alarm Actor must likewise be seeded from an isolated copy.
- var alarmChild = await Sys.ActorSelection(testRef.Path / "alarm-HighTemp")
- .ResolveOne(TimeSpan.FromSeconds(5));
- var alarmActor = (AlarmActor)GetActorInstance(alarmChild);
+ var alarmActor = await ResolveChildInstanceAsync(testRef, "alarm-HighTemp");
Assert.NotNull(alarmActor.SeedAttributesReference);
Assert.False(
diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptDeadlineAtEnqueueTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptDeadlineAtEnqueueTests.cs
index 9ce81e89..b1ea03cf 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptDeadlineAtEnqueueTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptDeadlineAtEnqueueTests.cs
@@ -101,8 +101,26 @@ public class ScriptDeadlineAtEnqueueTests : TestKit, IDisposable
var caller = CreateTestProbe();
late.Tell(new ScriptCallRequest("Late", null, 0, "corr-late"), caller.Ref);
- // Let its whole budget elapse while it is still queued, then free the worker.
- Thread.Sleep(TimeSpan.FromSeconds(2));
+ // Register row 40 — edge 1 (replaces a bare Thread.Sleep(2s)): the late run must
+ // actually be SITTING IN THE SCHEDULER QUEUE. The old test only slept and hoped;
+ // a blocker that failed to occupy the worker would have let the late run execute
+ // normally, and the shed-at-dequeue path under test would never have been reached.
+ AwaitCondition(() => scheduler.QueueDepth >= 1, TimeSpan.FromSeconds(15),
+ TimeSpan.FromMilliseconds(25));
+
+ // Register row 40 — edge 2: hold until the late run's 1 s budget has provably
+ // elapsed, re-reading the queue on every poll and bailing out early the instant the
+ // run leaves the queue. The post-condition below then proves the budget burned
+ // *while queued* — the actual design claim ("queue wait consumes the run's budget"),
+ // which a fixed sleep asserted nothing about.
+ var queuedFor = System.Diagnostics.Stopwatch.StartNew();
+ AwaitCondition(
+ () => queuedFor.Elapsed >= TimeSpan.FromSeconds(1.5) || scheduler.QueueDepth == 0,
+ TimeSpan.FromSeconds(15), TimeSpan.FromMilliseconds(25));
+ Assert.True(scheduler.QueueDepth >= 1,
+ "the late run left the queue before its budget elapsed, so this run never " +
+ "exercised the shed-at-dequeue path");
+
DeadlineHooks.Gate.Release();
var result = caller.ExpectMsg(TimeSpan.FromSeconds(15));
@@ -123,12 +141,56 @@ public class ScriptDeadlineAtEnqueueTests : TestKit, IDisposable
Assert.DoesNotContain(rows, r => r.Message.Contains("started", StringComparison.OrdinalIgnoreCase));
}, TimeSpan.FromSeconds(5));
- // And the watchdog did NOT report it as a stuck thread: it was cancelled while queued,
- // so it never held a worker.
- Thread.Sleep(TimeSpan.FromSeconds(2)); // well past the 1 s grace
+ // ── Register row 40 — the watchdog absence, proved by a positive control ──
+ //
+ // The claim is that the shed run was NOT reported as a stuck thread: it was cancelled
+ // while queued, so it never held a worker. The old test slept 2 s and asserted the
+ // absence, which false-passes whenever the watchdog simply had not got around to
+ // evaluating anything yet — the assertion could not distinguish "the watchdog looked
+ // and correctly stayed silent" from "the watchdog never looked".
+ //
+ // So: enqueue a script that genuinely wedges a worker, on the SAME scheduler and with
+ // the SAME 1 s timeout + 1 s grace, and armed strictly LATER than the late run's
+ // watchdog. Its stuck report is the observable edge — once it lands, every watchdog
+ // armed at or before it has provably run its grace delay and made its decision.
+ //
+ // The body blocks its worker SYNCHRONOUSLY (so the watchdog can detach the thread it
+ // holds) but for a BOUNDED 2.5 s that comfortably clears 1 s timeout + 1 s grace and
+ // then self-releases. A gate held until test teardown would work too, but it leaves a
+ // blocked OS thread alive past the test and perturbs timing-sensitive tests elsewhere
+ // in this assembly.
+ var wedged = BuildScriptActor(
+ "Wedged",
+ CompileRaw("System.Threading.Thread.Sleep(2500); return null;"),
+ new SiteRuntimeOptions { ScriptExecutionTimeoutSeconds = 1, StuckScriptGraceMs = 1000 },
+ scheduler, new SingleServiceProvider(siteLog));
+ wedged.Tell(new ScriptCallRequest("Wedged", null, 0, "corr-wedged"), ActorRefs.NoSender);
+
+ // Positive event, bounded wait: the watchdog fires, names the wedged script and
+ // detaches its worker.
+ AwaitAssert(() =>
+ {
+ Assert.Contains(siteLog.OfType("script"),
+ r => r.Message.Contains("'Wedged'", StringComparison.Ordinal) &&
+ r.Message.Contains("still executing", StringComparison.OrdinalIgnoreCase));
+ }, TimeSpan.FromSeconds(30), TimeSpan.FromMilliseconds(50));
+
+ // Negative control, evaluated only now that a later-armed watchdog has demonstrably
+ // completed its grace delay: the shed run was never reported stuck.
Assert.DoesNotContain(siteLog.OfType("script"),
- r => r.Message.Contains("still executing", StringComparison.OrdinalIgnoreCase));
- Assert.Equal(0, scheduler.DetachedThreadCount);
+ r => r.Message.Contains("'Late'", StringComparison.Ordinal) &&
+ r.Message.Contains("still executing", StringComparison.OrdinalIgnoreCase));
+
+ // And the detach counter moved for exactly one run — the wedged one. The old
+ // `Assert.Equal(0, DetachedThreadCount)` was satisfied by a counter that never moves
+ // for any reason; this pins that it does move, and that the shed run did not move it.
+ Assert.Equal(1, scheduler.DetachedThreadCount);
+
+ // Wait for the positive control's body to unwind and its detached worker to exit
+ // BEFORE the scheduler is disposed at end of scope, so the test leaves no threads
+ // behind for the rest of the assembly to contend with.
+ AwaitCondition(() => scheduler.BusyThreadCount == 0 && scheduler.DetachedThreadCount == 0,
+ TimeSpan.FromSeconds(20), TimeSpan.FromMilliseconds(25));
}
}
diff --git a/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/CachedCallAttemptEmissionTests.cs b/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/CachedCallAttemptEmissionTests.cs
index f01746fa..3ee0ae61 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/CachedCallAttemptEmissionTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/CachedCallAttemptEmissionTests.cs
@@ -17,7 +17,13 @@ namespace ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests;
/// DefaultRetryInterval=Zero so the timer-driven retry sweep is
/// short-circuited by directly invoking
/// .
+///
+/// Register row 32: one fact here calls StartAsync, which writes the
+/// process-global queue-depth provider slot, so this class joins
+/// to stay serialized against
+/// .
///
+[Collection(QueueDepthGaugeCollection.Name)]
public class CachedCallAttemptEmissionTests : IAsyncLifetime, IDisposable
{
private readonly TestLocalDb _localDb;
diff --git a/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/QueueDepthGaugeCollection.cs b/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/QueueDepthGaugeCollection.cs
new file mode 100644
index 00000000..3baf5298
--- /dev/null
+++ b/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/QueueDepthGaugeCollection.cs
@@ -0,0 +1,124 @@
+using System.Runtime.CompilerServices;
+using System.Text.RegularExpressions;
+
+namespace ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests;
+
+///
+/// Deferred-work register row 32 — isolation for the process-global StoreAndForward
+/// queue-depth gauge.
+///
+///
+/// scadabridge.store_and_forward.queue.depth is an
+/// created once on ScadaBridgeTelemetry's static meter, reading a single
+/// process-global provider slot (SetQueueDepthProvider /
+/// ClearQueueDepthProvider). Every StoreAndForwardService.StartAsync
+/// writes that one slot — last registrant wins. That is the PRODUCT design (one S&F
+/// engine per node), and three of ' facts assert
+/// exactly the slot-ownership semantics (compare-and-clear identity, stop/restart
+/// re-seed), so they cannot be rewritten against a per-test meter without changing
+/// what they prove — the global slot IS their subject.
+///
+///
+///
+/// The resource therefore needs mutual exclusion, not duplication: every test class in
+/// this assembly that starts a StoreAndForwardService (and so writes the slot)
+/// joins this collection, which xUnit runs serially with respect to itself. Classes
+/// that never start a service stay free to run in parallel. Before this, a concurrent
+/// StartAsync in a sibling class clobbered the gauge mid-assertion and
+/// Gauge_TracksBufferedDepth_AcrossEnqueueDrainAndPark read the sibling's depth
+/// (observed: expected 0, actual 6 — roughly 2 runs in 5 with
+/// parallelizeTestCollections: true).
+///
+///
+///
+/// keeps the membership honest: a new
+/// test class that starts a service without joining this collection fails the guard
+/// rather than silently reintroducing the flake.
+///
+///
+[CollectionDefinition(Name)]
+public sealed class QueueDepthGaugeCollection
+{
+ /// Collection name; referenced from [Collection(...)] on member classes.
+ public const string Name = "StoreAndForward queue-depth gauge (process-global provider slot)";
+}
+
+///
+/// Staleness self-test for : asserts that every
+/// source file in this test project which both constructs a StoreAndForwardService
+/// and calls StartAsync — i.e. writes the process-global queue-depth provider
+/// slot — declares [Collection(QueueDepthGaugeCollection.Name)].
+///
+///
+/// The project directory is resolved from (baked in
+/// at compile time by the compiler that built these sources), not from the test host's
+/// working directory or the output folder, so the scan finds the sources that produced
+/// this assembly.
+///
+///
+public class QueueDepthGaugeCollectionGuardTests
+{
+ /// Absolute path of this source file, captured at compile time.
+ private static string ThisFilePath([CallerFilePath] string path = "") => path;
+
+ /// Matches a construction of the service under a global-slot-writing test.
+ private static readonly Regex ConstructsService =
+ new(@"new\s+StoreAndForwardService\s*\(", RegexOptions.Compiled);
+
+ /// Matches a start call, which is what actually writes the global provider slot.
+ private static readonly Regex StartsService =
+ new(@"\.StartAsync\s*\(", RegexOptions.Compiled);
+
+ /// Matches the collection attribute that puts a class in the serialized group.
+ private static readonly Regex DeclaresCollection =
+ new(@"\[\s*Collection\s*\(\s*QueueDepthGaugeCollection\.Name\s*\)\s*\]", RegexOptions.Compiled);
+
+ [Fact]
+ public void EverySourceFileThatStartsAService_JoinsTheGaugeCollection()
+ {
+ var projectDir = Path.GetDirectoryName(ThisFilePath())!;
+ Assert.True(Directory.Exists(projectDir),
+ $"Guard could not locate the test project sources at '{projectDir}'. " +
+ "The [CallerFilePath] anchor must point at this project's directory.");
+
+ var offenders = new List();
+ var members = new List();
+
+ foreach (var file in Directory.EnumerateFiles(projectDir, "*.cs", SearchOption.AllDirectories))
+ {
+ // Skip generated output (bin/obj) and this guard's own definition file.
+ var relative = Path.GetRelativePath(projectDir, file);
+ if (relative.StartsWith("bin", StringComparison.Ordinal) ||
+ relative.StartsWith("obj", StringComparison.Ordinal))
+ {
+ continue;
+ }
+
+ var text = File.ReadAllText(file);
+ if (!ConstructsService.IsMatch(text) || !StartsService.IsMatch(text))
+ {
+ continue;
+ }
+
+ if (DeclaresCollection.IsMatch(text))
+ {
+ members.Add(relative);
+ }
+ else
+ {
+ offenders.Add(relative);
+ }
+ }
+
+ // Sanity: the scan must actually find files, or a rename has silently defanged it.
+ Assert.True(members.Count + offenders.Count > 0,
+ "Guard found no source file constructing and starting a StoreAndForwardService — " +
+ "the scan patterns are stale.");
+
+ Assert.True(offenders.Count == 0,
+ "These test sources start a StoreAndForwardService (writing the process-global " +
+ $"queue-depth provider slot) without joining [Collection(QueueDepthGaugeCollection.Name)]: " +
+ string.Join(", ", offenders) +
+ ". Add the attribute, or the queue-depth gauge assertions will flake under parallel collections.");
+ }
+}
diff --git a/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/QueueDepthGaugeTests.cs b/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/QueueDepthGaugeTests.cs
index 6b435f5f..a58524ad 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/QueueDepthGaugeTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/QueueDepthGaugeTests.cs
@@ -17,7 +17,13 @@ namespace ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests;
/// and does no I/O, which is the whole point of caching the count).
/// seeds the counter from storage and registers the provider against this service
/// instance, so the gauge resolves to this test's counter.
+///
+/// Register row 32: that provider slot is process-global, so this class joins
+/// along with every other class in this assembly
+/// that starts a service — see that type for why mutual exclusion (rather than a per-test
+/// meter) is the correct isolation here.
///
+[Collection(QueueDepthGaugeCollection.Name)]
public class QueueDepthGaugeTests : IAsyncLifetime, IDisposable
{
private readonly TestLocalDb _localDb;
diff --git a/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/StoreAndForwardServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/StoreAndForwardServiceTests.cs
index f24032d6..c18a8a74 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/StoreAndForwardServiceTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/StoreAndForwardServiceTests.cs
@@ -9,7 +9,13 @@ namespace ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests;
///
/// WP-10/12/13/14: Tests for the StoreAndForwardService retry engine and management.
+///
+/// Register row 32: several facts here call StartAsync, which writes the
+/// process-global queue-depth provider slot, so this class joins
+/// to stay serialized against
+/// .
///
+[Collection(QueueDepthGaugeCollection.Name)]
public class StoreAndForwardServiceTests : IAsyncLifetime, IDisposable
{
private readonly TestLocalDb _localDb;
diff --git a/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/xunit.runner.json b/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/xunit.runner.json
index 08c512b3..4c6a0fdf 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/xunit.runner.json
+++ b/tests/ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests/xunit.runner.json
@@ -1,4 +1,4 @@
{
"$schema": "https://xunit.net/schema/current/xunit.runner.schema.json",
- "parallelizeTestCollections": false
+ "parallelizeTestCollections": true
}