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 58f23c41..7028e886 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentWarmThenGateTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentWarmThenGateTests.cs @@ -248,15 +248,20 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable "BatchTwo", ConfigJson("BatchTwo", sharedCode), "d2", "h2", true); SiteScriptCompileCache.Clear(); - Assert.Equal(0, SiteScriptCompileCache.Hits); + // Baseline rather than Assert.Equal(0, Hits): Hits is a process-global counter and test + // classes outside this collection compile scripts in parallel, so a foreign hit can land + // between the Clear() and the read (deferred-work register row 91). Everything below is + // asserted as a delta over this baseline. + var hitsBefore = SiteScriptCompileCache.Hits; CreateDeploymentManager(); AwaitAssert(() => { // One compile, then repeated hits: the second pre-warm plus both PreStarts. - Assert.True(SiteScriptCompileCache.Hits >= 3, - $"expected the pre-warmed body to be served from cache, saw {SiteScriptCompileCache.Hits} hits"); + Assert.True(SiteScriptCompileCache.Hits - hitsBefore >= 3, + $"expected the pre-warmed body to be served from cache, saw " + + $"{SiteScriptCompileCache.Hits - hitsBefore} hits over baseline {hitsBefore}"); }, TimeSpan.FromSeconds(20)); } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/SiteScriptCompileCacheTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/SiteScriptCompileCacheTests.cs index d25f7e9c..125b415b 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/SiteScriptCompileCacheTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Scripts/SiteScriptCompileCacheTests.cs @@ -7,39 +7,98 @@ namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Scripts; /// /// Tests for the process-wide . The cache is /// static/process-wide state, so this class shares the "SiteScriptCompileCache" xunit -/// collection with ScriptCompilationServiceTests (no cross-class parallelism) and -/// calls at the top of each test. +/// collection with ScriptCompilationServiceTests and DeploymentWarmThenGateTests +/// (no cross-class parallelism between those three, so no concurrent +/// ). /// +/// +/// +/// Deferred-work register row 91. That collection does NOT stop the rest of the +/// assembly: any test class that compiles a script — Instance/Script actor tests, deployment +/// tests — goes through too, in parallel, and +/// bumps the same process-global / +/// . Absolute assertions such as +/// Assert.Equal(1, Hits) after a Clear() therefore only held while nothing else +/// happened to be compiling; they failed roughly 1 run in 4 of a full parallel solution run and +/// were always green in isolation. +/// +/// +/// The fix follows the register's row-74 pattern: assert the DELTA attributable to this test's +/// OWN keys across its own scoped action, never an absolute global value. Two habits carry it: +/// every test keys its cache entries on a per-test-instance unique token (), +/// so no other class can collide with or serve them; and the global counters are only ever +/// asserted as one-sided bounds (>= baseline + n), since foreign traffic can only add. +/// A collection fixture spanning every script-compiling class in the assembly would fix it too, +/// but would serialise most of the suite for the benefit of five facts. +/// +/// [Collection("SiteScriptCompileCache")] public class SiteScriptCompileCacheTests { + /// + /// Slack allowed on bounds for entries inserted + /// by test classes outside this collection while a bound test runs (register row 91). The + /// exactness of the eviction claims comes from the per-key probes, not from this bound. + /// + private const int ForeignEntrySlack = 512; + + /// Per-test-instance token making every cache key in this test uniquely ours. + private readonly string _uniq = Guid.NewGuid().ToString("N"); + + /// Builds a cache key body that no other test class in the assembly can produce. + private string Code(string body) => $"/*{_uniq}*/ {body}"; + private static ScriptCompilationResult Ok() => ScriptCompilationResult.Succeeded(CSharpScript.Create("1", ScriptOptions.Default, typeof(ScriptGlobals))); [Fact] public void GetOrAdd_SameCodeAndGlobals_ComputesOnce() { - SiteScriptCompileCache.Clear(); + var code = Code("return 1;"); + var hitsBefore = SiteScriptCompileCache.Hits; var factoryCalls = 0; ScriptCompilationResult Factory() { factoryCalls++; return Ok(); } - var r1 = SiteScriptCompileCache.GetOrAdd("return 1;", typeof(ScriptGlobals), Factory); - var r2 = SiteScriptCompileCache.GetOrAdd("return 1;", typeof(ScriptGlobals), Factory); + var r1 = SiteScriptCompileCache.GetOrAdd(code, typeof(ScriptGlobals), Factory); + var r2 = SiteScriptCompileCache.GetOrAdd(code, typeof(ScriptGlobals), Factory); + // Scoped to our own key, so these two are exact and are the real claim: one compile, + // and the second lookup returned that very instance rather than a re-computation. Assert.Equal(1, factoryCalls); Assert.Same(r1, r2); - Assert.Equal(1, SiteScriptCompileCache.Hits); + + // Hits is process-global; a parallel class's compile can only push the delta up, so the + // claim "the second lookup was counted as a hit" is a lower bound, not an equality. + Assert.True(SiteScriptCompileCache.Hits >= hitsBefore + 1, + $"expected the second GetOrAdd to register a hit; hits went {hitsBefore} -> {SiteScriptCompileCache.Hits}"); } [Fact] public void GetOrAdd_SameCode_DifferentGlobals_AreSeparateEntries() { - SiteScriptCompileCache.Clear(); - SiteScriptCompileCache.GetOrAdd("1 > 0", typeof(ScriptGlobals), Ok); - SiteScriptCompileCache.GetOrAdd("1 > 0", typeof(TriggerExpressionGlobals), Ok); + var code = Code("1 > 0"); + var scriptCompiles = 0; + var triggerCompiles = 0; - Assert.Equal(2, SiteScriptCompileCache.Count); - Assert.Equal(0, SiteScriptCompileCache.Hits); // no cross-globals hit + var script = SiteScriptCompileCache.GetOrAdd(code, typeof(ScriptGlobals), + () => { scriptCompiles++; return Ok(); }); + var trigger = SiteScriptCompileCache.GetOrAdd(code, typeof(TriggerExpressionGlobals), + () => { triggerCompiles++; return Ok(); }); + + // "No cross-globals hit" asserted where it is actually observable and ours: the second + // lookup had to compute, and the two globals surfaces hold distinct entries. (The old + // form — Count == 2 and Hits == 0 on the process-global counters — was only true while + // no other class was compiling.) + Assert.Equal(1, scriptCompiles); + Assert.Equal(1, triggerCompiles); + Assert.NotSame(script, trigger); + + // Each entry is independently addressable afterwards: re-getting either must serve the + // matching cached result, never the other globals surface's. + Assert.Same(script, SiteScriptCompileCache.GetOrAdd(code, typeof(ScriptGlobals), + () => throw new InvalidOperationException("script entry was not cached"))); + Assert.Same(trigger, SiteScriptCompileCache.GetOrAdd(code, typeof(TriggerExpressionGlobals), + () => throw new InvalidOperationException("trigger entry was not cached"))); } [Fact] @@ -47,9 +106,22 @@ public class SiteScriptCompileCacheTests { SiteScriptCompileCache.Clear(); for (var i = 0; i <= SiteScriptCompileCache.MaxEntries; i++) - SiteScriptCompileCache.GetOrAdd($"return {i};", typeof(ScriptGlobals), Ok); + SiteScriptCompileCache.GetOrAdd(Code($"return {i};"), typeof(ScriptGlobals), Ok); - Assert.True(SiteScriptCompileCache.Count <= SiteScriptCompileCache.MaxEntries); + // MaxEntries + 1 distinct inserts must not all be resident — the sweep ran. The bound + // carries slack for foreign inserts (register row 91); the sharp claim is the eviction + // probe below, which is scoped to keys only this test can produce. + Assert.True(SiteScriptCompileCache.Count <= SiteScriptCompileCache.MaxEntries + ForeignEntrySlack, + $"cache grew past its bound: {SiteScriptCompileCache.Count} vs max {SiteScriptCompileCache.MaxEntries}"); + + var recomputed = 0; + for (var i = 0; i < 16; i++) + { + var code = Code($"return {i};"); + SiteScriptCompileCache.GetOrAdd(code, typeof(ScriptGlobals), () => { recomputed++; return Ok(); }); + } + + Assert.True(recomputed > 0, "no oldest-batch entry was evicted — the overflow sweep did not run"); } // ── WP3.1: approximate LRU replaced the wholesale-Clear overflow cliff ────────── @@ -70,15 +142,15 @@ public class SiteScriptCompileCacheTests // Fill to the bound. Entries 1..N are inserted oldest-first. for (var i = 0; i < SiteScriptCompileCache.MaxEntries; i++) - SiteScriptCompileCache.GetOrAdd($"return {i};", typeof(ScriptGlobals), Ok); + SiteScriptCompileCache.GetOrAdd(Code($"return {i};"), typeof(ScriptGlobals), Ok); // Touch entry 0 so it carries the NEWEST access stamp despite being inserted first. - var hot = SiteScriptCompileCache.GetOrAdd("return 0;", typeof(ScriptGlobals), Ok); + var hot = SiteScriptCompileCache.GetOrAdd(Code("return 0;"), typeof(ScriptGlobals), Ok); var hitsAfterTouch = SiteScriptCompileCache.Hits; // Push well past the bound so at least one eviction sweep definitely runs. for (var i = 0; i < 200; i++) - SiteScriptCompileCache.GetOrAdd($"return overflow{i};", typeof(ScriptGlobals), Ok); + SiteScriptCompileCache.GetOrAdd(Code($"return overflow{i};"), typeof(ScriptGlobals), Ok); // Nothing was cleared wholesale: the cache is still most of the way full and the hit // counter survived (Clear() would have reset it to 0). @@ -89,16 +161,20 @@ public class SiteScriptCompileCacheTests // The touched entry survived the sweep — a hit, not a recompile. This is the LRU // property: recency, not insertion order, decides what stays. var hitsBefore = SiteScriptCompileCache.Hits; - var again = SiteScriptCompileCache.GetOrAdd("return 0;", typeof(ScriptGlobals), + var again = SiteScriptCompileCache.GetOrAdd(Code("return 0;"), typeof(ScriptGlobals), () => throw new InvalidOperationException("hot entry was evicted — LRU is not keeping recently-used entries")); + // Assert.Same is the sharp claim (our own key, our own instance). The counter check is a + // lower bound: Hits is process-global and a parallel class's hit lands in the same window + // (register row 91 — this was `Assert.Equal(hitsBefore + 1, …)`). Assert.Same(hot, again); - Assert.Equal(hitsBefore + 1, SiteScriptCompileCache.Hits); + Assert.True(SiteScriptCompileCache.Hits >= hitsBefore + 1, + $"expected the surviving hot entry to register a hit; hits went {hitsBefore} -> {SiteScriptCompileCache.Hits}"); // …and untouched old entries genuinely were evicted, so the sweep really ran. var recomputed = 0; for (var i = 1; i <= 16; i++) { - var code = $"return {i};"; + var code = Code($"return {i};"); SiteScriptCompileCache.GetOrAdd(code, typeof(ScriptGlobals), () => { recomputed++; return Ok(); }); } @@ -119,7 +195,7 @@ public class SiteScriptCompileCacheTests var tasks = Enumerable.Range(0, 8).Select(t => Task.Run(() => { for (var i = 0; i < perTask; i++) - SiteScriptCompileCache.GetOrAdd($"return {t}_{i};", typeof(ScriptGlobals), Ok); + SiteScriptCompileCache.GetOrAdd(Code($"return {t}_{i};"), typeof(ScriptGlobals), Ok); })); await Task.WhenAll(tasks); @@ -131,7 +207,7 @@ public class SiteScriptCompileCacheTests // eviction bug would show up here as unbounded growth, not as a handful of extra rows. // (Other test classes compile into this process-wide cache concurrently, which is the // other reason the bound is asserted with slack rather than exactly.) - Assert.True(SiteScriptCompileCache.Count <= SiteScriptCompileCache.MaxEntries + 512, + Assert.True(SiteScriptCompileCache.Count <= SiteScriptCompileCache.MaxEntries + ForeignEntrySlack, $"cache grew past its bound: {SiteScriptCompileCache.Count} vs max {SiteScriptCompileCache.MaxEntries} " + $"after {8 * perTask} distinct inserts"); }