Transfer ownership in ReflectionMetadataHook.Register instead of cloning

[Amaury Levé (Evangelink)]

What

The source-generated ReflectionMetadataHook.Register defensively cloned every array and dictionary it received (types.Clone(), per-value MethodInfo[]/Attribute[] clones, assemblyAttributes.Clone(), per-constructor ParameterTypes.Clone(), plus shallow dictionary re-materialization). This PR replaces that with an ownership-transfer contract, eliminating the cloning at startup.

Why

Register is public only because the MSTest source generator emits a [ModuleInitializer] that calls it across the assembly boundary — it is documented as not for hand-written use. Both generators (the shipping ReflectionMetadataEmitter and the AOT RuntimeRegistrationEmitter) build fresh, throwaway collections for each call and never touch them again. So the defensive copies were guarding against a caller mutation that the only sanctioned caller cannot perform.

True caller-identity authentication isn't possible (generated code lives in the untrusted test assembly, so any token/sentinel is forgeable). But we don't need identity trust — we need mutation safety, which we get structurally by taking ownership of the fresh collections the generator hands over.

How

• Store the passed Type[] / object[] arrays by reference (no clone).
• Reuse the caller's dictionary as-is when it's already a concrete Dictionary<,> (the shape both generators always emit) via a new AsOwnedDictionary helper; only exotic IReadOnlyDictionary implementations get a one-time shallow copy.
• The ConstructorInvokerInfo → internal ConstructorInvoker projection remains (a representation change, not a defensive copy) but no longer clones parameter arrays.
• Flips the XML-doc contract: the old "caller may mutate after the call" promise is replaced with an explicit ownership-transfer contract in the remarks.

Multi-TFM safe: uses only is-pattern, capacity ctors, and foreach, so it compiles on the full matrix down to net462 / netstandard2.0 / uap10.0.16299 (avoids newer BCL APIs like new Dictionary(IEnumerable<KVP>)).

Verification

• Build: all TFMs (net462, net8.0, net9.0, net8.0-windows, net9.0-windows, uap10.0.16299) — 0 warnings, 0 errors.
• Unit tests: MSTestAdapter.PlatformServices.UnitTests — 893/893 pass.
• Acceptance (runtime Register path, after -pack): SourceGenerationNonAotTests (2/2, 3-arg overload); InconclusiveTests (24/24) across Reflection, SourceGeneration, and AotSourceGeneration modes — the last exercises the modified 8-arg overload end-to-end.

[Copilot]

Pull request overview

This PR updates the MSTest adapter’s source-generation registration hook (ReflectionMetadataHook.Register) to avoid defensive cloning of arrays and dictionaries, switching to an ownership-transfer contract so startup registration can be zero-copy.

Changes:

• Replaces defensive Clone() patterns with direct storage of generator-provided collections (ownership transfer).
• Adds AsOwnedDictionary to reuse concrete Dictionary<,> inputs (and shallow-copy only non-DictionaryIReadOnlyDictionary implementations).
• Stops cloning constructor-parameter type arrays when projecting ConstructorInvokerInfo into internal constructor invokers.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook.cs	Switches registration to ownership-transfer semantics, reuses dictionaries via AsOwnedDictionary, and updates XML docs to reflect the new contract.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Low

[Amaury Levé (Evangelink)]

/backport to rel/4.3

[github-actions]

Started backporting to rel/4.3: https://github.com/microsoft/testfx/actions/runs/28668317883

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Medium

[github-actions]

🧪 Test quality grade — PR #9586

Grade	Test	Notes
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ CreatesInstance_ WithoutActivator	Tests both GetConstructorInvoker and CreateInstance in one body; consider splitting for per-method isolation.
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetTestMethodInvoker_ ReturnsRegisteredDelegate_ AndInvokesWithoutReflection	Tests delegate retrieval and invocation together; assertions are excellent — consider one test per behavior.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ FallsBackToReflection_ WhenArgumentsDoNotMatchRegisteredConstructor	Value.Should().BeNull() correctly distinguishes the fallback path from the registered constructor. No issues found.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetPropertySetter_ AssignsValue_ WithoutSetValue	Clean identity + side-effect assertion pair; concise and well-structured. No issues found.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 71.3 AIC · ⌖ 10.1 AIC · ⊞ 9.5K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

