diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 6ab2b246..5aa6b8f5 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -97,6 +97,83 @@ Copy this block when adding a new issue: --- +## #252 — App test classes raced on process-global camera/render statics + +**Status:** DONE — 2026-07-28; serialized via `CameraDiagnosticsCollection` +**Severity:** LOW (test-infrastructure only; no production defect) +**Filed:** 2026-07-28 +**Component:** tests / xUnit parallelism + +**Description:** A full Release `AcDream.App.Tests` run failed once at +`Issue181WallPressEquilibriumTests.Diagnostic_WallPressedCamera_EyeWanderAndViewerCellStability`. +The test passed in isolation and did not recur across five further whole-suite +runs. The diff under test touched only the world texture-creation stack — +nothing in camera, visibility or physics — so a regression was never a +plausible mechanism. + +**Root cause:** Ten App test classes share three *process-global* mutable +statics. xUnit runs distinct test classes in parallel by default, so one +class's mutation window is observable by another class mid-test: + +- `AcDream.Core.Rendering.CameraDiagnostics` — `AlignToSlope`, + `CollideCamera`, `TranslationStiffness`, `RotationStiffness`, + `UseRetailChaseCamera`. `RetailChaseCameraTests` sets `AlignToSlope` and + `CollideCamera` to `false`, and three classes set `UseRetailChaseCamera` to + `false` — all *away from* their defaults. Read by `RetailChaseCamera.Update`, + `CameraController.Active`, `CameraFrameController`, `WorldRenderFrameBuilder` + and `MouseLookController`. +- `AcDream.Core.Rendering.RenderingDiagnostics.ProbeFlapEnabled` — written by + `CornerFloodReplayTests` and `Issue181WallPressEquilibriumTests`. +- `System.Console.Out` — redirected via `Console.SetOut` by those same two + classes to capture probe output. + +Every class saved and restored in `try`/`finally`, which is correct *within* a +class but not sufficient across classes. A `finally` bounds the mutation in +time along its own thread; it cannot stop a concurrent class from reading the +static inside that window. Two overlapping save/restore pairs can also +interleave so the second restore writes back the *first* one's temporary +value, leaving the global wrong for the remainder of the run. The +`Console.Out` case is the sharpest: an interleaved restore can install a +**disposed** `StringWriter` as the process-wide `Console.Out`, which then +throws in unrelated tests. + +**Fix:** `tests/AcDream.App.Tests/Rendering/CameraDiagnosticsCollection.cs` +adds a marker `[CollectionDefinition]` (no fixture — each member still needs +its own per-test values, several as `[Theory]` cases, so a fixture cannot own +the save/restore) applied to the ten sharing classes. Follows the existing +`WorldEnvironmentControllerCollection` precedent. No production code changed +and no assertion was weakened. Membership is deliberately narrow: classes that +merely construct a `CameraController` without a retail chase camera are not +members, because their reads are insensitive. + +**Verification:** Base commit `f6275f45` measured empirically at 3,763 passed / +3 skipped. Post-fix, 136 whole-suite Release runs; every failure observed was +in the pre-existing zero-allocation family (see #250) and none was in any +collection member. A matched 55-run baseline at `f6275f45` reproduced the same +zero-allocation family. Serialization cost is within run-to-run noise — the +suite is ~3 s of a ~4.5 s wall-clock `dotnet test`. + +**Files:** `tests/AcDream.App.Tests/Rendering/CameraDiagnosticsCollection.cs` +(new); `[Collection]` applied to `Issue181WallPressEquilibriumTests`, +`Issue181CameraParkStabilityTests`, `Issue177StairDescentCameraFloodTests`, +`RetailChaseCameraTests`, `CameraControllerTests`, +`WorldRenderFrameBuilderTests`, `CornerFloodReplayTests`, +`HouseExitWalkReplayTests`, `CameraFrameControllerTests`, +`MouseLookControllerTests`. + +**Follow-up (not fixed here):** `CameraDiagnostics` and `RenderingDiagnostics` +are `static` by design (the `PhysicsDiagnostics` runtime-toggle pattern). The +test-side collection is the correct fix for the observed race; making the +camera knobs an injectable instance carried by the camera would remove the +class of defect entirely, but that is production surgery and out of scope. +`AcDream.Core.Tests` has the same shape in a separate assembly (hence a +separate process, so it does not race with App): `CameraDiagnosticsTests` is +its only `CameraDiagnostics` writer, but four classes there call +`Console.SetOut` — `CellarLipWedgeTests`, `CameraCornerSealReplayTests`, +`Issue137CorridorSeamReplayTests`, `RenderingDiagnosticsVisibilityTests`. + +--- + ## #251 — glClientWaitSync returned 0 and crashed the render loop, once in nine connected runs **Status:** OPEN diff --git a/tests/AcDream.App.Tests/Input/MouseLookControllerTests.cs b/tests/AcDream.App.Tests/Input/MouseLookControllerTests.cs index 4d1702c7..6a8ad131 100644 --- a/tests/AcDream.App.Tests/Input/MouseLookControllerTests.cs +++ b/tests/AcDream.App.Tests/Input/MouseLookControllerTests.cs @@ -10,6 +10,7 @@ using Silk.NET.Input; namespace AcDream.App.Tests.Input; +[Collection(AcDream.App.Tests.Rendering.CameraDiagnosticsCollection.Name)] public sealed class MouseLookControllerTests { [Fact] diff --git a/tests/AcDream.App.Tests/Rendering/CameraControllerTests.cs b/tests/AcDream.App.Tests/Rendering/CameraControllerTests.cs index e84b20be..b8810c66 100644 --- a/tests/AcDream.App.Tests/Rendering/CameraControllerTests.cs +++ b/tests/AcDream.App.Tests/Rendering/CameraControllerTests.cs @@ -4,6 +4,7 @@ using Xunit; namespace AcDream.App.Tests.Rendering; +[Collection(CameraDiagnosticsCollection.Name)] public class CameraControllerTests { private static (CameraController ctl, ChaseCamera legacy, RetailChaseCamera retail) MakeChaseFixture() diff --git a/tests/AcDream.App.Tests/Rendering/CameraDiagnosticsCollection.cs b/tests/AcDream.App.Tests/Rendering/CameraDiagnosticsCollection.cs new file mode 100644 index 00000000..9f73f3cf --- /dev/null +++ b/tests/AcDream.App.Tests/Rendering/CameraDiagnosticsCollection.cs @@ -0,0 +1,66 @@ +using Xunit; + +namespace AcDream.App.Tests.Rendering; + +/// +/// Serializes every test class that shares the camera/render family of +/// process-global mutable statics. xUnit runs distinct test classes in +/// PARALLEL by default, and these statics live on the process, not on the test +/// — so two classes running concurrently observe each other's writes. +/// +/// +/// The per-class try/finally save/restore blocks are correct +/// within a class and remain necessary — but they are not sufficient. +/// A finally only bounds the mutation in TIME along its own thread; it +/// cannot stop another class from reading the static inside that window. Worse, +/// two overlapping save/restore pairs can interleave so the second restore +/// writes back the FIRST one's temporary value, leaving the global permanently +/// wrong for the rest of the run. Serializing the sharers is what actually makes +/// each class's finally sufficient. +/// +/// +/// Three distinct globals are covered, and every one of them is a shared +/// edge between at least two classes in this collection: +/// +/// AcDream.Core.Rendering.CameraDiagnostics — +/// AlignToSlope, CollideCamera, TranslationStiffness, +/// RotationStiffness, UseRetailChaseCamera. Read by +/// RetailChaseCamera.Update, CameraController.Active, +/// CameraFrameController, WorldRenderFrameBuilder and +/// MouseLookController. +/// AcDream.Core.Rendering.RenderingDiagnostics.ProbeFlapEnabled +/// — written by CornerFloodReplayTests and +/// Issue181WallPressEquilibriumTests. +/// System.Console.Out — redirected via +/// Console.SetOut by those same two classes to capture probe output. +/// Interleaved redirection can restore a DISPOSED StringWriter as the +/// process-wide Console.Out, which then throws in unrelated +/// tests. +/// +/// +/// +/// Membership is deliberately narrow: only classes that write one of these +/// globals, or that drive production code which READS a knob another member +/// mutates away from its default (RetailChaseCameraTests sets +/// AlignToSlope and CollideCamera to ; +/// the UseRetailChaseCamera writers set it to ). +/// Classes that merely construct a CameraController without a retail +/// chase camera are NOT members — their reads are insensitive. +/// +/// +/// +/// Marker only, no collection fixture: each member still needs its own +/// per-test values (several are [Theory] cases that set different knobs +/// per case), so a fixture cannot own the save/restore without rewriting every +/// member's internals. The existing per-test finally blocks already +/// return the defaults; this attribute supplies the mutual exclusion they were +/// missing. Follows the WorldEnvironmentControllerCollection precedent. +/// +/// +/// Evidence + history: docs/ISSUES.md #251. +/// +[CollectionDefinition(Name)] +public sealed class CameraDiagnosticsCollection +{ + public const string Name = "Camera diagnostics globals"; +} diff --git a/tests/AcDream.App.Tests/Rendering/CameraFrameControllerTests.cs b/tests/AcDream.App.Tests/Rendering/CameraFrameControllerTests.cs index 732906b3..3cc6580b 100644 --- a/tests/AcDream.App.Tests/Rendering/CameraFrameControllerTests.cs +++ b/tests/AcDream.App.Tests/Rendering/CameraFrameControllerTests.cs @@ -7,6 +7,7 @@ using AcDream.Core.Physics; namespace AcDream.App.Tests.Rendering; +[Collection(CameraDiagnosticsCollection.Name)] public sealed class CameraFrameControllerTests { [Fact] diff --git a/tests/AcDream.App.Tests/Rendering/CornerFloodReplayTests.cs b/tests/AcDream.App.Tests/Rendering/CornerFloodReplayTests.cs index 3d5a8213..351b356b 100644 --- a/tests/AcDream.App.Tests/Rendering/CornerFloodReplayTests.cs +++ b/tests/AcDream.App.Tests/Rendering/CornerFloodReplayTests.cs @@ -30,6 +30,7 @@ namespace AcDream.App.Tests.Rendering; /// corner press: (159.94, 7.70, 94.00) in 0172; captured eyes hover near (157.4..157.5, /// 7.91, 96.25) — inside 0171, 0.35 m past the doorway plane, near the ceiling. /// +[Collection(CameraDiagnosticsCollection.Name)] public class CornerFloodReplayTests { private readonly ITestOutputHelper _out; diff --git a/tests/AcDream.App.Tests/Rendering/HouseExitWalkReplayTests.cs b/tests/AcDream.App.Tests/Rendering/HouseExitWalkReplayTests.cs index e0c93dd8..ab1f1604 100644 --- a/tests/AcDream.App.Tests/Rendering/HouseExitWalkReplayTests.cs +++ b/tests/AcDream.App.Tests/Rendering/HouseExitWalkReplayTests.cs @@ -48,6 +48,7 @@ namespace AcDream.App.Tests.Rendering; /// exactly as GameWindow builds it (full-screen /// OutsideView ⇒ outdoor dynamics trivially cone-pass). /// +[Collection(CameraDiagnosticsCollection.Name)] public class HouseExitWalkReplayTests { private readonly ITestOutputHelper _out; diff --git a/tests/AcDream.App.Tests/Rendering/Issue177StairDescentCameraFloodTests.cs b/tests/AcDream.App.Tests/Rendering/Issue177StairDescentCameraFloodTests.cs index 2652301b..5cf4e472 100644 --- a/tests/AcDream.App.Tests/Rendering/Issue177StairDescentCameraFloodTests.cs +++ b/tests/AcDream.App.Tests/Rendering/Issue177StairDescentCameraFloodTests.cs @@ -31,6 +31,7 @@ namespace AcDream.App.Tests.Rendering; /// and the printed per-portal side-test D names the failing gate. If they stay /// admitted, the flood is exonerated and the artifact lives downstream. /// +[Collection(CameraDiagnosticsCollection.Name)] public class Issue177StairDescentCameraFloodTests { private const uint FacilityHub = 0x8A020000u; diff --git a/tests/AcDream.App.Tests/Rendering/Issue181CameraParkStabilityTests.cs b/tests/AcDream.App.Tests/Rendering/Issue181CameraParkStabilityTests.cs index 3e2950db..14172fdb 100644 --- a/tests/AcDream.App.Tests/Rendering/Issue181CameraParkStabilityTests.cs +++ b/tests/AcDream.App.Tests/Rendering/Issue181CameraParkStabilityTests.cs @@ -19,6 +19,7 @@ namespace AcDream.App.Tests.Rendering; /// from its INPUTS (player position/yaw jitter out of GameWindow); if it /// wobbles here, the camera loop itself fails to reach the fixed point. /// +[Collection(CameraDiagnosticsCollection.Name)] public class Issue181CameraParkStabilityTests { private readonly ITestOutputHelper _out; diff --git a/tests/AcDream.App.Tests/Rendering/Issue181WallPressEquilibriumTests.cs b/tests/AcDream.App.Tests/Rendering/Issue181WallPressEquilibriumTests.cs index bd57825b..0234a96c 100644 --- a/tests/AcDream.App.Tests/Rendering/Issue181WallPressEquilibriumTests.cs +++ b/tests/AcDream.App.Tests/Rendering/Issue181WallPressEquilibriumTests.cs @@ -31,6 +31,7 @@ namespace AcDream.App.Tests.Rendering; /// Diagnostic (reporting) first; the equilibrium fix turns the wander/flap /// numbers into hard pins. /// +[Collection(CameraDiagnosticsCollection.Name)] public class Issue181WallPressEquilibriumTests { private const uint FacilityHubLandblock = 0x8A020000u; diff --git a/tests/AcDream.App.Tests/Rendering/RetailChaseCameraTests.cs b/tests/AcDream.App.Tests/Rendering/RetailChaseCameraTests.cs index 32675ee3..653a5182 100644 --- a/tests/AcDream.App.Tests/Rendering/RetailChaseCameraTests.cs +++ b/tests/AcDream.App.Tests/Rendering/RetailChaseCameraTests.cs @@ -6,6 +6,7 @@ using Xunit; namespace AcDream.App.Tests.Rendering; +[Collection(CameraDiagnosticsCollection.Name)] public class RetailChaseCameraTests { [Fact] diff --git a/tests/AcDream.App.Tests/Rendering/WorldRenderFrameBuilderTests.cs b/tests/AcDream.App.Tests/Rendering/WorldRenderFrameBuilderTests.cs index 6bf373bf..a2812357 100644 --- a/tests/AcDream.App.Tests/Rendering/WorldRenderFrameBuilderTests.cs +++ b/tests/AcDream.App.Tests/Rendering/WorldRenderFrameBuilderTests.cs @@ -11,6 +11,7 @@ using AcDream.Core.World.Cells; namespace AcDream.App.Tests.Rendering; +[Collection(CameraDiagnosticsCollection.Name)] public sealed class WorldRenderFrameBuilderTests { [Fact]