test: audit helper-mediated source reads

This commit is contained in:
Erik 2026-08-18 14:18:00 +02:00
parent dc94b0fe32
commit c5f0fbaaa4
2 changed files with 135 additions and 36 deletions

View file

@ -38,7 +38,7 @@ pwsh ./tools/audit-test-inventory.ps1
The generated JSON lives under ignored `artifacts/test-audit/`; it is not a
second 6.8 MB checked-in source of truth. The script and this reviewed ledger
are durable, while paths and line numbers regenerate after every batch. The
inventory refreshed for Batch L reports:
inventory refreshed through Batch U reports:
| Syntax signal | Current count |
|---|---:|
@ -56,7 +56,7 @@ inventory refreshed for Batch L reports:
| Methods directly using `Thread.Sleep` / `Task.Delay` | 15 / 14 |
| Cancellable-infinite-only / elapsed-time methods | 5 / 24 |
| Methods directly reading environment variables | 47 |
| Methods directly reading `.cs` source text | 63 |
| Methods directly / through same-file helpers reading `.cs` source text | 67 / 107 |
The three remaining prerequisite-return candidates are all reviewed branch
false positives: the two Windows/Linux factory assertions and the launcher's
@ -140,7 +140,7 @@ identifiable in the lane report.
| T-009 wall-clock double-click tests | resolved in batch E | Four sleeps were replaced by a deterministic test clock behind an internal factory overload. The production factory still reads `Environment.TickCount64` exactly as before. |
| T-010 two useless cases | high-confidence cleanup batch A | Delete `SmokeTest.TestProject_IsWired` and `ChaseCameraTests.ImplementsICamera`; compilation already proves both claims. |
| T-011 diagnostic-only methods | resolved in batches C, L, and R | The reviewed current set is 82 methods / 103 cases. All carry `Purpose=Diagnostic`, preserving the apparatus while removing it from release pass totals. Batch R catches investigations whose only assertion validates fixture/DAT availability, which the original mechanical output-only scan could not distinguish from an oracle. |
| T-012 source-text freezes | replacement map complete in batch I; 46 implementation-shape facts remain staged | Twelve whole-tree architecture rules and five cross-artifact contracts stay. The other 46 are campaign-era literal implementation freezes; retire each only beside its semantic/behavioral replacement, with user approval for owner/lifecycle assertions that have no equivalent yet. |
| T-012 source-text freezes | direct-read map completed in batch I and helper-mediated gap corrected in batch U; 85 implementation/test-model facts remain staged | Seventeen whole-tree architecture rules and five cross-artifact contracts stay. The other 85 are literal implementation or test-model freezes; retire each only beside its semantic/behavioral replacement, with user approval for owner/lifecycle assertions that have no equivalent yet. |
| T-013 controller self-comparison | high-confidence cleanup batch A | Capture the first controller next to the first body and compare every retry with that reference. |
| T-014 seven load-sensitive tests | six mechanisms repaired in batch F; one product defect classified | Virtual/wall-clock mixing, tiered-JIT allocation noise, a live controller clock, and a ThreadPool-start timing oracle were removed without weakening behavioral contracts. `DatSoundCache` #321 is a real in-flight-entry race and now runs as `Status=KnownFailure` pending a product fix. |
| T-015 four non-prerequisite skips | resolved in batches A/B | PVS scaffold deleted with rationale preserved; redundant chat/radar generators deleted in favor of the comprehensive Manual lane; tower oracle is `Status=KnownFailure`. |
@ -469,25 +469,33 @@ only methods, and 33 methods with some elapsed-time wait.
## Batch I source-text replacement map
The refreshed inventory finds 63 attributed methods that directly read `.cs`
source text. Reconciliation with historical T-012 assigns every method to one
of three dispositions:
Batch I's first inventory found 63 attributed methods that directly read `.cs`
source text. Batch U later proved that count was incomplete: tests that called
a local `BuilderSource()`, `ReadSource(...)`, or method-group
`File.ReadAllText` helper were invisible to the direct body regex. The
corrected syntax inventory follows same-file helper calls, recognizes invoked
and method-group readers, and only treats actual string content—not comments
such as `GameWindow.cs:5893`—as a C# path signal. It finds 67 direct and 40
helper-mediated methods, 107 total.
Manual reconciliation assigns every candidate to one of three dispositions:
| Disposition | Methods | Owning test files |
|---|---:|---|
| Campaign-era implementation-shape freeze; replace in stages | 46 | `ContentEffectsAudioCompositionTests`, `FrameRootCompositionTests`, `HostInputCameraCompositionTests`, `InteractionRetainedUiCompositionTests`, `LivePresentationCompositionTests`, `SessionPlayerCompositionTests`, `WorldRenderCompositionTests`, `GameWindowLiveSessionOwnershipTests`, `GameWindowRenderLeafCompositionTests`, `GameWindowHostBoundaryTests`, `ResourceCleanupGroupTests`, `RetailPViewPassExecutorTests`, `WorldRenderFrameBuilderTests`, `WorldSceneRendererTests`, `LandblockBuildOriginTests`, `LandblockPhysicsPublisherTests`, `LandblockRenderPublisherTests`, `GameWindowLiveEntityCompositionTests`, `UpdateFrameOrchestratorTests` |
| Whole-tree architecture policy; retain, then make syntax/metadata-aware | 12 | `LinuxPlatformBoundaryTests`, `RenderSceneArchitectureTests`, `RuntimePhysicsOwnershipTests`, `RuntimeEntityOwnershipTests` |
| Implementation-shape or test-model freeze; replace in stages | 85 | Thirty files across composition, input/physics/network wiring, graphical host/render ownership, Runtime gameplay-owner wiring, streaming, and frame orchestration; the exact stage manifest follows below. |
| Whole-tree architecture policy; retain, then make syntax/metadata-aware | 17 | `LinuxPlatformBoundaryTests`, `RenderSceneArchitectureTests`, `RuntimePhysicsOwnershipTests`, `RuntimeEntityOwnershipTests`, plus the whole-tree methods in `RuntimeActionOwnershipTests` and `RuntimeMovementOwnershipTests`, `RuntimeWorldTransitOwnershipTests`, and `RuntimeDatAccessArchitectureTests` |
| Source/project/artifact text is the actual cross-artifact contract; retain | 5 | `ParticleBindlessInstanceTests`, `PerformanceToolsTests`, `LauncherProjectBoundaryTests` |
The 46 implementation-shape facts were useful during mechanical ownership
cutovers, but are not durable behavioral evidence. They pin constructor
spellings, local names, exact call fragments, or statement order; comments can
satisfy many of their `Contains` checks, while a harmless equivalent refactor
can fail them. Their intent is often valuable—single ownership, ordering,
handoff, teardown, or absence of a parallel path—so wholesale deletion would
discard real notes and weaken some boundaries.
The 85 implementation/test-model facts were useful during mechanical ownership
cutovers and defect investigations, but are not durable behavioral evidence.
They pin constructor spellings, local names, exact call fragments, regex-shaped
method bodies, or statement order; comments can satisfy many of their
`Contains` checks, while a harmless equivalent refactor can fail them. Their
intent is often valuable—single ownership, ordering, handoff, teardown,
absence of a parallel path, or continued validity of a synthetic test model—so
wholesale deletion would discard real notes and weaken some boundaries.
R3 therefore uses this replacement rule for those 46 facts:
R3 therefore uses this replacement rule for those 85 facts:
1. preserve the ownership/order rationale in the architecture document or this
ledger;
@ -496,14 +504,15 @@ R3 therefore uses this replacement rule for those 46 facts:
3. add the semantic replacement first when none exists; and
4. remove the literal source freeze in the same small batch.
The 12 whole-tree rules remain valuable even before conversion: they ban
The 17 whole-tree rules remain valuable even before conversion: they ban
platform calls outside owners, enforce assembly/namespace containment, and
prevent duplicate Runtime entity/physics authorities. The five cross-artifact
tests compare two independently consumed artifacts (for example CPU/shader
constants, launcher project/workflow/markup, and CLI route tooling); reading
text is appropriate there.
prevent duplicate Runtime entity, physics, movement, action, transit, and DAT
authorities. The five cross-artifact tests compare two independently consumed
artifacts (for example CPU/shader constants, launcher
project/workflow/markup, and CLI route tooling); reading text is appropriate
there.
No source-freeze fact is removed in this classification batch. The 46 staged
No source-freeze fact is removed in this classification batch. The 85 staged
facts are an ambiguous architecture decision and will be presented for user
approval in behavior-owner groups rather than deleted from a numeric heuristic.
@ -515,30 +524,37 @@ and safest replacement path divide into six owner groups:
| Owner group | Facts | What the literal checks still prove | R3 recommendation |
|---|---:|---|---|
| Composition-root wiring | 13 | `GameWindow` calls the intended phase objects and no longer contains the extracted construction bodies. Phase-local tests cover ordering and rollback inside each phase, but not every root-to-phase link. | Keep until one symbol-aware composition-graph guard proves the root links. Then remove the literal constructor/local-name checks. |
| Runtime root, host, and lifetime | 12 | One canonical Runtime root/command surface, typed owner handoffs, acquisition order, and native-window-last shutdown. Existing Runtime/lifecycle tests prove much of the behavior, but several sole-owner and transfer claims remain unique. | Replace with direct ownership receipts, terminal-ledger assertions, and metadata dependency guards before removing any source freeze. |
| Render delegation and cleanup | 4 | Rendering is delegated and `GameWindow` does not regain leaf draw branches or resource ownership. The owning render suites already exercise the executor, frame builder, renderer, and resource disposal behavior. | Best first conversion: add one symbol-aware boundary guard for the absence/delegation claim, then remove the four literal checks. |
| Runtime root, graphical host, and lifetime | 24 | One canonical Runtime root/command surface, typed owner handoffs, run/render/status ordering, acquisition order, and native-window-last shutdown. Existing Runtime/lifecycle tests prove much of the behavior, but several sole-owner and transfer claims remain unique. | Replace with direct ownership receipts, terminal-ledger assertions, observable host traces, and metadata dependency guards before removing any source freeze. |
| Render delegation and cleanup | 9 | Rendering is delegated and `GameWindow` does not regain leaf draw branches or resource ownership; several tests also pin exact renderer/preparation statement order. The owning render suites already exercise the executor, frame builder, renderer, and resource disposal behavior. | Best first conversion: add a symbol-aware boundary guard for absence/delegation and observable fake traces for order, then remove the literal checks. |
| Streaming publication | 4 | The window does not own render/physics publication bodies, captured build origins flow through the extracted owners, and streamer teardown follows session reset. Publisher/build/origin suites already provide broad behavioral coverage. | Best second conversion: consolidate the absence claims into a whole-tree architecture guard and retain the behavioral origin/teardown contracts. |
| Live-entity composition | 2 | Typed sources replace window closures and reset closes every streaming/readiness owner. Lifecycle tests cover convergence, but the exact constructor-shape claim is still literal. | Replace with constructor/field metadata inspection plus the existing teardown ledger; do not delete alone. |
| Frame orchestration | 11 | Exact-once publication, frame-stage order, adapter ownership, and a deliberately thin `OnUpdate`. These are high-value order rules, but exact statement fragments are the most brittle implementation specification in the set. | Replace order assertions with an observable fake event trace and use a symbol-aware dependency guard for the thin-root rule. This is the largest conversion and should be last. |
| Frame orchestration and resource preparation | 14 | Exact-once publication, frame/resource-stage order, adapter ownership, and a deliberately thin `OnUpdate`. These are high-value order rules, but exact statement fragments are the most brittle implementation specification in the set. | Replace order assertions with observable fake event traces and use a symbol-aware dependency guard for the thin-root rule. This is the largest conversion and should be last. |
| Input, physics, and network wiring | 9 | Typed owner entry points, retry/reset wiring, and local/remote branch ordering remain at exact source locations. Behavioral controller suites cover the operations, but not every production composition route. | Add symbol-aware call-graph boundaries and route the behavioral fixtures through the production entry seam before removing the text checks. |
| Runtime gameplay-owner wiring | 9 | UI/session/shutdown consumers borrow exact action, character, inventory, and movement owners without mirrors or second mutations. Runtime ledgers cover terminal ownership but several App consumer links are literal only. | Replace consumer links with metadata dependency checks and exact borrowed-instance integration assertions. |
| Test-model/source correspondence | 1 | The hand-called create-authority drift probe still models the executor's sole production advance site. | Expose an observable executor stage/receipt or re-derive the synthetic model before removing this explicit stale-model alarm. |
Approval should authorize this replacement sequence, not wholesale deletion:
render delegation, streaming, live entities, composition roots, Runtime/host
lifetime, then frame orchestration. If the semantic replacement exposes a
claim that exists only as historical campaign wording, preserve the rationale
and bring that individual deletion back for review.
render delegation, streaming, live entities, input/physics/network wiring,
composition roots, Runtime gameplay owners, Runtime/graphical-host lifetime,
frame orchestration, then the one test-model/source pin. If the semantic
replacement exposes a claim that exists only as historical campaign wording,
preserve the rationale and bring that individual deletion back for review.
The exact staged file manifest is:
| Stage | Facts | Complete owning-file set |
|---|---:|---|
| Render delegation and cleanup | 4 | `ResourceCleanupGroupTests`, `RetailPViewPassExecutorTests`, `WorldRenderFrameBuilderTests`, `WorldSceneRendererTests` |
| Render delegation and cleanup | 9 | `ResourceCleanupGroupTests`, `RetailPViewPassExecutorTests`, `WorldRenderFrameBuilderTests`, `WorldSceneRendererTests` |
| Streaming publication | 4 | `LandblockBuildOriginTests`, `LandblockPhysicsPublisherTests`, `LandblockRenderPublisherTests` |
| Live-entity composition | 2 | `GameWindowLiveEntityCompositionTests` |
| Composition-root wiring | 13 | `ContentEffectsAudioCompositionTests`, `FrameRootCompositionTests`, `HostInputCameraCompositionTests`, `InteractionRetainedUiCompositionTests`, `LivePresentationCompositionTests`, `SessionPlayerCompositionTests`, `WorldRenderCompositionTests` |
| Runtime root, host, and lifetime | 12 | `GameWindowLiveSessionOwnershipTests`, `GameWindowRenderLeafCompositionTests`, `GameWindowHostBoundaryTests` |
| Frame orchestration | 11 | `UpdateFrameOrchestratorTests` |
| Runtime root, graphical host, and lifetime | 24 | `GameWindowLiveSessionOwnershipTests`, `GameWindowCrashStatusTests`, `GameWindowRenderLeafCompositionTests`, `GameWindowHostBoundaryTests` |
| Frame orchestration and resource preparation | 14 | `UpdateFrameOrchestratorTests`, `RenderFrameResourceControllerTests` |
| Input, physics, and network wiring | 9 | `C3cF2AutoEntryWiringTests`, `C3cF1ProductionWiringTests`, `Issue270ProductionWiringTests`, `LiveEntityNetworkBranchRoutingTests` |
| Runtime gameplay-owner wiring | 9 | The non-whole-tree methods in `RuntimeActionOwnershipTests` and `RuntimeMovementOwnershipTests`, plus `RuntimeCharacterOwnershipTests` and `RuntimeInventoryOwnershipTests` |
| Test-model/source correspondence | 1 | `CreateAuthorityDriftModelSourcePinTests` |
This manifest accounts for all 46 methods exactly. It excludes the 12
This manifest accounts for all 85 methods exactly. It excludes the 17
whole-tree policy methods and five genuine cross-artifact text contracts.
## Batch P dormant panel reachability reconciliation
@ -945,3 +961,40 @@ Verification:
and
- the no-retry complete hermetic Release gate passes 14,379/14,379 with zero
skips or failures across all 12 test assemblies.
## Batch U helper-mediated source-text audit correction
Preparing the first semantic replacement exposed a material inventory gap:
`RetailPViewPassExecutorTests` and `WorldRenderFrameBuilderTests` contain tests
that read production source through local helpers, but Batch I's direct body
regex counted only the one method in each file that called
`File.ReadAllText(...)` itself. Parameterized helpers such as
`ReadSource("GameWindow.cs")` and method-group reads such as
`.Select(File.ReadAllText)` exposed two more missed forms.
Batch U corrects the audit tool, not any test or product behavior. It now:
- detects invoked and method-group `ReadAllText`/`ReadAllLines` syntax;
- follows the same-file helper graph already used for failure and prerequisite
signals;
- follows the `.cs` path signal separately, allowing the caller to supply the
filename while a helper performs the read; and
- derives the path signal only from string/interpolated-string content, so a
comment such as `GameWindow.cs:5893` cannot classify an ordinary JSON-lines
test as a source reader.
The corrected inventory reports 67 direct and 40 helper-mediated source-text
tests, 107 total. Manual method-by-method reconciliation assigns 85 to staged
semantic replacement, 17 to retained whole-tree architecture policy, and five
to retained cross-artifact contracts. The focused inventory assertions prove a
helper-mediated builder test is included, a method-group whole-tree test is
included directly, and the prior comment/path false positive is excluded.
Verification:
- all 1,256 tracked C# test files parse and the focused 67/107 count and three
exemplar assertions pass;
- the complete 44-project Release build reports zero warnings and zero errors;
and
- the no-retry complete hermetic Release gate passes 14,379/14,379 with zero
skips or failures across all 12 test assemblies.