What

The source-generated ReflectionMetadataHook.Register defensively cloned every array and dictionary it received (types.Clone(), per-value MethodInfo[]/Attribute[] clones, assemblyAttributes.Clone(), per-constructor ParameterTypes.Clone(), plus shallow dictionary re-materialization). This PR replaces that with an ownership-transfer contract, eliminating the cloning at startup.

Why

Register is public only because the MSTest source generator emits a [ModuleInitializer] that calls it across the assembly boundary — it is documented as not for hand-written use. Both generators (the shipping ReflectionMetadataEmitter and the AOT RuntimeRegistrationEmitter) build fresh, throwaway collections for each call and never touch them again. So the defensive copies were guarding against a caller mutation that the only sanctioned caller cannot perform.

True caller-identity authentication isn't possible (generated code lives in the untrusted test assembly, so any token/sentinel is forgeable). But we don't need identity trust — we need mutation safety, which we get structurally by taking ownership of the fresh collections the generator hands over.

How

• Store the passed Type[] / object[] arrays by reference (no clone).
• Reuse the caller's dictionary as-is when it's already a concrete Dictionary<,> (the shape both generators always emit) via a new AsOwnedDictionary helper; only exotic IReadOnlyDictionary implementations get a one-time shallow copy.
• The ConstructorInvokerInfo → internal ConstructorInvoker projection remains (a representation change, not a defensive copy) but no longer clones parameter arrays.
• Flips the XML-doc contract: the old "caller may mutate after the call" promise is replaced with an explicit ownership-transfer contract in the remarks.

Multi-TFM safe: uses only is-pattern, capacity ctors, and foreach, so it compiles on the full matrix down to net462 / netstandard2.0 / uap10.0.16299 (avoids newer BCL APIs like new Dictionary(IEnumerable<KVP>)).

Verification

• Build: all TFMs (net462, net8.0, net9.0, net8.0-windows, net9.0-windows, uap10.0.16299) — 0 warnings, 0 errors.
• Unit tests: MSTestAdapter.PlatformServices.UnitTests — 893/893 pass.
• Acceptance (runtime Register path, after -pack): SourceGenerationNonAotTests (2/2, 3-arg overload); InconclusiveTests (24/24) across Reflection, SourceGeneration, and AotSourceGeneration modes — the last exercises the modified 8-arg overload end-to-end.

[Copilot]

Pull request overview

This PR updates the MSTest adapter’s source-generation registration hook (ReflectionMetadataHook.Register) to avoid defensive cloning of arrays and dictionaries, switching to an ownership-transfer contract so startup registration can be zero-copy.

Changes:

• Replaces defensive Clone() patterns with direct storage of generator-provided collections (ownership transfer).
• Adds AsOwnedDictionary to reuse concrete Dictionary<,> inputs (and shallow-copy only non-DictionaryIReadOnlyDictionary implementations).
• Stops cloning constructor-parameter type arrays when projecting ConstructorInvokerInfo into internal constructor invokers.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook.cs	Switches registration to ownership-transfer semantics, reuses dictionaries via AsOwnedDictionary, and updates XML docs to reflect the new contract.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Low

[Amaury Levé (Evangelink)]

/backport to rel/4.3

[github-actions]

Started backporting to rel/4.3: https://github.com/microsoft/testfx/actions/runs/28668317883

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Medium

[github-actions]

🧪 Test quality grade — PR #9586

Grade	Test	Notes
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ CreatesInstance_ WithoutActivator	Tests both GetConstructorInvoker and CreateInstance in one body; consider splitting for per-method isolation.
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetTestMethodInvoker_ ReturnsRegisteredDelegate_ AndInvokesWithoutReflection	Tests delegate retrieval and invocation together; assertions are excellent — consider one test per behavior.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ FallsBackToReflection_ WhenArgumentsDoNotMatchRegisteredConstructor	Value.Should().BeNull() correctly distinguishes the fallback path from the registered constructor. No issues found.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetPropertySetter_ AssignsValue_ WithoutSetValue	Clean identity + side-effect assertion pair; concise and well-structured. No issues found.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 71.3 AIC · ⌖ 10.1 AIC · ⊞ 9.5K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

