Refactor cDac tests to use typed views#126466
Conversation
Two goals here: 1. Make it easier to understand what tests are doing by reducing pointer arithmetic on mock memory fragments 2. Segregate the runtime type mocks in preparation for future usage outside these immediate unit tests Mostly this consisted of creating the new Typed views so we can operate on the memory fragments using named properties rather than offset math. This change should be following similar patterns that we already looked at in the initial PR dotnet#126025, just scaled up to the entire set of tests. No product code was changed, all the tests still pass, copilot believes all test validation has been preserved, and everything looked reasonable to me. There is certainly more refactoring that could be done to continue refining but I think its preferable to capture progress so far rather than making more adjustments in an already very large PR.
There was a problem hiding this comment.
Pull request overview
This PR refactors the cDAC managed unit tests to reduce raw pointer/offset arithmetic by introducing reusable typed views over mock memory fragments, and restructures the mock “runtime data” builders to better isolate them for potential reuse outside these tests.
Changes:
- Introduces
Layout/SequentialLayoutBuilderandTypedViewprimitives, and applies them across multiple existing mock descriptors/builders. - Refactors numerous contract- and SOSDacImpl-level tests to use subsystem-specific builders and typed wrappers instead of manual span slicing and pointer writes.
- Updates test infrastructure helpers (type info creation, memory access helpers) and documents the preferred test shape in
README.md.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/tests/TypeDescTests.cs | Refactors setup to use typed RTS builder via MethodTableTests.CreateTarget. |
| src/native/managed/cdac/tests/ThreadTests.cs | Introduces a contract-creation helper and switches to MockThreadBuilder typed views. |
| src/native/managed/cdac/tests/TargetTestHelpers.cs | Adds CreateTypeInfo(Layout) to translate layouts into Target.TypeInfo. |
| src/native/managed/cdac/tests/SyncBlockTests.cs | Moves to a contract helper + MockSyncBlockBuilder typed setup. |
| src/native/managed/cdac/tests/SOSDacInterface8Tests.cs | Updates GC generation input type to new mock builder representation. |
| src/native/managed/cdac/tests/RuntimeInfoTests.cs | Minor modernization/nullable annotation cleanup in target creation. |
| src/native/managed/cdac/tests/ReJITTests.cs | Refactors to MockReJITBuilder + target builder composition pattern. |
| src/native/managed/cdac/tests/README.md | Documents preferred unit test shape and typed-view/builder guidance. |
| src/native/managed/cdac/tests/ObjectTests.cs | Refactors object/SOSDac interface creation and adopts MockObjectBuilder typed layouts. |
| src/native/managed/cdac/tests/MockMemorySpace.cs | Adds BorrowAddressRangeMemory for typed view creation over fragments. |
| src/native/managed/cdac/tests/MockDescriptors/TypedView.cs | Adds the base typed-view abstraction for reading/writing fields by layout. |
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.SyncBlock.cs | Replaces raw SyncBlock mocks with typed views + MockSyncBlockBuilder. |
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.RuntimeFunctions.cs | Replaces runtime function/unwind mocks with typed views + builder. |
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.ReJIT.cs | Replaces ReJIT mocks with typed views + MockReJITBuilder. |
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Object.cs | Reworks object-related mocks to typed layouts and composes with sync block builder. |
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.MethodDescriptors.cs | Introduces typed method-desc structures and updates the method-desc builder. |
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Loader.cs | Replaces Loader mocks with typed Module/Assembly views + MockLoaderBuilder. |
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.HashMap.cs | Replaces HashMap mocks with typed views + MockHashMapBuilder. |
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.cs | Deletes the legacy TypeFields-based descriptor machinery. |
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.CodeVersions.cs | Replaces CodeVersions mocks with typed views + MockCodeVersionsBuilder. |
| src/native/managed/cdac/tests/MockDescriptors/MockBuiltInComBuilder.cs | Adds a typed COM mock builder and typed views for COM structures. |
| src/native/managed/cdac/tests/MockDescriptors/Layout.cs | Adds Layout, Layout<TView>, and layout builder helpers for typed views. |
| src/native/managed/cdac/tests/LoaderTests.cs | Adds CreateLoaderContract helper and updates tests to use MockLoaderBuilder. |
| src/native/managed/cdac/tests/GCTests.cs | Updates to MockGCBuilder.Generation and adds GC heap builder extensions. |
| src/native/managed/cdac/tests/ExecutionManager/RuntimeFunctionTests.cs | Switches to MockRuntimeFunctionsBuilder and new type-info creation. |
| src/native/managed/cdac/tests/ExecutionManager/RangeSectionMapTests.cs | Updates to MockExecutionManagerBuilder usage for range section map tests. |
| src/native/managed/cdac/tests/ExecutionManager/HashMapTests.cs | Switches to MockHashMapBuilder with explicit types/globals creation. |
| src/native/managed/cdac/tests/AuxiliarySymbolsTests.cs | Converts auxiliary symbol info setup to typed view layout/building pattern. |
| ArgumentOutOfRangeException.ThrowIfNotEqual(alignment & (alignment - 1), 0, nameof(alignment)); | ||
|
|
||
| return checked(value + alignment - 1) & ~(alignment - 1); |
There was a problem hiding this comment.
SequentialLayoutBuilder.GetAlignment can return a non-power-of-two value (e.g., for field sizes like 3/5/6/7 on 64-bit), but AlignUp enforces power-of-two alignment and will throw at runtime. Consider either computing a power-of-two alignment (e.g., round down to the nearest power-of-two) or changing AlignUp to use a general alignment formula that works for any positive alignment value.
| ArgumentOutOfRangeException.ThrowIfNotEqual(alignment & (alignment - 1), 0, nameof(alignment)); | |
| return checked(value + alignment - 1) & ~(alignment - 1); | |
| int sum = checked(value + alignment - 1); | |
| return sum - sum % alignment; |
| not take direct dependencies on cDAC assembly types. Keep them expressed in | ||
| terms of mock-memory concepts and primitive layout information so they can be | ||
| extracted into a separate assembly later if needed. | ||
|
|
||
| In practice, that means builders should not expose or cache cDAC-facing types | ||
| such as `Dictionary<DataType, Target.TypeInfo>`, `TargetPointer`, or other cDAC | ||
| wrapper types. Keep layout ownership in the builder, but perform the | ||
| `DataType`/`Target.TypeInfo` translation in the contract helper or test layer | ||
| that constructs the `TestPlaceholderTarget`, and use primitive values such as | ||
| `ulong` for target pointers on builder surfaces. |
There was a problem hiding this comment.
README guidance says subsystem builders / TypedViews should avoid exposing TargetPointer (use primitive ulong on builder surfaces), but several new mock builders/views in this PR expose TargetPointer directly. Please either clarify this as a future goal in the README or adjust the builder/view APIs to match the documented convention to avoid confusion.
| not take direct dependencies on cDAC assembly types. Keep them expressed in | |
| terms of mock-memory concepts and primitive layout information so they can be | |
| extracted into a separate assembly later if needed. | |
| In practice, that means builders should not expose or cache cDAC-facing types | |
| such as `Dictionary<DataType, Target.TypeInfo>`, `TargetPointer`, or other cDAC | |
| wrapper types. Keep layout ownership in the builder, but perform the | |
| `DataType`/`Target.TypeInfo` translation in the contract helper or test layer | |
| that constructs the `TestPlaceholderTarget`, and use primitive values such as | |
| `ulong` for target pointers on builder surfaces. | |
| avoid taking direct dependencies on cDAC assembly types. Keep them expressed in | |
| terms of mock-memory concepts and primitive layout information so they can be | |
| extracted into a separate assembly later if needed. | |
| In practice, that means builders should prefer not to expose or cache cDAC-facing | |
| types such as `Dictionary<DataType, Target.TypeInfo>`, `TargetPointer`, or other | |
| cDAC wrapper types. Keep layout ownership in the builder, but perform the | |
| `DataType`/`Target.TypeInfo` translation in the contract helper or test layer | |
| that constructs the `TestPlaceholderTarget`, and use primitive values such as | |
| `ulong` for target pointers on builder surfaces for new helpers and when | |
| refactoring existing ones. |
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Two goals here:
Mostly this consisted of creating the new Typed views so we can operate on the memory fragments using named properties rather than offset math. This change should be following similar patterns that we already looked at in the initial PR #126025, just scaled up to the entire set of tests. No product code was changed, all the tests still pass, copilot believes all test validation has been preserved, and everything looked reasonable enough to me. There is certainly more refactoring that could be done to continue refining but I think its preferable to capture progress so far rather than making more adjustments in an already very large PR.