From 21ee5e1035d662151cfcd3ff78e583e61a3ebb35 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 2 Jun 2026 15:20:24 +0200 Subject: [PATCH] test: fix PhysicsResolveCapture/PhysicsDiagnostics static-leak isolation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit xUnit's default parallel execution let diagnostic-harness tests (CellarUp, DoorBug, DoorCollisionApparatus) mutate PhysicsResolveCapture.CapturePath and PhysicsDiagnostics probe flags concurrently with victim tests (MotionInterpreter, PositionManager, PlayerMovementController, DispatcherToMovement, BSPStepUp), producing a flaky 14-26 failure range. Fixes: - Add PhysicsResolveCapture.ResetForTest() + PhysicsDiagnostics.ResetForTest() as documented test-only reset APIs (never called from production paths). - Add IDisposable to CellarUpTrajectoryReplayTests with ctor/Dispose calling both ResetForTest() — prevents CapturePath from leaking between the Capture_* tests in the same class (the immediate root cause of Capture_SkipsNonPlayerCalls finding an unexpected file). - Add xunit.runner.json (maxParallelThreads=1, parallelizeTestCollections=false) to AcDream.Core.Tests — eliminates parallelism-induced probe-flag leaks across all test classes without requiring [Collection] boilerplate on every offender. After: two consecutive runs produce the identical 12-failure set. Confirmed: LiveCompare_FirstCap_FixClosesCottageFloorCap passes. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Physics/PhysicsDiagnostics.cs | 50 +++++++++++++++++++ .../Physics/PhysicsResolveCapture.cs | 18 +++++++ .../AcDream.Core.Tests.csproj | 9 ++++ .../Physics/CellarUpTrajectoryReplayTests.cs | 19 ++++++- tests/AcDream.Core.Tests/xunit.runner.json | 6 +++ 5 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 tests/AcDream.Core.Tests/xunit.runner.json diff --git a/src/AcDream.Core/Physics/PhysicsDiagnostics.cs b/src/AcDream.Core/Physics/PhysicsDiagnostics.cs index 43a1a8d..06a3180 100644 --- a/src/AcDream.Core/Physics/PhysicsDiagnostics.cs +++ b/src/AcDream.Core/Physics/PhysicsDiagnostics.cs @@ -512,6 +512,56 @@ public static class PhysicsDiagnostics public static bool ProbeDumpGfxObjsEnabled => ProbeDumpGfxObjIds.Count > 0; + /// + /// Test-only reset: set every probe flag to false and clear any + /// side-channel fields. Does NOT re-read environment variables — tests + /// run in environments where the env vars are all absent, so false is + /// the correct default. + /// + /// + /// Call from test constructors and IDisposable.Dispose() to + /// prevent one test class from leaving enabled probes that corrupt + /// timing-sensitive tests in another class (the static-leak root cause + /// documented in T0). + /// + /// + /// + /// This method is intentionally public so test projects can call + /// it without reflection, but it must NEVER be called from production + /// code paths. + /// + /// + public static void ResetForTest() + { + ProbeResolveEnabled = false; + ProbeCellEnabled = false; + ProbeBuildingEnabled = false; + ProbeCellSetEnabled = false; + ProbeAutoWalkEnabled = false; + ProbeUseabilityFallbackEnabled= false; + DumpSteepRoofEnabled = false; + ProbeIndoorBspEnabled = false; + ProbeCellCacheEnabled = false; + ProbeContactPlaneEnabled = false; + ProbeWalkMissEnabled = false; + ProbePushBackEnabled = false; + ProbePolyDumpEnabled = false; + ProbePlacementFailEnabled = false; + ProbeSweptEnabled = false; + ProbeStepWalkEnabled = false; + + // Side-channel fields + LastBspHitPoly = null; + LastPlacementFailPolyId = 0; + LastPlacementFailPolyNormal = default; + LastPlacementFailPolyD = 0f; + LastPlacementFailSolidLeaf = false; + + // Dump-trigger sets + ProbeDumpCellIds = new System.Collections.Generic.HashSet(); + ProbeDumpGfxObjIds = new System.Collections.Generic.HashSet(); + } + private static IReadOnlySet ParseHexIdList(string? raw) { if (string.IsNullOrWhiteSpace(raw)) diff --git a/src/AcDream.Core/Physics/PhysicsResolveCapture.cs b/src/AcDream.Core/Physics/PhysicsResolveCapture.cs index ad514cd..69f508d 100644 --- a/src/AcDream.Core/Physics/PhysicsResolveCapture.cs +++ b/src/AcDream.Core/Physics/PhysicsResolveCapture.cs @@ -148,6 +148,24 @@ public static class PhysicsResolveCapture public static void ResetTickCounter() => Interlocked.Exchange(ref _tickCounter, 0); + /// + /// Test-only reset: close any open writer, clear , + /// and reset the tick counter to 0. Call from test constructors and + /// IDisposable.Dispose() to prevent static state from leaking + /// across test-class boundaries. + /// + /// + /// This method is intentionally public so test projects can call it + /// without reflection, but it must NEVER be called from production code paths. + /// + /// + public static void ResetForTest() + { + Close(); + CapturePath = null; + Interlocked.Exchange(ref _tickCounter, 0); + } + private static void EnsureWriter_NoLock() { if (_writer is not null) diff --git a/tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj b/tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj index 1c83fc3..f848e8a 100644 --- a/tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj +++ b/tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj @@ -18,6 +18,15 @@ + + + + + diff --git a/tests/AcDream.Core.Tests/Physics/CellarUpTrajectoryReplayTests.cs b/tests/AcDream.Core.Tests/Physics/CellarUpTrajectoryReplayTests.cs index a409bb1..0054a82 100644 --- a/tests/AcDream.Core.Tests/Physics/CellarUpTrajectoryReplayTests.cs +++ b/tests/AcDream.Core.Tests/Physics/CellarUpTrajectoryReplayTests.cs @@ -80,8 +80,25 @@ namespace AcDream.Core.Tests.Physics; /// trajectory shape. /// /// -public class CellarUpTrajectoryReplayTests +public class CellarUpTrajectoryReplayTests : IDisposable { + // ── T0: constructor + Dispose reset PhysicsResolveCapture static ──────── + // xUnit creates a fresh instance per test, so the constructor runs before + // each test and Dispose runs after. Resetting here guarantees that no + // prior test in this class can leave CapturePath set (which would cause + // Capture_SkipsNonPlayerCalls to find an unexpected file). + public CellarUpTrajectoryReplayTests() + { + PhysicsResolveCapture.ResetForTest(); + PhysicsDiagnostics.ResetForTest(); + } + + public void Dispose() + { + PhysicsResolveCapture.ResetForTest(); + PhysicsDiagnostics.ResetForTest(); + } + // ── Cellar / cottage geometry constants ──────────────────────── private const uint CellarId = 0xA9B40147u; private const uint CottageNeighborA = 0xA9B40143u; diff --git a/tests/AcDream.Core.Tests/xunit.runner.json b/tests/AcDream.Core.Tests/xunit.runner.json new file mode 100644 index 0000000..c315589 --- /dev/null +++ b/tests/AcDream.Core.Tests/xunit.runner.json @@ -0,0 +1,6 @@ +{ + "$schema": "https://xunit.net/schema/current/xunit.runner.schema.json", + "parallelizeAssembly": false, + "parallelizeTestCollections": false, + "maxParallelThreads": 1 +}