What

The source-generated ReflectionMetadataHook.Register defensively cloned every array and dictionary it received (types.Clone(), per-value MethodInfo[]/Attribute[] clones, assemblyAttributes.Clone(), per-constructor ParameterTypes.Clone(), plus shallow dictionary re-materialization). This PR replaces that with an ownership-transfer contract, eliminating the cloning at startup.

Why

Register is public only because the MSTest source generator emits a [ModuleInitializer] that calls it across the assembly boundary — it is documented as not for hand-written use. Both generators (the shipping ReflectionMetadataEmitter and the AOT RuntimeRegistrationEmitter) build fresh, throwaway collections for each call and never touch them again. So the defensive copies were guarding against a caller mutation that the only sanctioned caller cannot perform.

True caller-identity authentication isn't possible (generated code lives in the untrusted test assembly, so any token/sentinel is forgeable). But we don't need identity trust — we need mutation safety, which we get structurally by taking ownership of the fresh collections the generator hands over.

How

• Store the passed Type[] / object[] arrays by reference (no clone).
• Reuse the caller's dictionary as-is when it's already a concrete Dictionary<,> (the shape both generators always emit) via a new AsOwnedDictionary helper; only exotic IReadOnlyDictionary implementations get a one-time shallow copy.
• The ConstructorInvokerInfo → internal ConstructorInvoker projection remains (a representation change, not a defensive copy) but no longer clones parameter arrays.
• Flips the XML-doc contract: the old "caller may mutate after the call" promise is replaced with an explicit ownership-transfer contract in the remarks.

Multi-TFM safe: uses only is-pattern, capacity ctors, and foreach, so it compiles on the full matrix down to net462 / netstandard2.0 / uap10.0.16299 (avoids newer BCL APIs like new Dictionary(IEnumerable<KVP>)).

Verification

• Build: all TFMs (net462, net8.0, net9.0, net8.0-windows, net9.0-windows, uap10.0.16299) — 0 warnings, 0 errors.
• Unit tests: MSTestAdapter.PlatformServices.UnitTests — 893/893 pass.
• Acceptance (runtime Register path, after -pack): SourceGenerationNonAotTests (2/2, 3-arg overload); InconclusiveTests (24/24) across Reflection, SourceGeneration, and AotSourceGeneration modes — the last exercises the modified 8-arg overload end-to-end.

[Copilot]

Pull request overview

This PR updates the MSTest adapter’s source-generation registration hook (ReflectionMetadataHook.Register) to avoid defensive cloning of arrays and dictionaries, switching to an ownership-transfer contract so startup registration can be zero-copy.

Changes:

• Replaces defensive Clone() patterns with direct storage of generator-provided collections (ownership transfer).
• Adds AsOwnedDictionary to reuse concrete Dictionary<,> inputs (and shallow-copy only non-DictionaryIReadOnlyDictionary implementations).
• Stops cloning constructor-parameter type arrays when projecting ConstructorInvokerInfo into internal constructor invokers.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook.cs	Switches registration to ownership-transfer semantics, reuses dictionaries via AsOwnedDictionary, and updates XML docs to reflect the new contract.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Low

[Amaury Levé (Evangelink)]

/backport to rel/4.3

[github-actions]

Started backporting to rel/4.3: https://github.com/microsoft/testfx/actions/runs/28668317883

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Medium

[github-actions]

🧪 Test quality grade — PR #9586

Grade	Test	Notes
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ CreatesInstance_ WithoutActivator	Tests both GetConstructorInvoker and CreateInstance in one body; consider splitting for per-method isolation.
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetTestMethodInvoker_ ReturnsRegisteredDelegate_ AndInvokesWithoutReflection	Tests delegate retrieval and invocation together; assertions are excellent — consider one test per behavior.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ FallsBackToReflection_ WhenArgumentsDoNotMatchRegisteredConstructor	Value.Should().BeNull() correctly distinguishes the fallback path from the registered constructor. No issues found.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetPropertySetter_ AssignsValue_ WithoutSetValue	Clean identity + side-effect assertion pair; concise and well-structured. No issues found.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 71.3 AIC · ⌖ 10.1 AIC · ⊞ 9.5K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

