test(storeandforward): isolate the process-global queue-depth gauge — register row 32
QueueDepthGaugeTests failed intermittently whenever a sibling test class started a StoreAndForwardService concurrently. `scadabridge.store_and_forward.queue.depth` is an ObservableGauge on ScadaBridgeTelemetry's static meter reading ONE process-global provider slot; every StartAsync writes it, last registrant wins. A concurrent StartAsync clobbered the gauge mid-assertion and the test read the sibling's depth (observed: expected 0, actual 6 — 2 runs in 5 with parallel collections enabled). The global slot cannot be swapped for a per-test meter: three of the six facts assert the slot-ownership semantics themselves (compare-and-clear identity, stop/restart re-seed), so the slot IS their subject. The correct isolation is mutual exclusion — QueueDepthGaugeTests, StoreAndForwardServiceTests and CachedCallAttemptEmissionTests (the three classes that call StartAsync) now share the QueueDepthGaugeCollection, which xUnit runs serially; the other eight classes stay parallel. With real isolation in place the assembly-wide `parallelizeTestCollections: false` blanket switch is no longer needed and is flipped back on, so the isolation is actually exercised rather than papered over. QueueDepthGaugeCollectionGuardTests is a staleness self-test: it scans this project's sources (anchored by [CallerFilePath], not the output dir) for files that both construct and start a service, and fails if one omits the collection attribute — so a new test class cannot silently reintroduce the flake. Negative-controlled: dropping the attribute from StoreAndForwardServiceTests makes the guard fail with the offending file name, and the original gauge flake reappears in the same run. Verification: 8 consecutive full-suite runs green with parallel collections ON (135 tests), plus 5 more after the negative control was reverted. Test-only change; no src/ involvement.
This commit is contained in:
Joseph Doherty
@@ -17,7 +17,13 @@ namespace ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests;
|
||||
/// <c>DefaultRetryInterval=Zero</c> so the timer-driven retry sweep is
|
||||
/// short-circuited by directly invoking
|
||||
/// <see cref="StoreAndForwardService.RetryPendingMessagesAsync"/>.
|
||||
///
|
||||
/// Register row 32: one fact here calls <c>StartAsync</c>, which writes the
|
||||
/// process-global queue-depth provider slot, so this class joins
|
||||
/// <see cref="QueueDepthGaugeCollection"/> to stay serialized against
|
||||
/// <see cref="QueueDepthGaugeTests"/>.
|
||||
/// </summary>
|
||||
[Collection(QueueDepthGaugeCollection.Name)]
|
||||
public class CachedCallAttemptEmissionTests : IAsyncLifetime, IDisposable
|
||||
{
|
||||
private readonly TestLocalDb _localDb;
|
||||
|
||||
@@ -0,0 +1,124 @@
|
||||
using System.Runtime.CompilerServices;
|
||||
using System.Text.RegularExpressions;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Deferred-work register row 32 — isolation for the process-global StoreAndForward
|
||||
/// queue-depth gauge.
|
||||
///
|
||||
/// <para>
|
||||
/// <c>scadabridge.store_and_forward.queue.depth</c> is an <see cref="System.Diagnostics.Metrics.ObservableGauge{T}"/>
|
||||
/// created once on <c>ScadaBridgeTelemetry</c>'s static meter, reading a single
|
||||
/// process-global provider slot (<c>SetQueueDepthProvider</c> /
|
||||
/// <c>ClearQueueDepthProvider</c>). Every <c>StoreAndForwardService.StartAsync</c>
|
||||
/// writes that one slot — last registrant wins. That is the PRODUCT design (one S&F
|
||||
/// engine per node), and three of <see cref="QueueDepthGaugeTests"/>' 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.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// The resource therefore needs mutual exclusion, not duplication: every test class in
|
||||
/// this assembly that starts a <c>StoreAndForwardService</c> (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
|
||||
/// <c>StartAsync</c> in a sibling class clobbered the gauge mid-assertion and
|
||||
/// <c>Gauge_TracksBufferedDepth_AcrossEnqueueDrainAndPark</c> read the sibling's depth
|
||||
/// (observed: expected 0, actual 6 — roughly 2 runs in 5 with
|
||||
/// <c>parallelizeTestCollections: true</c>).
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// <see cref="QueueDepthGaugeCollectionGuardTests"/> 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.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
[CollectionDefinition(Name)]
|
||||
public sealed class QueueDepthGaugeCollection
|
||||
{
|
||||
/// <summary>Collection name; referenced from <c>[Collection(...)]</c> on member classes.</summary>
|
||||
public const string Name = "StoreAndForward queue-depth gauge (process-global provider slot)";
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Staleness self-test for <see cref="QueueDepthGaugeCollection"/>: asserts that every
|
||||
/// source file in this test project which both constructs a <c>StoreAndForwardService</c>
|
||||
/// and calls <c>StartAsync</c> — i.e. writes the process-global queue-depth provider
|
||||
/// slot — declares <c>[Collection(QueueDepthGaugeCollection.Name)]</c>.
|
||||
///
|
||||
/// <para>
|
||||
/// The project directory is resolved from <see cref="CallerFilePathAttribute"/> (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.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public class QueueDepthGaugeCollectionGuardTests
|
||||
{
|
||||
/// <summary>Absolute path of this source file, captured at compile time.</summary>
|
||||
private static string ThisFilePath([CallerFilePath] string path = "") => path;
|
||||
|
||||
/// <summary>Matches a construction of the service under a global-slot-writing test.</summary>
|
||||
private static readonly Regex ConstructsService =
|
||||
new(@"new\s+StoreAndForwardService\s*\(", RegexOptions.Compiled);
|
||||
|
||||
/// <summary>Matches a start call, which is what actually writes the global provider slot.</summary>
|
||||
private static readonly Regex StartsService =
|
||||
new(@"\.StartAsync\s*\(", RegexOptions.Compiled);
|
||||
|
||||
/// <summary>Matches the collection attribute that puts a class in the serialized group.</summary>
|
||||
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<string>();
|
||||
var members = new List<string>();
|
||||
|
||||
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.");
|
||||
}
|
||||
}
|
||||
@@ -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). <see cref="StartAsync"/>
|
||||
/// 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
|
||||
/// <see cref="QueueDepthGaugeCollection"/> 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.
|
||||
/// </summary>
|
||||
[Collection(QueueDepthGaugeCollection.Name)]
|
||||
public class QueueDepthGaugeTests : IAsyncLifetime, IDisposable
|
||||
{
|
||||
private readonly TestLocalDb _localDb;
|
||||
|
||||
@@ -9,7 +9,13 @@ namespace ZB.MOM.WW.ScadaBridge.StoreAndForward.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// WP-10/12/13/14: Tests for the StoreAndForwardService retry engine and management.
|
||||
///
|
||||
/// Register row 32: several facts here call <c>StartAsync</c>, which writes the
|
||||
/// process-global queue-depth provider slot, so this class joins
|
||||
/// <see cref="QueueDepthGaugeCollection"/> to stay serialized against
|
||||
/// <see cref="QueueDepthGaugeTests"/>.
|
||||
/// </summary>
|
||||
[Collection(QueueDepthGaugeCollection.Name)]
|
||||
public class StoreAndForwardServiceTests : IAsyncLifetime, IDisposable
|
||||
{
|
||||
private readonly TestLocalDb _localDb;
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
{
|
||||
"$schema": "https://xunit.net/schema/current/xunit.runner.schema.json",
|
||||
"parallelizeTestCollections": false
|
||||
"parallelizeTestCollections": true
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user