What

The source-generated ReflectionMetadataHook.Register defensively cloned every array and dictionary it received (types.Clone(), per-value MethodInfo[]/Attribute[] clones, assemblyAttributes.Clone(), per-constructor ParameterTypes.Clone(), plus shallow dictionary re-materialization). This PR replaces that with an ownership-transfer contract, eliminating the cloning at startup.

Why

Register is public only because the MSTest source generator emits a [ModuleInitializer] that calls it across the assembly boundary — it is documented as not for hand-written use. Both generators (the shipping ReflectionMetadataEmitter and the AOT RuntimeRegistrationEmitter) build fresh, throwaway collections for each call and never touch them again. So the defensive copies were guarding against a caller mutation that the only sanctioned caller cannot perform.

True caller-identity authentication isn't possible (generated code lives in the untrusted test assembly, so any token/sentinel is forgeable). But we don't need identity trust — we need mutation safety, which we get structurally by taking ownership of the fresh collections the generator hands over.

How

• Store the passed Type[] / object[] arrays by reference (no clone).
• Reuse the caller's dictionary as-is when it's already a concrete Dictionary<,> (the shape both generators always emit) via a new AsOwnedDictionary helper; only exotic IReadOnlyDictionary implementations get a one-time shallow copy.
• The ConstructorInvokerInfo → internal ConstructorInvoker projection remains (a representation change, not a defensive copy) but no longer clones parameter arrays.
• Flips the XML-doc contract: the old "caller may mutate after the call" promise is replaced with an explicit ownership-transfer contract in the remarks.

Multi-TFM safe: uses only is-pattern, capacity ctors, and foreach, so it compiles on the full matrix down to net462 / netstandard2.0 / uap10.0.16299 (avoids newer BCL APIs like new Dictionary(IEnumerable<KVP>)).

Verification

• Build: all TFMs (net462, net8.0, net9.0, net8.0-windows, net9.0-windows, uap10.0.16299) — 0 warnings, 0 errors.
• Unit tests: MSTestAdapter.PlatformServices.UnitTests — 893/893 pass.
• Acceptance (runtime Register path, after -pack): SourceGenerationNonAotTests (2/2, 3-arg overload); InconclusiveTests (24/24) across Reflection, SourceGeneration, and AotSourceGeneration modes — the last exercises the modified 8-arg overload end-to-end.

[Copilot]

Pull request overview

This PR updates the MSTest adapter’s source-generation registration hook (ReflectionMetadataHook.Register) to avoid defensive cloning of arrays and dictionaries, switching to an ownership-transfer contract so startup registration can be zero-copy.

Changes:

• Replaces defensive Clone() patterns with direct storage of generator-provided collections (ownership transfer).
• Adds AsOwnedDictionary to reuse concrete Dictionary<,> inputs (and shallow-copy only non-DictionaryIReadOnlyDictionary implementations).
• Stops cloning constructor-parameter type arrays when projecting ConstructorInvokerInfo into internal constructor invokers.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook.cs	Switches registration to ownership-transfer semantics, reuses dictionaries via AsOwnedDictionary, and updates XML docs to reflect the new contract.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Low

[Amaury Levé (Evangelink)]

/backport to rel/4.3

[github-actions]

Started backporting to rel/4.3: https://github.com/microsoft/testfx/actions/runs/28668317883

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Medium

[github-actions]

🧪 Test quality grade — PR #9586

Grade	Test	Notes
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ CreatesInstance_ WithoutActivator	Tests both GetConstructorInvoker and CreateInstance in one body; consider splitting for per-method isolation.
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetTestMethodInvoker_ ReturnsRegisteredDelegate_ AndInvokesWithoutReflection	Tests delegate retrieval and invocation together; assertions are excellent — consider one test per behavior.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ FallsBackToReflection_ WhenArgumentsDoNotMatchRegisteredConstructor	Value.Should().BeNull() correctly distinguishes the fallback path from the registered constructor. No issues found.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetPropertySetter_ AssignsValue_ WithoutSetValue	Clean identity + side-effect assertion pair; concise and well-structured. No issues found.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 71.3 AIC · ⌖ 10.1 AIC · ⊞ 9.5K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

What

The source-generated ReflectionMetadataHook.Register defensively cloned every array and dictionary it received (types.Clone(), per-value MethodInfo[]/Attribute[] clones, assemblyAttributes.Clone(), per-constructor ParameterTypes.Clone(), plus shallow dictionary re-materialization). This PR replaces that with an ownership-transfer contract, eliminating the cloning at startup.

Why

Register is public only because the MSTest source generator emits a [ModuleInitializer] that calls it across the assembly boundary — it is documented as not for hand-written use. Both generators (the shipping ReflectionMetadataEmitter and the AOT RuntimeRegistrationEmitter) build fresh, throwaway collections for each call and never touch them again. So the defensive copies were guarding against a caller mutation that the only sanctioned caller cannot perform.

True caller-identity authentication isn't possible (generated code lives in the untrusted test assembly, so any token/sentinel is forgeable). But we don't need identity trust — we need mutation safety, which we get structurally by taking ownership of the fresh collections the generator hands over.

How

• Store the passed Type[] / object[] arrays by reference (no clone).
• Reuse the caller's dictionary as-is when it's already a concrete Dictionary<,> (the shape both generators always emit) via a new AsOwnedDictionary helper; only exotic IReadOnlyDictionary implementations get a one-time shallow copy.
• The ConstructorInvokerInfo → internal ConstructorInvoker projection remains (a representation change, not a defensive copy) but no longer clones parameter arrays.
• Flips the XML-doc contract: the old "caller may mutate after the call" promise is replaced with an explicit ownership-transfer contract in the remarks.

Multi-TFM safe: uses only is-pattern, capacity ctors, and foreach, so it compiles on the full matrix down to net462 / netstandard2.0 / uap10.0.16299 (avoids newer BCL APIs like new Dictionary(IEnumerable<KVP>)).

Verification

• Build: all TFMs (net462, net8.0, net9.0, net8.0-windows, net9.0-windows, uap10.0.16299) — 0 warnings, 0 errors.
• Unit tests: MSTestAdapter.PlatformServices.UnitTests — 893/893 pass.
• Acceptance (runtime Register path, after -pack): SourceGenerationNonAotTests (2/2, 3-arg overload); InconclusiveTests (24/24) across Reflection, SourceGeneration, and AotSourceGeneration modes — the last exercises the modified 8-arg overload end-to-end.

[Copilot]

Pull request overview

This PR updates the MSTest adapter’s source-generation registration hook (ReflectionMetadataHook.Register) to avoid defensive cloning of arrays and dictionaries, switching to an ownership-transfer contract so startup registration can be zero-copy.

Changes:

• Replaces defensive Clone() patterns with direct storage of generator-provided collections (ownership transfer).
• Adds AsOwnedDictionary to reuse concrete Dictionary<,> inputs (and shallow-copy only non-DictionaryIReadOnlyDictionary implementations).
• Stops cloning constructor-parameter type arrays when projecting ConstructorInvokerInfo into internal constructor invokers.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook.cs	Switches registration to ownership-transfer semantics, reuses dictionaries via AsOwnedDictionary, and updates XML docs to reflect the new contract.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Low

[Amaury Levé (Evangelink)]

/backport to rel/4.3

[github-actions]

Started backporting to rel/4.3: https://github.com/microsoft/testfx/actions/runs/28668317883

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Medium

[github-actions]

🧪 Test quality grade — PR #9586

Grade	Test	Notes
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ CreatesInstance_ WithoutActivator	Tests both GetConstructorInvoker and CreateInstance in one body; consider splitting for per-method isolation.
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetTestMethodInvoker_ ReturnsRegisteredDelegate_ AndInvokesWithoutReflection	Tests delegate retrieval and invocation together; assertions are excellent — consider one test per behavior.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ FallsBackToReflection_ WhenArgumentsDoNotMatchRegisteredConstructor	Value.Should().BeNull() correctly distinguishes the fallback path from the registered constructor. No issues found.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetPropertySetter_ AssignsValue_ WithoutSetValue	Clean identity + side-effect assertion pair; concise and well-structured. No issues found.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 71.3 AIC · ⌖ 10.1 AIC · ⊞ 9.5K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

What

The source-generated ReflectionMetadataHook.Register defensively cloned every array and dictionary it received (types.Clone(), per-value MethodInfo[]/Attribute[] clones, assemblyAttributes.Clone(), per-constructor ParameterTypes.Clone(), plus shallow dictionary re-materialization). This PR replaces that with an ownership-transfer contract, eliminating the cloning at startup.

Why

Register is public only because the MSTest source generator emits a [ModuleInitializer] that calls it across the assembly boundary — it is documented as not for hand-written use. Both generators (the shipping ReflectionMetadataEmitter and the AOT RuntimeRegistrationEmitter) build fresh, throwaway collections for each call and never touch them again. So the defensive copies were guarding against a caller mutation that the only sanctioned caller cannot perform.

True caller-identity authentication isn't possible (generated code lives in the untrusted test assembly, so any token/sentinel is forgeable). But we don't need identity trust — we need mutation safety, which we get structurally by taking ownership of the fresh collections the generator hands over.

How

• Store the passed Type[] / object[] arrays by reference (no clone).
• Reuse the caller's dictionary as-is when it's already a concrete Dictionary<,> (the shape both generators always emit) via a new AsOwnedDictionary helper; only exotic IReadOnlyDictionary implementations get a one-time shallow copy.
• The ConstructorInvokerInfo → internal ConstructorInvoker projection remains (a representation change, not a defensive copy) but no longer clones parameter arrays.
• Flips the XML-doc contract: the old "caller may mutate after the call" promise is replaced with an explicit ownership-transfer contract in the remarks.

Multi-TFM safe: uses only is-pattern, capacity ctors, and foreach, so it compiles on the full matrix down to net462 / netstandard2.0 / uap10.0.16299 (avoids newer BCL APIs like new Dictionary(IEnumerable<KVP>)).

Verification

• Build: all TFMs (net462, net8.0, net9.0, net8.0-windows, net9.0-windows, uap10.0.16299) — 0 warnings, 0 errors.
• Unit tests: MSTestAdapter.PlatformServices.UnitTests — 893/893 pass.
• Acceptance (runtime Register path, after -pack): SourceGenerationNonAotTests (2/2, 3-arg overload); InconclusiveTests (24/24) across Reflection, SourceGeneration, and AotSourceGeneration modes — the last exercises the modified 8-arg overload end-to-end.

[Copilot]

Pull request overview

This PR updates the MSTest adapter’s source-generation registration hook (ReflectionMetadataHook.Register) to avoid defensive cloning of arrays and dictionaries, switching to an ownership-transfer contract so startup registration can be zero-copy.

Changes:

• Replaces defensive Clone() patterns with direct storage of generator-provided collections (ownership transfer).
• Adds AsOwnedDictionary to reuse concrete Dictionary<,> inputs (and shallow-copy only non-DictionaryIReadOnlyDictionary implementations).
• Stops cloning constructor-parameter type arrays when projecting ConstructorInvokerInfo into internal constructor invokers.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook.cs	Switches registration to ownership-transfer semantics, reuses dictionaries via AsOwnedDictionary, and updates XML docs to reflect the new contract.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Low

[Amaury Levé (Evangelink)]

/backport to rel/4.3

[github-actions]

Started backporting to rel/4.3: https://github.com/microsoft/testfx/actions/runs/28668317883

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Medium

[github-actions]

🧪 Test quality grade — PR #9586

Grade	Test	Notes
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ CreatesInstance_ WithoutActivator	Tests both GetConstructorInvoker and CreateInstance in one body; consider splitting for per-method isolation.
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetTestMethodInvoker_ ReturnsRegisteredDelegate_ AndInvokesWithoutReflection	Tests delegate retrieval and invocation together; assertions are excellent — consider one test per behavior.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ FallsBackToReflection_ WhenArgumentsDoNotMatchRegisteredConstructor	Value.Should().BeNull() correctly distinguishes the fallback path from the registered constructor. No issues found.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetPropertySetter_ AssignsValue_ WithoutSetValue	Clean identity + side-effect assertion pair; concise and well-structured. No issues found.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 71.3 AIC · ⌖ 10.1 AIC · ⊞ 9.5K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

What

The source-generated ReflectionMetadataHook.Register defensively cloned every array and dictionary it received (types.Clone(), per-value MethodInfo[]/Attribute[] clones, assemblyAttributes.Clone(), per-constructor ParameterTypes.Clone(), plus shallow dictionary re-materialization). This PR replaces that with an ownership-transfer contract, eliminating the cloning at startup.

Why

Register is public only because the MSTest source generator emits a [ModuleInitializer] that calls it across the assembly boundary — it is documented as not for hand-written use. Both generators (the shipping ReflectionMetadataEmitter and the AOT RuntimeRegistrationEmitter) build fresh, throwaway collections for each call and never touch them again. So the defensive copies were guarding against a caller mutation that the only sanctioned caller cannot perform.

True caller-identity authentication isn't possible (generated code lives in the untrusted test assembly, so any token/sentinel is forgeable). But we don't need identity trust — we need mutation safety, which we get structurally by taking ownership of the fresh collections the generator hands over.

How

• Store the passed Type[] / object[] arrays by reference (no clone).
• Reuse the caller's dictionary as-is when it's already a concrete Dictionary<,> (the shape both generators always emit) via a new AsOwnedDictionary helper; only exotic IReadOnlyDictionary implementations get a one-time shallow copy.
• The ConstructorInvokerInfo → internal ConstructorInvoker projection remains (a representation change, not a defensive copy) but no longer clones parameter arrays.
• Flips the XML-doc contract: the old "caller may mutate after the call" promise is replaced with an explicit ownership-transfer contract in the remarks.

Multi-TFM safe: uses only is-pattern, capacity ctors, and foreach, so it compiles on the full matrix down to net462 / netstandard2.0 / uap10.0.16299 (avoids newer BCL APIs like new Dictionary(IEnumerable<KVP>)).

Verification

• Build: all TFMs (net462, net8.0, net9.0, net8.0-windows, net9.0-windows, uap10.0.16299) — 0 warnings, 0 errors.
• Unit tests: MSTestAdapter.PlatformServices.UnitTests — 893/893 pass.
• Acceptance (runtime Register path, after -pack): SourceGenerationNonAotTests (2/2, 3-arg overload); InconclusiveTests (24/24) across Reflection, SourceGeneration, and AotSourceGeneration modes — the last exercises the modified 8-arg overload end-to-end.

[Copilot]

Pull request overview

This PR updates the MSTest adapter’s source-generation registration hook (ReflectionMetadataHook.Register) to avoid defensive cloning of arrays and dictionaries, switching to an ownership-transfer contract so startup registration can be zero-copy.

Changes:

• Replaces defensive Clone() patterns with direct storage of generator-provided collections (ownership transfer).
• Adds AsOwnedDictionary to reuse concrete Dictionary<,> inputs (and shallow-copy only non-DictionaryIReadOnlyDictionary implementations).
• Stops cloning constructor-parameter type arrays when projecting ConstructorInvokerInfo into internal constructor invokers.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook.cs	Switches registration to ownership-transfer semantics, reuses dictionaries via AsOwnedDictionary, and updates XML docs to reflect the new contract.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Low

[Amaury Levé (Evangelink)]

/backport to rel/4.3

[github-actions]

Started backporting to rel/4.3: https://github.com/microsoft/testfx/actions/runs/28668317883

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Medium

[github-actions]

🧪 Test quality grade — PR #9586

Grade	Test	Notes
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ CreatesInstance_ WithoutActivator	Tests both GetConstructorInvoker and CreateInstance in one body; consider splitting for per-method isolation.
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetTestMethodInvoker_ ReturnsRegisteredDelegate_ AndInvokesWithoutReflection	Tests delegate retrieval and invocation together; assertions are excellent — consider one test per behavior.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ FallsBackToReflection_ WhenArgumentsDoNotMatchRegisteredConstructor	Value.Should().BeNull() correctly distinguishes the fallback path from the registered constructor. No issues found.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetPropertySetter_ AssignsValue_ WithoutSetValue	Clean identity + side-effect assertion pair; concise and well-structured. No issues found.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 71.3 AIC · ⌖ 10.1 AIC · ⊞ 9.5K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

What

The source-generated ReflectionMetadataHook.Register defensively cloned every array and dictionary it received (types.Clone(), per-value MethodInfo[]/Attribute[] clones, assemblyAttributes.Clone(), per-constructor ParameterTypes.Clone(), plus shallow dictionary re-materialization). This PR replaces that with an ownership-transfer contract, eliminating the cloning at startup.

Why

Register is public only because the MSTest source generator emits a [ModuleInitializer] that calls it across the assembly boundary — it is documented as not for hand-written use. Both generators (the shipping ReflectionMetadataEmitter and the AOT RuntimeRegistrationEmitter) build fresh, throwaway collections for each call and never touch them again. So the defensive copies were guarding against a caller mutation that the only sanctioned caller cannot perform.

True caller-identity authentication isn't possible (generated code lives in the untrusted test assembly, so any token/sentinel is forgeable). But we don't need identity trust — we need mutation safety, which we get structurally by taking ownership of the fresh collections the generator hands over.

How

• Store the passed Type[] / object[] arrays by reference (no clone).
• Reuse the caller's dictionary as-is when it's already a concrete Dictionary<,> (the shape both generators always emit) via a new AsOwnedDictionary helper; only exotic IReadOnlyDictionary implementations get a one-time shallow copy.
• The ConstructorInvokerInfo → internal ConstructorInvoker projection remains (a representation change, not a defensive copy) but no longer clones parameter arrays.
• Flips the XML-doc contract: the old "caller may mutate after the call" promise is replaced with an explicit ownership-transfer contract in the remarks.

Multi-TFM safe: uses only is-pattern, capacity ctors, and foreach, so it compiles on the full matrix down to net462 / netstandard2.0 / uap10.0.16299 (avoids newer BCL APIs like new Dictionary(IEnumerable<KVP>)).

Verification

• Build: all TFMs (net462, net8.0, net9.0, net8.0-windows, net9.0-windows, uap10.0.16299) — 0 warnings, 0 errors.
• Unit tests: MSTestAdapter.PlatformServices.UnitTests — 893/893 pass.
• Acceptance (runtime Register path, after -pack): SourceGenerationNonAotTests (2/2, 3-arg overload); InconclusiveTests (24/24) across Reflection, SourceGeneration, and AotSourceGeneration modes — the last exercises the modified 8-arg overload end-to-end.

[Copilot]

Pull request overview

This PR updates the MSTest adapter’s source-generation registration hook (ReflectionMetadataHook.Register) to avoid defensive cloning of arrays and dictionaries, switching to an ownership-transfer contract so startup registration can be zero-copy.

Changes:

• Replaces defensive Clone() patterns with direct storage of generator-provided collections (ownership transfer).
• Adds AsOwnedDictionary to reuse concrete Dictionary<,> inputs (and shallow-copy only non-DictionaryIReadOnlyDictionary implementations).
• Stops cloning constructor-parameter type arrays when projecting ConstructorInvokerInfo into internal constructor invokers.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook.cs	Switches registration to ownership-transfer semantics, reuses dictionaries via AsOwnedDictionary, and updates XML docs to reflect the new contract.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Low

[Amaury Levé (Evangelink)]

/backport to rel/4.3

[github-actions]

Started backporting to rel/4.3: https://github.com/microsoft/testfx/actions/runs/28668317883

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Medium

[github-actions]

🧪 Test quality grade — PR #9586

Grade	Test	Notes
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ CreatesInstance_ WithoutActivator	Tests both GetConstructorInvoker and CreateInstance in one body; consider splitting for per-method isolation.
B (80–89)	mod SourceGeneratedReflectionOperationsTests. GetTestMethodInvoker_ ReturnsRegisteredDelegate_ AndInvokesWithoutReflection	Tests delegate retrieval and invocation together; assertions are excellent — consider one test per behavior.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetConstructorInvoker_ FallsBackToReflection_ WhenArgumentsDoNotMatchRegisteredConstructor	Value.Should().BeNull() correctly distinguishes the fallback path from the registered constructor. No issues found.
A (90–100)	mod SourceGeneratedReflectionOperationsTests. GetPropertySetter_ AssignsValue_ WithoutSetValue	Clean identity + side-effect assertion pair; concise and well-structured. No issues found.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 71.3 AIC · ⌖ 10.1 AIC · ⊞ 9.5K · [◷]( · ◷)