Drop the ObjectModel package reference from PlatformServices (Phase 7 capstone)

[Amaury Levé (Evangelink)]

Phase 7 (capstone) — drop the ObjectModel package reference

The final slice of the initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. After this, PlatformServices no longer references the VSTest object model (Microsoft.TestPlatform.ObjectModel) at all — every VSTest dependency now lives in the MSTest.TestAdapter layer above it. Strict byte-for-byte, no behavior change.

Production changes

1. Remove the Microsoft.TestPlatform.ObjectModelPackageReference from MSTestAdapter.PlatformServices.csproj.
2. RunConfigurationSettings was the last consumer of a transitively-provided VSTest package: it used PlatformAbstractions' PlatformApartmentState enum {MTA, STA} to parse ExecutionThreadApartmentState. Replaced with a local internal enum ApartmentStateSetting { MTA, STA } of the same shape (same member names and order — MTA=0, STA=1), preserving the exact Enum.TryParse → STA/MTA → System.Threading.ApartmentState / else-throw behavior byte-for-byte on both the runsettings-XML and config paths. The member order is load-bearing (Enum.TryParse accepts numeric strings "0"/"1", so the numbering must match) and is commented as such. (Parsing directly to System.Threading.ApartmentState would change the handling of the Unknown value — it has that extra member — so a faithful 2-member local enum is required. The property type stays the BCL System.Threading.ApartmentState; only the parse-enum is neutralized.)
3. Add a direct System.Configurationframework reference on .NET Framework. ConfigurationManager/ConfigurationElementCollection (used by TestDataSource) were previously pulled in transitively via the object-model package; System.Configuration is a framework assembly, so it is now referenced directly.

Result: PlatformServices is fully platform-neutral

The compiled MSTestAdapter.PlatformServices assembly has zero references to any Microsoft.*.TestPlatform.* assembly on every real TFM (net462/net8.0/net9.0 + windows variants), verified via assembly metadata.

Guard test (the finish line)

ObjectModelDecouplingTests asserts the compiled PlatformServices assembly references no assembly whose name contains "TestPlatform" — catching ObjectModel, PlatformAbstractions, CoreUtilities, etc. (MSTest's own framework is MSTest.TestFramework, which doesn't match). This locks the platform-agnostic contract permanently.

Test-project fix

PlatformServices.Desktop.IntegrationTests uses ObjectModel's XmlRunSettingsUtilities directly (previously transitive through the PlatformServices project reference), so it gets its own direct Microsoft.TestPlatform.ObjectModelPackageReference. Test projects may reference the object model; only the production assembly must be neutral.

Verification

• All real TFMs build 0-warning (UWP builds via full msbuild in CI).
• Compiled dll: zeroTestPlatform references (net462 + net8.0, metadata-verified).
• MSTestAdapter.PlatformServices.UnitTests: 936/936 (net462), 898/898 (net8.0) — includes the new guard test and the STA/MTA parsing tests on both the runsettings-XML and config paths.
• PlatformServices.Desktop.IntegrationTests: 15/15.
• MSTestAdapter.UnitTests: 21/21. MSTest.TestAdapter and MSTest.IntegrationTests build clean.
• Expert-reviewer pass.

Stacking

Stacks on #9632 (Phase 6e-4c3); base branch dev/amauryleve/vstest-decoupling-suspendcoverage. Review/merge after the earlier PRs in the chain reach the base. Do not squash-rebase the base.

[github-actions]

Note

🤖 Automated review 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 Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

PR #9633 — VSTest ObjectModel decoupling from PlatformServices

#	Dimension	Verdict
1	Algorithmic Correctness	⚠️ 1 MODERATE
2	Threading & Concurrency	✅ LGTM
3	Security & IPC Contract Safety	✅ LGTM
4	Public API & Binary Compatibility	✅ LGTM
5	Performance & Allocations	✅ LGTM
6	Cross-TFM Compatibility	✅ LGTM
7	Resource & IDisposable Management	✅ LGTM
8	Defensive Coding at Boundaries	✅ LGTM (covered by #1)
9	Localization & Resources	✅ LGTM
10	Test Isolation	✅ LGTM
11	Assertion Quality	✅ LGTM
12	Flakiness Patterns	✅ LGTM
13	Test Completeness	⚠️ 1 MODERATE
14	Data-Driven Test Coverage	N/A
15	Code Structure & Simplification	✅ LGTM
16	Naming & Conventions	✅ LGTM
17	Documentation Accuracy	✅ LGTM
18	Analyzer & Code Fix Quality	N/A
19	IPC Wire Compatibility	N/A
20	Build Infrastructure & Dependencies	✅ LGTM
21	Scope & PR Discipline	✅ LGTM
22	PowerShell Scripting Hygiene	N/A

✅ 17/18 applicable dimensions clean.

---

Findings

• Algorithmic Correctness (MODERATE) — ArePublicKeyTokensEqual(byte[] left, byte[] right) in TestSourceHandler.cs line 142 dereferences both parameters unconditionally. AssemblyName.GetPublicKeyToken() returns null for unsigned assemblies, producing a NullReferenceException that is silently caught and converted to the conservative null → true path (false-positive discovery) rather than the correct false. See inline comment for the fix (annotate byte[]? and add a null guard at the top of the helper).

• Test Completeness (MODERATE) — The new SuspendCodeCoverage class (Utilities/SuspendCodeCoverage.cs) has no unit tests. Three behaviours are testable and could silently regress on .NET Framework TFMs without coverage: (1) constructor saves the previous env-var value and sets "TRUE", (2) Dispose() restores the previous value (null → delete), (3) the double-dispose guard prevents a second restoration. Suggested location: a new SuspendCodeCoverageTests.cs in MSTestAdapter.PlatformServices.UnitTests, guarded with #if NETFRAMEWORK.

---

Notable positives

• The ApartmentStateSetting enum ordering (MTA=0, STA=1) correctly matches the old PlatformApartmentState numeric values, preserving parse compatibility for numeric run-settings strings — and the load-bearing comment explaining this is clear.
• SuspendCodeCoverage.Dispose correctly passes null (the captured previous value when the env var was absent) to SetEnvironmentVariable, which is the documented way to delete the variable — no resource-leak risk.
• The System.Configuration explicit reference is correctly scoped to $(NetFrameworkMinimum) only — confirmed that MSTestAdapter.PlatformServices ships exactly one .NET Framework TFM (net462), so no framework TFM is missed.
• ObjectModelDecouplingTests correctly uses AwesomeAssertions (required by this project's BannedSymbols.txt), uses IndexOf instead of string.Contains(string, StringComparison) for .NET Framework compat, and verifies the compile-time manifest references — the right API for the stated contract.

[Copilot]

Pull request overview

This PR is the final slice of a multi-PR initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. It removes the Microsoft.TestPlatform.ObjectModel package reference from the production assembly, moving all remaining VSTest coupling up into the MSTest.TestAdapter layer. The work is a faithful, no-behavior-change decoupling, locked in by a new guard test.

I verified the key correctness claims: the new local ApartmentStateSetting { MTA, STA } enum exactly mirrors VSTest's PlatformApartmentState (MTA=0, STA=1, confirmed from the vstest source), so Enum.TryParse numeric-string compatibility is preserved on both the runsettings-XML and config paths; System.Configuration usage is entirely #if NETFRAMEWORK-guarded with net462 being the only netfx TFM; the UWP ObjectModel reference is consistent with VSTestBridge being excluded for UwpMinimum; and the guard test's assumption holds (MSTest's framework assemblies are named MSTest.TestFramework*, which don't contain "TestPlatform").

Changes:

• Remove the Microsoft.TestPlatform.ObjectModel package reference from PlatformServices; replace the last VSTest enum consumer with a local neutral ApartmentStateSetting, and add a direct System.Configuration framework reference on .NET Framework.
• Add explicit Microsoft.TestPlatform.ObjectModel references where the transitive path is now gone (MSTest.TestAdapter for UWP; the Desktop integration test project which uses XmlRunSettingsUtilities directly).
• Add ObjectModelDecouplingTests guard asserting the compiled PlatformServices assembly references no *TestPlatform* assembly; harden TestSourceHandler public-key-token comparison against null/empty tokens.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/ApartmentStateSetting.cs	New neutral enum replacing VSTest PlatformApartmentState, with load-bearing member order documented
src/Adapter/MSTestAdapter.PlatformServices/RunConfigurationSettings.cs	Parse apartment state via local enum on both XML and config paths
src/Adapter/MSTestAdapter.PlatformServices/MSTestAdapter.PlatformServices.csproj	Remove ObjectModel package ref; add netfx-only System.Configuration reference
src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHandler.cs	Make public-key-token comparison null/empty-safe
src/Adapter/MSTest.TestAdapter/MSTest.TestAdapter.csproj	Add explicit ObjectModel reference for UWP (VSTestBridge excluded there)
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/ObjectModelDecouplingTests.cs	New guard test enforcing the platform-neutral contract
test/IntegrationTests/PlatformServices.Desktop.IntegrationTests/PlatformServices.Desktop.IntegrationTests.csproj	Add direct ObjectModel package ref for XmlRunSettingsUtilities

Review details

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

[github-actions]

🧪 Test quality grade — PR #9633

Grade	Test	Notes
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenDiffers	Boundary test: differing non-null token; byte-array magic explained by comment; clean AAA. No issues found.
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenIsMissing	Boundary test: null public-key-token case; .Should().BeFalse() is complete for a bool return. No issues found.
A (90–100)	new ObjectModelDecouplingTests. PlatformServicesAssemblyShouldNotReferenceAnyTestPlatformAssembly	Contract guard via reflection with a well-messaged .Should().BeEmpty(). 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. · 61.4 AIC · ⌖ 11.1 AIC · ⊞ 9.5K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

Phase 7 (capstone) — drop the ObjectModel package reference

The final slice of the initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. After this, PlatformServices no longer references the VSTest object model (Microsoft.TestPlatform.ObjectModel) at all — every VSTest dependency now lives in the MSTest.TestAdapter layer above it. Strict byte-for-byte, no behavior change.

Production changes

1. Remove the Microsoft.TestPlatform.ObjectModelPackageReference from MSTestAdapter.PlatformServices.csproj.
2. RunConfigurationSettings was the last consumer of a transitively-provided VSTest package: it used PlatformAbstractions' PlatformApartmentState enum {MTA, STA} to parse ExecutionThreadApartmentState. Replaced with a local internal enum ApartmentStateSetting { MTA, STA } of the same shape (same member names and order — MTA=0, STA=1), preserving the exact Enum.TryParse → STA/MTA → System.Threading.ApartmentState / else-throw behavior byte-for-byte on both the runsettings-XML and config paths. The member order is load-bearing (Enum.TryParse accepts numeric strings "0"/"1", so the numbering must match) and is commented as such. (Parsing directly to System.Threading.ApartmentState would change the handling of the Unknown value — it has that extra member — so a faithful 2-member local enum is required. The property type stays the BCL System.Threading.ApartmentState; only the parse-enum is neutralized.)
3. Add a direct System.Configurationframework reference on .NET Framework. ConfigurationManager/ConfigurationElementCollection (used by TestDataSource) were previously pulled in transitively via the object-model package; System.Configuration is a framework assembly, so it is now referenced directly.

Result: PlatformServices is fully platform-neutral

The compiled MSTestAdapter.PlatformServices assembly has zero references to any Microsoft.*.TestPlatform.* assembly on every real TFM (net462/net8.0/net9.0 + windows variants), verified via assembly metadata.

Guard test (the finish line)

ObjectModelDecouplingTests asserts the compiled PlatformServices assembly references no assembly whose name contains "TestPlatform" — catching ObjectModel, PlatformAbstractions, CoreUtilities, etc. (MSTest's own framework is MSTest.TestFramework, which doesn't match). This locks the platform-agnostic contract permanently.

Test-project fix

PlatformServices.Desktop.IntegrationTests uses ObjectModel's XmlRunSettingsUtilities directly (previously transitive through the PlatformServices project reference), so it gets its own direct Microsoft.TestPlatform.ObjectModelPackageReference. Test projects may reference the object model; only the production assembly must be neutral.

Verification

• All real TFMs build 0-warning (UWP builds via full msbuild in CI).
• Compiled dll: zeroTestPlatform references (net462 + net8.0, metadata-verified).
• MSTestAdapter.PlatformServices.UnitTests: 936/936 (net462), 898/898 (net8.0) — includes the new guard test and the STA/MTA parsing tests on both the runsettings-XML and config paths.
• PlatformServices.Desktop.IntegrationTests: 15/15.
• MSTestAdapter.UnitTests: 21/21. MSTest.TestAdapter and MSTest.IntegrationTests build clean.
• Expert-reviewer pass.

Stacking

Stacks on #9632 (Phase 6e-4c3); base branch dev/amauryleve/vstest-decoupling-suspendcoverage. Review/merge after the earlier PRs in the chain reach the base. Do not squash-rebase the base.

[github-actions]

Note

🤖 Automated review 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 Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

PR #9633 — VSTest ObjectModel decoupling from PlatformServices

#	Dimension	Verdict
1	Algorithmic Correctness	⚠️ 1 MODERATE
2	Threading & Concurrency	✅ LGTM
3	Security & IPC Contract Safety	✅ LGTM
4	Public API & Binary Compatibility	✅ LGTM
5	Performance & Allocations	✅ LGTM
6	Cross-TFM Compatibility	✅ LGTM
7	Resource & IDisposable Management	✅ LGTM
8	Defensive Coding at Boundaries	✅ LGTM (covered by #1)
9	Localization & Resources	✅ LGTM
10	Test Isolation	✅ LGTM
11	Assertion Quality	✅ LGTM
12	Flakiness Patterns	✅ LGTM
13	Test Completeness	⚠️ 1 MODERATE
14	Data-Driven Test Coverage	N/A
15	Code Structure & Simplification	✅ LGTM
16	Naming & Conventions	✅ LGTM
17	Documentation Accuracy	✅ LGTM
18	Analyzer & Code Fix Quality	N/A
19	IPC Wire Compatibility	N/A
20	Build Infrastructure & Dependencies	✅ LGTM
21	Scope & PR Discipline	✅ LGTM
22	PowerShell Scripting Hygiene	N/A

✅ 17/18 applicable dimensions clean.

---

Findings

• Algorithmic Correctness (MODERATE) — ArePublicKeyTokensEqual(byte[] left, byte[] right) in TestSourceHandler.cs line 142 dereferences both parameters unconditionally. AssemblyName.GetPublicKeyToken() returns null for unsigned assemblies, producing a NullReferenceException that is silently caught and converted to the conservative null → true path (false-positive discovery) rather than the correct false. See inline comment for the fix (annotate byte[]? and add a null guard at the top of the helper).

• Test Completeness (MODERATE) — The new SuspendCodeCoverage class (Utilities/SuspendCodeCoverage.cs) has no unit tests. Three behaviours are testable and could silently regress on .NET Framework TFMs without coverage: (1) constructor saves the previous env-var value and sets "TRUE", (2) Dispose() restores the previous value (null → delete), (3) the double-dispose guard prevents a second restoration. Suggested location: a new SuspendCodeCoverageTests.cs in MSTestAdapter.PlatformServices.UnitTests, guarded with #if NETFRAMEWORK.

---

Notable positives

• The ApartmentStateSetting enum ordering (MTA=0, STA=1) correctly matches the old PlatformApartmentState numeric values, preserving parse compatibility for numeric run-settings strings — and the load-bearing comment explaining this is clear.
• SuspendCodeCoverage.Dispose correctly passes null (the captured previous value when the env var was absent) to SetEnvironmentVariable, which is the documented way to delete the variable — no resource-leak risk.
• The System.Configuration explicit reference is correctly scoped to $(NetFrameworkMinimum) only — confirmed that MSTestAdapter.PlatformServices ships exactly one .NET Framework TFM (net462), so no framework TFM is missed.
• ObjectModelDecouplingTests correctly uses AwesomeAssertions (required by this project's BannedSymbols.txt), uses IndexOf instead of string.Contains(string, StringComparison) for .NET Framework compat, and verifies the compile-time manifest references — the right API for the stated contract.

[Copilot]

Pull request overview

This PR is the final slice of a multi-PR initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. It removes the Microsoft.TestPlatform.ObjectModel package reference from the production assembly, moving all remaining VSTest coupling up into the MSTest.TestAdapter layer. The work is a faithful, no-behavior-change decoupling, locked in by a new guard test.

I verified the key correctness claims: the new local ApartmentStateSetting { MTA, STA } enum exactly mirrors VSTest's PlatformApartmentState (MTA=0, STA=1, confirmed from the vstest source), so Enum.TryParse numeric-string compatibility is preserved on both the runsettings-XML and config paths; System.Configuration usage is entirely #if NETFRAMEWORK-guarded with net462 being the only netfx TFM; the UWP ObjectModel reference is consistent with VSTestBridge being excluded for UwpMinimum; and the guard test's assumption holds (MSTest's framework assemblies are named MSTest.TestFramework*, which don't contain "TestPlatform").

Changes:

• Remove the Microsoft.TestPlatform.ObjectModel package reference from PlatformServices; replace the last VSTest enum consumer with a local neutral ApartmentStateSetting, and add a direct System.Configuration framework reference on .NET Framework.
• Add explicit Microsoft.TestPlatform.ObjectModel references where the transitive path is now gone (MSTest.TestAdapter for UWP; the Desktop integration test project which uses XmlRunSettingsUtilities directly).
• Add ObjectModelDecouplingTests guard asserting the compiled PlatformServices assembly references no *TestPlatform* assembly; harden TestSourceHandler public-key-token comparison against null/empty tokens.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/ApartmentStateSetting.cs	New neutral enum replacing VSTest PlatformApartmentState, with load-bearing member order documented
src/Adapter/MSTestAdapter.PlatformServices/RunConfigurationSettings.cs	Parse apartment state via local enum on both XML and config paths
src/Adapter/MSTestAdapter.PlatformServices/MSTestAdapter.PlatformServices.csproj	Remove ObjectModel package ref; add netfx-only System.Configuration reference
src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHandler.cs	Make public-key-token comparison null/empty-safe
src/Adapter/MSTest.TestAdapter/MSTest.TestAdapter.csproj	Add explicit ObjectModel reference for UWP (VSTestBridge excluded there)
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/ObjectModelDecouplingTests.cs	New guard test enforcing the platform-neutral contract
test/IntegrationTests/PlatformServices.Desktop.IntegrationTests/PlatformServices.Desktop.IntegrationTests.csproj	Add direct ObjectModel package ref for XmlRunSettingsUtilities

Review details

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

[github-actions]

🧪 Test quality grade — PR #9633

Grade	Test	Notes
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenDiffers	Boundary test: differing non-null token; byte-array magic explained by comment; clean AAA. No issues found.
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenIsMissing	Boundary test: null public-key-token case; .Should().BeFalse() is complete for a bool return. No issues found.
A (90–100)	new ObjectModelDecouplingTests. PlatformServicesAssemblyShouldNotReferenceAnyTestPlatformAssembly	Contract guard via reflection with a well-messaged .Should().BeEmpty(). 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. · 61.4 AIC · ⌖ 11.1 AIC · ⊞ 9.5K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

Phase 7 (capstone) — drop the ObjectModel package reference

The final slice of the initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. After this, PlatformServices no longer references the VSTest object model (Microsoft.TestPlatform.ObjectModel) at all — every VSTest dependency now lives in the MSTest.TestAdapter layer above it. Strict byte-for-byte, no behavior change.

Production changes

1. Remove the Microsoft.TestPlatform.ObjectModelPackageReference from MSTestAdapter.PlatformServices.csproj.
2. RunConfigurationSettings was the last consumer of a transitively-provided VSTest package: it used PlatformAbstractions' PlatformApartmentState enum {MTA, STA} to parse ExecutionThreadApartmentState. Replaced with a local internal enum ApartmentStateSetting { MTA, STA } of the same shape (same member names and order — MTA=0, STA=1), preserving the exact Enum.TryParse → STA/MTA → System.Threading.ApartmentState / else-throw behavior byte-for-byte on both the runsettings-XML and config paths. The member order is load-bearing (Enum.TryParse accepts numeric strings "0"/"1", so the numbering must match) and is commented as such. (Parsing directly to System.Threading.ApartmentState would change the handling of the Unknown value — it has that extra member — so a faithful 2-member local enum is required. The property type stays the BCL System.Threading.ApartmentState; only the parse-enum is neutralized.)
3. Add a direct System.Configurationframework reference on .NET Framework. ConfigurationManager/ConfigurationElementCollection (used by TestDataSource) were previously pulled in transitively via the object-model package; System.Configuration is a framework assembly, so it is now referenced directly.

Result: PlatformServices is fully platform-neutral

The compiled MSTestAdapter.PlatformServices assembly has zero references to any Microsoft.*.TestPlatform.* assembly on every real TFM (net462/net8.0/net9.0 + windows variants), verified via assembly metadata.

Guard test (the finish line)

ObjectModelDecouplingTests asserts the compiled PlatformServices assembly references no assembly whose name contains "TestPlatform" — catching ObjectModel, PlatformAbstractions, CoreUtilities, etc. (MSTest's own framework is MSTest.TestFramework, which doesn't match). This locks the platform-agnostic contract permanently.

Test-project fix

PlatformServices.Desktop.IntegrationTests uses ObjectModel's XmlRunSettingsUtilities directly (previously transitive through the PlatformServices project reference), so it gets its own direct Microsoft.TestPlatform.ObjectModelPackageReference. Test projects may reference the object model; only the production assembly must be neutral.

Verification

• All real TFMs build 0-warning (UWP builds via full msbuild in CI).
• Compiled dll: zeroTestPlatform references (net462 + net8.0, metadata-verified).
• MSTestAdapter.PlatformServices.UnitTests: 936/936 (net462), 898/898 (net8.0) — includes the new guard test and the STA/MTA parsing tests on both the runsettings-XML and config paths.
• PlatformServices.Desktop.IntegrationTests: 15/15.
• MSTestAdapter.UnitTests: 21/21. MSTest.TestAdapter and MSTest.IntegrationTests build clean.
• Expert-reviewer pass.

Stacking

Stacks on #9632 (Phase 6e-4c3); base branch dev/amauryleve/vstest-decoupling-suspendcoverage. Review/merge after the earlier PRs in the chain reach the base. Do not squash-rebase the base.

[github-actions]

Note

🤖 Automated review 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 Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

PR #9633 — VSTest ObjectModel decoupling from PlatformServices

#	Dimension	Verdict
1	Algorithmic Correctness	⚠️ 1 MODERATE
2	Threading & Concurrency	✅ LGTM
3	Security & IPC Contract Safety	✅ LGTM
4	Public API & Binary Compatibility	✅ LGTM
5	Performance & Allocations	✅ LGTM
6	Cross-TFM Compatibility	✅ LGTM
7	Resource & IDisposable Management	✅ LGTM
8	Defensive Coding at Boundaries	✅ LGTM (covered by #1)
9	Localization & Resources	✅ LGTM
10	Test Isolation	✅ LGTM
11	Assertion Quality	✅ LGTM
12	Flakiness Patterns	✅ LGTM
13	Test Completeness	⚠️ 1 MODERATE
14	Data-Driven Test Coverage	N/A
15	Code Structure & Simplification	✅ LGTM
16	Naming & Conventions	✅ LGTM
17	Documentation Accuracy	✅ LGTM
18	Analyzer & Code Fix Quality	N/A
19	IPC Wire Compatibility	N/A
20	Build Infrastructure & Dependencies	✅ LGTM
21	Scope & PR Discipline	✅ LGTM
22	PowerShell Scripting Hygiene	N/A

✅ 17/18 applicable dimensions clean.

---

Findings

• Algorithmic Correctness (MODERATE) — ArePublicKeyTokensEqual(byte[] left, byte[] right) in TestSourceHandler.cs line 142 dereferences both parameters unconditionally. AssemblyName.GetPublicKeyToken() returns null for unsigned assemblies, producing a NullReferenceException that is silently caught and converted to the conservative null → true path (false-positive discovery) rather than the correct false. See inline comment for the fix (annotate byte[]? and add a null guard at the top of the helper).

• Test Completeness (MODERATE) — The new SuspendCodeCoverage class (Utilities/SuspendCodeCoverage.cs) has no unit tests. Three behaviours are testable and could silently regress on .NET Framework TFMs without coverage: (1) constructor saves the previous env-var value and sets "TRUE", (2) Dispose() restores the previous value (null → delete), (3) the double-dispose guard prevents a second restoration. Suggested location: a new SuspendCodeCoverageTests.cs in MSTestAdapter.PlatformServices.UnitTests, guarded with #if NETFRAMEWORK.

---

Notable positives

• The ApartmentStateSetting enum ordering (MTA=0, STA=1) correctly matches the old PlatformApartmentState numeric values, preserving parse compatibility for numeric run-settings strings — and the load-bearing comment explaining this is clear.
• SuspendCodeCoverage.Dispose correctly passes null (the captured previous value when the env var was absent) to SetEnvironmentVariable, which is the documented way to delete the variable — no resource-leak risk.
• The System.Configuration explicit reference is correctly scoped to $(NetFrameworkMinimum) only — confirmed that MSTestAdapter.PlatformServices ships exactly one .NET Framework TFM (net462), so no framework TFM is missed.
• ObjectModelDecouplingTests correctly uses AwesomeAssertions (required by this project's BannedSymbols.txt), uses IndexOf instead of string.Contains(string, StringComparison) for .NET Framework compat, and verifies the compile-time manifest references — the right API for the stated contract.

[Copilot]

Pull request overview

This PR is the final slice of a multi-PR initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. It removes the Microsoft.TestPlatform.ObjectModel package reference from the production assembly, moving all remaining VSTest coupling up into the MSTest.TestAdapter layer. The work is a faithful, no-behavior-change decoupling, locked in by a new guard test.

I verified the key correctness claims: the new local ApartmentStateSetting { MTA, STA } enum exactly mirrors VSTest's PlatformApartmentState (MTA=0, STA=1, confirmed from the vstest source), so Enum.TryParse numeric-string compatibility is preserved on both the runsettings-XML and config paths; System.Configuration usage is entirely #if NETFRAMEWORK-guarded with net462 being the only netfx TFM; the UWP ObjectModel reference is consistent with VSTestBridge being excluded for UwpMinimum; and the guard test's assumption holds (MSTest's framework assemblies are named MSTest.TestFramework*, which don't contain "TestPlatform").

Changes:

• Remove the Microsoft.TestPlatform.ObjectModel package reference from PlatformServices; replace the last VSTest enum consumer with a local neutral ApartmentStateSetting, and add a direct System.Configuration framework reference on .NET Framework.
• Add explicit Microsoft.TestPlatform.ObjectModel references where the transitive path is now gone (MSTest.TestAdapter for UWP; the Desktop integration test project which uses XmlRunSettingsUtilities directly).
• Add ObjectModelDecouplingTests guard asserting the compiled PlatformServices assembly references no *TestPlatform* assembly; harden TestSourceHandler public-key-token comparison against null/empty tokens.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/ApartmentStateSetting.cs	New neutral enum replacing VSTest PlatformApartmentState, with load-bearing member order documented
src/Adapter/MSTestAdapter.PlatformServices/RunConfigurationSettings.cs	Parse apartment state via local enum on both XML and config paths
src/Adapter/MSTestAdapter.PlatformServices/MSTestAdapter.PlatformServices.csproj	Remove ObjectModel package ref; add netfx-only System.Configuration reference
src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHandler.cs	Make public-key-token comparison null/empty-safe
src/Adapter/MSTest.TestAdapter/MSTest.TestAdapter.csproj	Add explicit ObjectModel reference for UWP (VSTestBridge excluded there)
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/ObjectModelDecouplingTests.cs	New guard test enforcing the platform-neutral contract
test/IntegrationTests/PlatformServices.Desktop.IntegrationTests/PlatformServices.Desktop.IntegrationTests.csproj	Add direct ObjectModel package ref for XmlRunSettingsUtilities

Review details

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

[github-actions]

🧪 Test quality grade — PR #9633

Grade	Test	Notes
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenDiffers	Boundary test: differing non-null token; byte-array magic explained by comment; clean AAA. No issues found.
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenIsMissing	Boundary test: null public-key-token case; .Should().BeFalse() is complete for a bool return. No issues found.
A (90–100)	new ObjectModelDecouplingTests. PlatformServicesAssemblyShouldNotReferenceAnyTestPlatformAssembly	Contract guard via reflection with a well-messaged .Should().BeEmpty(). 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. · 61.4 AIC · ⌖ 11.1 AIC · ⊞ 9.5K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

Phase 7 (capstone) — drop the ObjectModel package reference

The final slice of the initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. After this, PlatformServices no longer references the VSTest object model (Microsoft.TestPlatform.ObjectModel) at all — every VSTest dependency now lives in the MSTest.TestAdapter layer above it. Strict byte-for-byte, no behavior change.

Production changes

1. Remove the Microsoft.TestPlatform.ObjectModelPackageReference from MSTestAdapter.PlatformServices.csproj.
2. RunConfigurationSettings was the last consumer of a transitively-provided VSTest package: it used PlatformAbstractions' PlatformApartmentState enum {MTA, STA} to parse ExecutionThreadApartmentState. Replaced with a local internal enum ApartmentStateSetting { MTA, STA } of the same shape (same member names and order — MTA=0, STA=1), preserving the exact Enum.TryParse → STA/MTA → System.Threading.ApartmentState / else-throw behavior byte-for-byte on both the runsettings-XML and config paths. The member order is load-bearing (Enum.TryParse accepts numeric strings "0"/"1", so the numbering must match) and is commented as such. (Parsing directly to System.Threading.ApartmentState would change the handling of the Unknown value — it has that extra member — so a faithful 2-member local enum is required. The property type stays the BCL System.Threading.ApartmentState; only the parse-enum is neutralized.)
3. Add a direct System.Configurationframework reference on .NET Framework. ConfigurationManager/ConfigurationElementCollection (used by TestDataSource) were previously pulled in transitively via the object-model package; System.Configuration is a framework assembly, so it is now referenced directly.

Result: PlatformServices is fully platform-neutral

The compiled MSTestAdapter.PlatformServices assembly has zero references to any Microsoft.*.TestPlatform.* assembly on every real TFM (net462/net8.0/net9.0 + windows variants), verified via assembly metadata.

Guard test (the finish line)

ObjectModelDecouplingTests asserts the compiled PlatformServices assembly references no assembly whose name contains "TestPlatform" — catching ObjectModel, PlatformAbstractions, CoreUtilities, etc. (MSTest's own framework is MSTest.TestFramework, which doesn't match). This locks the platform-agnostic contract permanently.

Test-project fix

PlatformServices.Desktop.IntegrationTests uses ObjectModel's XmlRunSettingsUtilities directly (previously transitive through the PlatformServices project reference), so it gets its own direct Microsoft.TestPlatform.ObjectModelPackageReference. Test projects may reference the object model; only the production assembly must be neutral.

Verification

• All real TFMs build 0-warning (UWP builds via full msbuild in CI).
• Compiled dll: zeroTestPlatform references (net462 + net8.0, metadata-verified).
• MSTestAdapter.PlatformServices.UnitTests: 936/936 (net462), 898/898 (net8.0) — includes the new guard test and the STA/MTA parsing tests on both the runsettings-XML and config paths.
• PlatformServices.Desktop.IntegrationTests: 15/15.
• MSTestAdapter.UnitTests: 21/21. MSTest.TestAdapter and MSTest.IntegrationTests build clean.
• Expert-reviewer pass.

Stacking

Stacks on #9632 (Phase 6e-4c3); base branch dev/amauryleve/vstest-decoupling-suspendcoverage. Review/merge after the earlier PRs in the chain reach the base. Do not squash-rebase the base.

[github-actions]

Note

🤖 Automated review 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 Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

PR #9633 — VSTest ObjectModel decoupling from PlatformServices

#	Dimension	Verdict
1	Algorithmic Correctness	⚠️ 1 MODERATE
2	Threading & Concurrency	✅ LGTM
3	Security & IPC Contract Safety	✅ LGTM
4	Public API & Binary Compatibility	✅ LGTM
5	Performance & Allocations	✅ LGTM
6	Cross-TFM Compatibility	✅ LGTM
7	Resource & IDisposable Management	✅ LGTM
8	Defensive Coding at Boundaries	✅ LGTM (covered by #1)
9	Localization & Resources	✅ LGTM
10	Test Isolation	✅ LGTM
11	Assertion Quality	✅ LGTM
12	Flakiness Patterns	✅ LGTM
13	Test Completeness	⚠️ 1 MODERATE
14	Data-Driven Test Coverage	N/A
15	Code Structure & Simplification	✅ LGTM
16	Naming & Conventions	✅ LGTM
17	Documentation Accuracy	✅ LGTM
18	Analyzer & Code Fix Quality	N/A
19	IPC Wire Compatibility	N/A
20	Build Infrastructure & Dependencies	✅ LGTM
21	Scope & PR Discipline	✅ LGTM
22	PowerShell Scripting Hygiene	N/A

✅ 17/18 applicable dimensions clean.

---

Findings

• Algorithmic Correctness (MODERATE) — ArePublicKeyTokensEqual(byte[] left, byte[] right) in TestSourceHandler.cs line 142 dereferences both parameters unconditionally. AssemblyName.GetPublicKeyToken() returns null for unsigned assemblies, producing a NullReferenceException that is silently caught and converted to the conservative null → true path (false-positive discovery) rather than the correct false. See inline comment for the fix (annotate byte[]? and add a null guard at the top of the helper).

• Test Completeness (MODERATE) — The new SuspendCodeCoverage class (Utilities/SuspendCodeCoverage.cs) has no unit tests. Three behaviours are testable and could silently regress on .NET Framework TFMs without coverage: (1) constructor saves the previous env-var value and sets "TRUE", (2) Dispose() restores the previous value (null → delete), (3) the double-dispose guard prevents a second restoration. Suggested location: a new SuspendCodeCoverageTests.cs in MSTestAdapter.PlatformServices.UnitTests, guarded with #if NETFRAMEWORK.

---

Notable positives

• The ApartmentStateSetting enum ordering (MTA=0, STA=1) correctly matches the old PlatformApartmentState numeric values, preserving parse compatibility for numeric run-settings strings — and the load-bearing comment explaining this is clear.
• SuspendCodeCoverage.Dispose correctly passes null (the captured previous value when the env var was absent) to SetEnvironmentVariable, which is the documented way to delete the variable — no resource-leak risk.
• The System.Configuration explicit reference is correctly scoped to $(NetFrameworkMinimum) only — confirmed that MSTestAdapter.PlatformServices ships exactly one .NET Framework TFM (net462), so no framework TFM is missed.
• ObjectModelDecouplingTests correctly uses AwesomeAssertions (required by this project's BannedSymbols.txt), uses IndexOf instead of string.Contains(string, StringComparison) for .NET Framework compat, and verifies the compile-time manifest references — the right API for the stated contract.

[Copilot]

Pull request overview

This PR is the final slice of a multi-PR initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. It removes the Microsoft.TestPlatform.ObjectModel package reference from the production assembly, moving all remaining VSTest coupling up into the MSTest.TestAdapter layer. The work is a faithful, no-behavior-change decoupling, locked in by a new guard test.

I verified the key correctness claims: the new local ApartmentStateSetting { MTA, STA } enum exactly mirrors VSTest's PlatformApartmentState (MTA=0, STA=1, confirmed from the vstest source), so Enum.TryParse numeric-string compatibility is preserved on both the runsettings-XML and config paths; System.Configuration usage is entirely #if NETFRAMEWORK-guarded with net462 being the only netfx TFM; the UWP ObjectModel reference is consistent with VSTestBridge being excluded for UwpMinimum; and the guard test's assumption holds (MSTest's framework assemblies are named MSTest.TestFramework*, which don't contain "TestPlatform").

Changes:

• Remove the Microsoft.TestPlatform.ObjectModel package reference from PlatformServices; replace the last VSTest enum consumer with a local neutral ApartmentStateSetting, and add a direct System.Configuration framework reference on .NET Framework.
• Add explicit Microsoft.TestPlatform.ObjectModel references where the transitive path is now gone (MSTest.TestAdapter for UWP; the Desktop integration test project which uses XmlRunSettingsUtilities directly).
• Add ObjectModelDecouplingTests guard asserting the compiled PlatformServices assembly references no *TestPlatform* assembly; harden TestSourceHandler public-key-token comparison against null/empty tokens.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/ApartmentStateSetting.cs	New neutral enum replacing VSTest PlatformApartmentState, with load-bearing member order documented
src/Adapter/MSTestAdapter.PlatformServices/RunConfigurationSettings.cs	Parse apartment state via local enum on both XML and config paths
src/Adapter/MSTestAdapter.PlatformServices/MSTestAdapter.PlatformServices.csproj	Remove ObjectModel package ref; add netfx-only System.Configuration reference
src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHandler.cs	Make public-key-token comparison null/empty-safe
src/Adapter/MSTest.TestAdapter/MSTest.TestAdapter.csproj	Add explicit ObjectModel reference for UWP (VSTestBridge excluded there)
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/ObjectModelDecouplingTests.cs	New guard test enforcing the platform-neutral contract
test/IntegrationTests/PlatformServices.Desktop.IntegrationTests/PlatformServices.Desktop.IntegrationTests.csproj	Add direct ObjectModel package ref for XmlRunSettingsUtilities

Review details

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

[github-actions]

🧪 Test quality grade — PR #9633

Grade	Test	Notes
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenDiffers	Boundary test: differing non-null token; byte-array magic explained by comment; clean AAA. No issues found.
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenIsMissing	Boundary test: null public-key-token case; .Should().BeFalse() is complete for a bool return. No issues found.
A (90–100)	new ObjectModelDecouplingTests. PlatformServicesAssemblyShouldNotReferenceAnyTestPlatformAssembly	Contract guard via reflection with a well-messaged .Should().BeEmpty(). 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. · 61.4 AIC · ⌖ 11.1 AIC · ⊞ 9.5K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

Phase 7 (capstone) — drop the ObjectModel package reference

The final slice of the initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. After this, PlatformServices no longer references the VSTest object model (Microsoft.TestPlatform.ObjectModel) at all — every VSTest dependency now lives in the MSTest.TestAdapter layer above it. Strict byte-for-byte, no behavior change.

Production changes

1. Remove the Microsoft.TestPlatform.ObjectModelPackageReference from MSTestAdapter.PlatformServices.csproj.
2. RunConfigurationSettings was the last consumer of a transitively-provided VSTest package: it used PlatformAbstractions' PlatformApartmentState enum {MTA, STA} to parse ExecutionThreadApartmentState. Replaced with a local internal enum ApartmentStateSetting { MTA, STA } of the same shape (same member names and order — MTA=0, STA=1), preserving the exact Enum.TryParse → STA/MTA → System.Threading.ApartmentState / else-throw behavior byte-for-byte on both the runsettings-XML and config paths. The member order is load-bearing (Enum.TryParse accepts numeric strings "0"/"1", so the numbering must match) and is commented as such. (Parsing directly to System.Threading.ApartmentState would change the handling of the Unknown value — it has that extra member — so a faithful 2-member local enum is required. The property type stays the BCL System.Threading.ApartmentState; only the parse-enum is neutralized.)
3. Add a direct System.Configurationframework reference on .NET Framework. ConfigurationManager/ConfigurationElementCollection (used by TestDataSource) were previously pulled in transitively via the object-model package; System.Configuration is a framework assembly, so it is now referenced directly.

Result: PlatformServices is fully platform-neutral

The compiled MSTestAdapter.PlatformServices assembly has zero references to any Microsoft.*.TestPlatform.* assembly on every real TFM (net462/net8.0/net9.0 + windows variants), verified via assembly metadata.

Guard test (the finish line)

ObjectModelDecouplingTests asserts the compiled PlatformServices assembly references no assembly whose name contains "TestPlatform" — catching ObjectModel, PlatformAbstractions, CoreUtilities, etc. (MSTest's own framework is MSTest.TestFramework, which doesn't match). This locks the platform-agnostic contract permanently.

Test-project fix

PlatformServices.Desktop.IntegrationTests uses ObjectModel's XmlRunSettingsUtilities directly (previously transitive through the PlatformServices project reference), so it gets its own direct Microsoft.TestPlatform.ObjectModelPackageReference. Test projects may reference the object model; only the production assembly must be neutral.

Verification

• All real TFMs build 0-warning (UWP builds via full msbuild in CI).
• Compiled dll: zeroTestPlatform references (net462 + net8.0, metadata-verified).
• MSTestAdapter.PlatformServices.UnitTests: 936/936 (net462), 898/898 (net8.0) — includes the new guard test and the STA/MTA parsing tests on both the runsettings-XML and config paths.
• PlatformServices.Desktop.IntegrationTests: 15/15.
• MSTestAdapter.UnitTests: 21/21. MSTest.TestAdapter and MSTest.IntegrationTests build clean.
• Expert-reviewer pass.

Stacking

Stacks on #9632 (Phase 6e-4c3); base branch dev/amauryleve/vstest-decoupling-suspendcoverage. Review/merge after the earlier PRs in the chain reach the base. Do not squash-rebase the base.

[github-actions]

Note

🤖 Automated review 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 Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

PR #9633 — VSTest ObjectModel decoupling from PlatformServices

#	Dimension	Verdict
1	Algorithmic Correctness	⚠️ 1 MODERATE
2	Threading & Concurrency	✅ LGTM
3	Security & IPC Contract Safety	✅ LGTM
4	Public API & Binary Compatibility	✅ LGTM
5	Performance & Allocations	✅ LGTM
6	Cross-TFM Compatibility	✅ LGTM
7	Resource & IDisposable Management	✅ LGTM
8	Defensive Coding at Boundaries	✅ LGTM (covered by #1)
9	Localization & Resources	✅ LGTM
10	Test Isolation	✅ LGTM
11	Assertion Quality	✅ LGTM
12	Flakiness Patterns	✅ LGTM
13	Test Completeness	⚠️ 1 MODERATE
14	Data-Driven Test Coverage	N/A
15	Code Structure & Simplification	✅ LGTM
16	Naming & Conventions	✅ LGTM
17	Documentation Accuracy	✅ LGTM
18	Analyzer & Code Fix Quality	N/A
19	IPC Wire Compatibility	N/A
20	Build Infrastructure & Dependencies	✅ LGTM
21	Scope & PR Discipline	✅ LGTM
22	PowerShell Scripting Hygiene	N/A

✅ 17/18 applicable dimensions clean.

---

Findings

• Algorithmic Correctness (MODERATE) — ArePublicKeyTokensEqual(byte[] left, byte[] right) in TestSourceHandler.cs line 142 dereferences both parameters unconditionally. AssemblyName.GetPublicKeyToken() returns null for unsigned assemblies, producing a NullReferenceException that is silently caught and converted to the conservative null → true path (false-positive discovery) rather than the correct false. See inline comment for the fix (annotate byte[]? and add a null guard at the top of the helper).

• Test Completeness (MODERATE) — The new SuspendCodeCoverage class (Utilities/SuspendCodeCoverage.cs) has no unit tests. Three behaviours are testable and could silently regress on .NET Framework TFMs without coverage: (1) constructor saves the previous env-var value and sets "TRUE", (2) Dispose() restores the previous value (null → delete), (3) the double-dispose guard prevents a second restoration. Suggested location: a new SuspendCodeCoverageTests.cs in MSTestAdapter.PlatformServices.UnitTests, guarded with #if NETFRAMEWORK.

---

Notable positives

• The ApartmentStateSetting enum ordering (MTA=0, STA=1) correctly matches the old PlatformApartmentState numeric values, preserving parse compatibility for numeric run-settings strings — and the load-bearing comment explaining this is clear.
• SuspendCodeCoverage.Dispose correctly passes null (the captured previous value when the env var was absent) to SetEnvironmentVariable, which is the documented way to delete the variable — no resource-leak risk.
• The System.Configuration explicit reference is correctly scoped to $(NetFrameworkMinimum) only — confirmed that MSTestAdapter.PlatformServices ships exactly one .NET Framework TFM (net462), so no framework TFM is missed.
• ObjectModelDecouplingTests correctly uses AwesomeAssertions (required by this project's BannedSymbols.txt), uses IndexOf instead of string.Contains(string, StringComparison) for .NET Framework compat, and verifies the compile-time manifest references — the right API for the stated contract.

[Copilot]

Pull request overview

This PR is the final slice of a multi-PR initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. It removes the Microsoft.TestPlatform.ObjectModel package reference from the production assembly, moving all remaining VSTest coupling up into the MSTest.TestAdapter layer. The work is a faithful, no-behavior-change decoupling, locked in by a new guard test.

I verified the key correctness claims: the new local ApartmentStateSetting { MTA, STA } enum exactly mirrors VSTest's PlatformApartmentState (MTA=0, STA=1, confirmed from the vstest source), so Enum.TryParse numeric-string compatibility is preserved on both the runsettings-XML and config paths; System.Configuration usage is entirely #if NETFRAMEWORK-guarded with net462 being the only netfx TFM; the UWP ObjectModel reference is consistent with VSTestBridge being excluded for UwpMinimum; and the guard test's assumption holds (MSTest's framework assemblies are named MSTest.TestFramework*, which don't contain "TestPlatform").

Changes:

• Remove the Microsoft.TestPlatform.ObjectModel package reference from PlatformServices; replace the last VSTest enum consumer with a local neutral ApartmentStateSetting, and add a direct System.Configuration framework reference on .NET Framework.
• Add explicit Microsoft.TestPlatform.ObjectModel references where the transitive path is now gone (MSTest.TestAdapter for UWP; the Desktop integration test project which uses XmlRunSettingsUtilities directly).
• Add ObjectModelDecouplingTests guard asserting the compiled PlatformServices assembly references no *TestPlatform* assembly; harden TestSourceHandler public-key-token comparison against null/empty tokens.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/ApartmentStateSetting.cs	New neutral enum replacing VSTest PlatformApartmentState, with load-bearing member order documented
src/Adapter/MSTestAdapter.PlatformServices/RunConfigurationSettings.cs	Parse apartment state via local enum on both XML and config paths
src/Adapter/MSTestAdapter.PlatformServices/MSTestAdapter.PlatformServices.csproj	Remove ObjectModel package ref; add netfx-only System.Configuration reference
src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHandler.cs	Make public-key-token comparison null/empty-safe
src/Adapter/MSTest.TestAdapter/MSTest.TestAdapter.csproj	Add explicit ObjectModel reference for UWP (VSTestBridge excluded there)
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/ObjectModelDecouplingTests.cs	New guard test enforcing the platform-neutral contract
test/IntegrationTests/PlatformServices.Desktop.IntegrationTests/PlatformServices.Desktop.IntegrationTests.csproj	Add direct ObjectModel package ref for XmlRunSettingsUtilities

Review details

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

[github-actions]

🧪 Test quality grade — PR #9633

Grade	Test	Notes
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenDiffers	Boundary test: differing non-null token; byte-array magic explained by comment; clean AAA. No issues found.
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenIsMissing	Boundary test: null public-key-token case; .Should().BeFalse() is complete for a bool return. No issues found.
A (90–100)	new ObjectModelDecouplingTests. PlatformServicesAssemblyShouldNotReferenceAnyTestPlatformAssembly	Contract guard via reflection with a well-messaged .Should().BeEmpty(). 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. · 61.4 AIC · ⌖ 11.1 AIC · ⊞ 9.5K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

Phase 7 (capstone) — drop the ObjectModel package reference

The final slice of the initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. After this, PlatformServices no longer references the VSTest object model (Microsoft.TestPlatform.ObjectModel) at all — every VSTest dependency now lives in the MSTest.TestAdapter layer above it. Strict byte-for-byte, no behavior change.

Production changes

1. Remove the Microsoft.TestPlatform.ObjectModelPackageReference from MSTestAdapter.PlatformServices.csproj.
2. RunConfigurationSettings was the last consumer of a transitively-provided VSTest package: it used PlatformAbstractions' PlatformApartmentState enum {MTA, STA} to parse ExecutionThreadApartmentState. Replaced with a local internal enum ApartmentStateSetting { MTA, STA } of the same shape (same member names and order — MTA=0, STA=1), preserving the exact Enum.TryParse → STA/MTA → System.Threading.ApartmentState / else-throw behavior byte-for-byte on both the runsettings-XML and config paths. The member order is load-bearing (Enum.TryParse accepts numeric strings "0"/"1", so the numbering must match) and is commented as such. (Parsing directly to System.Threading.ApartmentState would change the handling of the Unknown value — it has that extra member — so a faithful 2-member local enum is required. The property type stays the BCL System.Threading.ApartmentState; only the parse-enum is neutralized.)
3. Add a direct System.Configurationframework reference on .NET Framework. ConfigurationManager/ConfigurationElementCollection (used by TestDataSource) were previously pulled in transitively via the object-model package; System.Configuration is a framework assembly, so it is now referenced directly.

Result: PlatformServices is fully platform-neutral

The compiled MSTestAdapter.PlatformServices assembly has zero references to any Microsoft.*.TestPlatform.* assembly on every real TFM (net462/net8.0/net9.0 + windows variants), verified via assembly metadata.

Guard test (the finish line)

ObjectModelDecouplingTests asserts the compiled PlatformServices assembly references no assembly whose name contains "TestPlatform" — catching ObjectModel, PlatformAbstractions, CoreUtilities, etc. (MSTest's own framework is MSTest.TestFramework, which doesn't match). This locks the platform-agnostic contract permanently.

Test-project fix

PlatformServices.Desktop.IntegrationTests uses ObjectModel's XmlRunSettingsUtilities directly (previously transitive through the PlatformServices project reference), so it gets its own direct Microsoft.TestPlatform.ObjectModelPackageReference. Test projects may reference the object model; only the production assembly must be neutral.

Verification

• All real TFMs build 0-warning (UWP builds via full msbuild in CI).
• Compiled dll: zeroTestPlatform references (net462 + net8.0, metadata-verified).
• MSTestAdapter.PlatformServices.UnitTests: 936/936 (net462), 898/898 (net8.0) — includes the new guard test and the STA/MTA parsing tests on both the runsettings-XML and config paths.
• PlatformServices.Desktop.IntegrationTests: 15/15.
• MSTestAdapter.UnitTests: 21/21. MSTest.TestAdapter and MSTest.IntegrationTests build clean.
• Expert-reviewer pass.

Stacking

Stacks on #9632 (Phase 6e-4c3); base branch dev/amauryleve/vstest-decoupling-suspendcoverage. Review/merge after the earlier PRs in the chain reach the base. Do not squash-rebase the base.

[github-actions]

Note

🤖 Automated review 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 Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

PR #9633 — VSTest ObjectModel decoupling from PlatformServices

#	Dimension	Verdict
1	Algorithmic Correctness	⚠️ 1 MODERATE
2	Threading & Concurrency	✅ LGTM
3	Security & IPC Contract Safety	✅ LGTM
4	Public API & Binary Compatibility	✅ LGTM
5	Performance & Allocations	✅ LGTM
6	Cross-TFM Compatibility	✅ LGTM
7	Resource & IDisposable Management	✅ LGTM
8	Defensive Coding at Boundaries	✅ LGTM (covered by #1)
9	Localization & Resources	✅ LGTM
10	Test Isolation	✅ LGTM
11	Assertion Quality	✅ LGTM
12	Flakiness Patterns	✅ LGTM
13	Test Completeness	⚠️ 1 MODERATE
14	Data-Driven Test Coverage	N/A
15	Code Structure & Simplification	✅ LGTM
16	Naming & Conventions	✅ LGTM
17	Documentation Accuracy	✅ LGTM
18	Analyzer & Code Fix Quality	N/A
19	IPC Wire Compatibility	N/A
20	Build Infrastructure & Dependencies	✅ LGTM
21	Scope & PR Discipline	✅ LGTM
22	PowerShell Scripting Hygiene	N/A

✅ 17/18 applicable dimensions clean.

---

Findings

• Algorithmic Correctness (MODERATE) — ArePublicKeyTokensEqual(byte[] left, byte[] right) in TestSourceHandler.cs line 142 dereferences both parameters unconditionally. AssemblyName.GetPublicKeyToken() returns null for unsigned assemblies, producing a NullReferenceException that is silently caught and converted to the conservative null → true path (false-positive discovery) rather than the correct false. See inline comment for the fix (annotate byte[]? and add a null guard at the top of the helper).

• Test Completeness (MODERATE) — The new SuspendCodeCoverage class (Utilities/SuspendCodeCoverage.cs) has no unit tests. Three behaviours are testable and could silently regress on .NET Framework TFMs without coverage: (1) constructor saves the previous env-var value and sets "TRUE", (2) Dispose() restores the previous value (null → delete), (3) the double-dispose guard prevents a second restoration. Suggested location: a new SuspendCodeCoverageTests.cs in MSTestAdapter.PlatformServices.UnitTests, guarded with #if NETFRAMEWORK.

---

Notable positives

• The ApartmentStateSetting enum ordering (MTA=0, STA=1) correctly matches the old PlatformApartmentState numeric values, preserving parse compatibility for numeric run-settings strings — and the load-bearing comment explaining this is clear.
• SuspendCodeCoverage.Dispose correctly passes null (the captured previous value when the env var was absent) to SetEnvironmentVariable, which is the documented way to delete the variable — no resource-leak risk.
• The System.Configuration explicit reference is correctly scoped to $(NetFrameworkMinimum) only — confirmed that MSTestAdapter.PlatformServices ships exactly one .NET Framework TFM (net462), so no framework TFM is missed.
• ObjectModelDecouplingTests correctly uses AwesomeAssertions (required by this project's BannedSymbols.txt), uses IndexOf instead of string.Contains(string, StringComparison) for .NET Framework compat, and verifies the compile-time manifest references — the right API for the stated contract.

[Copilot]

Pull request overview

This PR is the final slice of a multi-PR initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. It removes the Microsoft.TestPlatform.ObjectModel package reference from the production assembly, moving all remaining VSTest coupling up into the MSTest.TestAdapter layer. The work is a faithful, no-behavior-change decoupling, locked in by a new guard test.

I verified the key correctness claims: the new local ApartmentStateSetting { MTA, STA } enum exactly mirrors VSTest's PlatformApartmentState (MTA=0, STA=1, confirmed from the vstest source), so Enum.TryParse numeric-string compatibility is preserved on both the runsettings-XML and config paths; System.Configuration usage is entirely #if NETFRAMEWORK-guarded with net462 being the only netfx TFM; the UWP ObjectModel reference is consistent with VSTestBridge being excluded for UwpMinimum; and the guard test's assumption holds (MSTest's framework assemblies are named MSTest.TestFramework*, which don't contain "TestPlatform").

Changes:

• Remove the Microsoft.TestPlatform.ObjectModel package reference from PlatformServices; replace the last VSTest enum consumer with a local neutral ApartmentStateSetting, and add a direct System.Configuration framework reference on .NET Framework.
• Add explicit Microsoft.TestPlatform.ObjectModel references where the transitive path is now gone (MSTest.TestAdapter for UWP; the Desktop integration test project which uses XmlRunSettingsUtilities directly).
• Add ObjectModelDecouplingTests guard asserting the compiled PlatformServices assembly references no *TestPlatform* assembly; harden TestSourceHandler public-key-token comparison against null/empty tokens.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/ApartmentStateSetting.cs	New neutral enum replacing VSTest PlatformApartmentState, with load-bearing member order documented
src/Adapter/MSTestAdapter.PlatformServices/RunConfigurationSettings.cs	Parse apartment state via local enum on both XML and config paths
src/Adapter/MSTestAdapter.PlatformServices/MSTestAdapter.PlatformServices.csproj	Remove ObjectModel package ref; add netfx-only System.Configuration reference
src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHandler.cs	Make public-key-token comparison null/empty-safe
src/Adapter/MSTest.TestAdapter/MSTest.TestAdapter.csproj	Add explicit ObjectModel reference for UWP (VSTestBridge excluded there)
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/ObjectModelDecouplingTests.cs	New guard test enforcing the platform-neutral contract
test/IntegrationTests/PlatformServices.Desktop.IntegrationTests/PlatformServices.Desktop.IntegrationTests.csproj	Add direct ObjectModel package ref for XmlRunSettingsUtilities

Review details

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

[github-actions]

🧪 Test quality grade — PR #9633

Grade	Test	Notes
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenDiffers	Boundary test: differing non-null token; byte-array magic explained by comment; clean AAA. No issues found.
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenIsMissing	Boundary test: null public-key-token case; .Should().BeFalse() is complete for a bool return. No issues found.
A (90–100)	new ObjectModelDecouplingTests. PlatformServicesAssemblyShouldNotReferenceAnyTestPlatformAssembly	Contract guard via reflection with a well-messaged .Should().BeEmpty(). 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. · 61.4 AIC · ⌖ 11.1 AIC · ⊞ 9.5K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

Phase 7 (capstone) — drop the ObjectModel package reference

The final slice of the initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. After this, PlatformServices no longer references the VSTest object model (Microsoft.TestPlatform.ObjectModel) at all — every VSTest dependency now lives in the MSTest.TestAdapter layer above it. Strict byte-for-byte, no behavior change.

Production changes

1. Remove the Microsoft.TestPlatform.ObjectModelPackageReference from MSTestAdapter.PlatformServices.csproj.
2. RunConfigurationSettings was the last consumer of a transitively-provided VSTest package: it used PlatformAbstractions' PlatformApartmentState enum {MTA, STA} to parse ExecutionThreadApartmentState. Replaced with a local internal enum ApartmentStateSetting { MTA, STA } of the same shape (same member names and order — MTA=0, STA=1), preserving the exact Enum.TryParse → STA/MTA → System.Threading.ApartmentState / else-throw behavior byte-for-byte on both the runsettings-XML and config paths. The member order is load-bearing (Enum.TryParse accepts numeric strings "0"/"1", so the numbering must match) and is commented as such. (Parsing directly to System.Threading.ApartmentState would change the handling of the Unknown value — it has that extra member — so a faithful 2-member local enum is required. The property type stays the BCL System.Threading.ApartmentState; only the parse-enum is neutralized.)
3. Add a direct System.Configurationframework reference on .NET Framework. ConfigurationManager/ConfigurationElementCollection (used by TestDataSource) were previously pulled in transitively via the object-model package; System.Configuration is a framework assembly, so it is now referenced directly.

Result: PlatformServices is fully platform-neutral

The compiled MSTestAdapter.PlatformServices assembly has zero references to any Microsoft.*.TestPlatform.* assembly on every real TFM (net462/net8.0/net9.0 + windows variants), verified via assembly metadata.

Guard test (the finish line)

ObjectModelDecouplingTests asserts the compiled PlatformServices assembly references no assembly whose name contains "TestPlatform" — catching ObjectModel, PlatformAbstractions, CoreUtilities, etc. (MSTest's own framework is MSTest.TestFramework, which doesn't match). This locks the platform-agnostic contract permanently.

Test-project fix

PlatformServices.Desktop.IntegrationTests uses ObjectModel's XmlRunSettingsUtilities directly (previously transitive through the PlatformServices project reference), so it gets its own direct Microsoft.TestPlatform.ObjectModelPackageReference. Test projects may reference the object model; only the production assembly must be neutral.

Verification

• All real TFMs build 0-warning (UWP builds via full msbuild in CI).
• Compiled dll: zeroTestPlatform references (net462 + net8.0, metadata-verified).
• MSTestAdapter.PlatformServices.UnitTests: 936/936 (net462), 898/898 (net8.0) — includes the new guard test and the STA/MTA parsing tests on both the runsettings-XML and config paths.
• PlatformServices.Desktop.IntegrationTests: 15/15.
• MSTestAdapter.UnitTests: 21/21. MSTest.TestAdapter and MSTest.IntegrationTests build clean.
• Expert-reviewer pass.

Stacking

Stacks on #9632 (Phase 6e-4c3); base branch dev/amauryleve/vstest-decoupling-suspendcoverage. Review/merge after the earlier PRs in the chain reach the base. Do not squash-rebase the base.

[github-actions]

Note

🤖 Automated review 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 Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

PR #9633 — VSTest ObjectModel decoupling from PlatformServices

#	Dimension	Verdict
1	Algorithmic Correctness	⚠️ 1 MODERATE
2	Threading & Concurrency	✅ LGTM
3	Security & IPC Contract Safety	✅ LGTM
4	Public API & Binary Compatibility	✅ LGTM
5	Performance & Allocations	✅ LGTM
6	Cross-TFM Compatibility	✅ LGTM
7	Resource & IDisposable Management	✅ LGTM
8	Defensive Coding at Boundaries	✅ LGTM (covered by #1)
9	Localization & Resources	✅ LGTM
10	Test Isolation	✅ LGTM
11	Assertion Quality	✅ LGTM
12	Flakiness Patterns	✅ LGTM
13	Test Completeness	⚠️ 1 MODERATE
14	Data-Driven Test Coverage	N/A
15	Code Structure & Simplification	✅ LGTM
16	Naming & Conventions	✅ LGTM
17	Documentation Accuracy	✅ LGTM
18	Analyzer & Code Fix Quality	N/A
19	IPC Wire Compatibility	N/A
20	Build Infrastructure & Dependencies	✅ LGTM
21	Scope & PR Discipline	✅ LGTM
22	PowerShell Scripting Hygiene	N/A

✅ 17/18 applicable dimensions clean.

---

Findings

• Algorithmic Correctness (MODERATE) — ArePublicKeyTokensEqual(byte[] left, byte[] right) in TestSourceHandler.cs line 142 dereferences both parameters unconditionally. AssemblyName.GetPublicKeyToken() returns null for unsigned assemblies, producing a NullReferenceException that is silently caught and converted to the conservative null → true path (false-positive discovery) rather than the correct false. See inline comment for the fix (annotate byte[]? and add a null guard at the top of the helper).

• Test Completeness (MODERATE) — The new SuspendCodeCoverage class (Utilities/SuspendCodeCoverage.cs) has no unit tests. Three behaviours are testable and could silently regress on .NET Framework TFMs without coverage: (1) constructor saves the previous env-var value and sets "TRUE", (2) Dispose() restores the previous value (null → delete), (3) the double-dispose guard prevents a second restoration. Suggested location: a new SuspendCodeCoverageTests.cs in MSTestAdapter.PlatformServices.UnitTests, guarded with #if NETFRAMEWORK.

---

Notable positives

• The ApartmentStateSetting enum ordering (MTA=0, STA=1) correctly matches the old PlatformApartmentState numeric values, preserving parse compatibility for numeric run-settings strings — and the load-bearing comment explaining this is clear.
• SuspendCodeCoverage.Dispose correctly passes null (the captured previous value when the env var was absent) to SetEnvironmentVariable, which is the documented way to delete the variable — no resource-leak risk.
• The System.Configuration explicit reference is correctly scoped to $(NetFrameworkMinimum) only — confirmed that MSTestAdapter.PlatformServices ships exactly one .NET Framework TFM (net462), so no framework TFM is missed.
• ObjectModelDecouplingTests correctly uses AwesomeAssertions (required by this project's BannedSymbols.txt), uses IndexOf instead of string.Contains(string, StringComparison) for .NET Framework compat, and verifies the compile-time manifest references — the right API for the stated contract.

[Copilot]

Pull request overview

This PR is the final slice of a multi-PR initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. It removes the Microsoft.TestPlatform.ObjectModel package reference from the production assembly, moving all remaining VSTest coupling up into the MSTest.TestAdapter layer. The work is a faithful, no-behavior-change decoupling, locked in by a new guard test.

I verified the key correctness claims: the new local ApartmentStateSetting { MTA, STA } enum exactly mirrors VSTest's PlatformApartmentState (MTA=0, STA=1, confirmed from the vstest source), so Enum.TryParse numeric-string compatibility is preserved on both the runsettings-XML and config paths; System.Configuration usage is entirely #if NETFRAMEWORK-guarded with net462 being the only netfx TFM; the UWP ObjectModel reference is consistent with VSTestBridge being excluded for UwpMinimum; and the guard test's assumption holds (MSTest's framework assemblies are named MSTest.TestFramework*, which don't contain "TestPlatform").

Changes:

• Remove the Microsoft.TestPlatform.ObjectModel package reference from PlatformServices; replace the last VSTest enum consumer with a local neutral ApartmentStateSetting, and add a direct System.Configuration framework reference on .NET Framework.
• Add explicit Microsoft.TestPlatform.ObjectModel references where the transitive path is now gone (MSTest.TestAdapter for UWP; the Desktop integration test project which uses XmlRunSettingsUtilities directly).
• Add ObjectModelDecouplingTests guard asserting the compiled PlatformServices assembly references no *TestPlatform* assembly; harden TestSourceHandler public-key-token comparison against null/empty tokens.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/ApartmentStateSetting.cs	New neutral enum replacing VSTest PlatformApartmentState, with load-bearing member order documented
src/Adapter/MSTestAdapter.PlatformServices/RunConfigurationSettings.cs	Parse apartment state via local enum on both XML and config paths
src/Adapter/MSTestAdapter.PlatformServices/MSTestAdapter.PlatformServices.csproj	Remove ObjectModel package ref; add netfx-only System.Configuration reference
src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHandler.cs	Make public-key-token comparison null/empty-safe
src/Adapter/MSTest.TestAdapter/MSTest.TestAdapter.csproj	Add explicit ObjectModel reference for UWP (VSTestBridge excluded there)
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/ObjectModelDecouplingTests.cs	New guard test enforcing the platform-neutral contract
test/IntegrationTests/PlatformServices.Desktop.IntegrationTests/PlatformServices.Desktop.IntegrationTests.csproj	Add direct ObjectModel package ref for XmlRunSettingsUtilities

Review details

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

[github-actions]

🧪 Test quality grade — PR #9633

Grade	Test	Notes
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenDiffers	Boundary test: differing non-null token; byte-array magic explained by comment; clean AAA. No issues found.
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenIsMissing	Boundary test: null public-key-token case; .Should().BeFalse() is complete for a bool return. No issues found.
A (90–100)	new ObjectModelDecouplingTests. PlatformServicesAssemblyShouldNotReferenceAnyTestPlatformAssembly	Contract guard via reflection with a well-messaged .Should().BeEmpty(). 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. · 61.4 AIC · ⌖ 11.1 AIC · ⊞ 9.5K · [◷]( · ◷)

[Amaury Levé (Evangelink)]

Phase 7 (capstone) — drop the ObjectModel package reference

The final slice of the initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. After this, PlatformServices no longer references the VSTest object model (Microsoft.TestPlatform.ObjectModel) at all — every VSTest dependency now lives in the MSTest.TestAdapter layer above it. Strict byte-for-byte, no behavior change.

Production changes

1. Remove the Microsoft.TestPlatform.ObjectModelPackageReference from MSTestAdapter.PlatformServices.csproj.
2. RunConfigurationSettings was the last consumer of a transitively-provided VSTest package: it used PlatformAbstractions' PlatformApartmentState enum {MTA, STA} to parse ExecutionThreadApartmentState. Replaced with a local internal enum ApartmentStateSetting { MTA, STA } of the same shape (same member names and order — MTA=0, STA=1), preserving the exact Enum.TryParse → STA/MTA → System.Threading.ApartmentState / else-throw behavior byte-for-byte on both the runsettings-XML and config paths. The member order is load-bearing (Enum.TryParse accepts numeric strings "0"/"1", so the numbering must match) and is commented as such. (Parsing directly to System.Threading.ApartmentState would change the handling of the Unknown value — it has that extra member — so a faithful 2-member local enum is required. The property type stays the BCL System.Threading.ApartmentState; only the parse-enum is neutralized.)
3. Add a direct System.Configurationframework reference on .NET Framework. ConfigurationManager/ConfigurationElementCollection (used by TestDataSource) were previously pulled in transitively via the object-model package; System.Configuration is a framework assembly, so it is now referenced directly.

Result: PlatformServices is fully platform-neutral

The compiled MSTestAdapter.PlatformServices assembly has zero references to any Microsoft.*.TestPlatform.* assembly on every real TFM (net462/net8.0/net9.0 + windows variants), verified via assembly metadata.

Guard test (the finish line)

ObjectModelDecouplingTests asserts the compiled PlatformServices assembly references no assembly whose name contains "TestPlatform" — catching ObjectModel, PlatformAbstractions, CoreUtilities, etc. (MSTest's own framework is MSTest.TestFramework, which doesn't match). This locks the platform-agnostic contract permanently.

Test-project fix

PlatformServices.Desktop.IntegrationTests uses ObjectModel's XmlRunSettingsUtilities directly (previously transitive through the PlatformServices project reference), so it gets its own direct Microsoft.TestPlatform.ObjectModelPackageReference. Test projects may reference the object model; only the production assembly must be neutral.

Verification

• All real TFMs build 0-warning (UWP builds via full msbuild in CI).
• Compiled dll: zeroTestPlatform references (net462 + net8.0, metadata-verified).
• MSTestAdapter.PlatformServices.UnitTests: 936/936 (net462), 898/898 (net8.0) — includes the new guard test and the STA/MTA parsing tests on both the runsettings-XML and config paths.
• PlatformServices.Desktop.IntegrationTests: 15/15.
• MSTestAdapter.UnitTests: 21/21. MSTest.TestAdapter and MSTest.IntegrationTests build clean.
• Expert-reviewer pass.

Stacking

Stacks on #9632 (Phase 6e-4c3); base branch dev/amauryleve/vstest-decoupling-suspendcoverage. Review/merge after the earlier PRs in the chain reach the base. Do not squash-rebase the base.

[github-actions]

Note

🤖 Automated review 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 Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

PR #9633 — VSTest ObjectModel decoupling from PlatformServices

#	Dimension	Verdict
1	Algorithmic Correctness	⚠️ 1 MODERATE
2	Threading & Concurrency	✅ LGTM
3	Security & IPC Contract Safety	✅ LGTM
4	Public API & Binary Compatibility	✅ LGTM
5	Performance & Allocations	✅ LGTM
6	Cross-TFM Compatibility	✅ LGTM
7	Resource & IDisposable Management	✅ LGTM
8	Defensive Coding at Boundaries	✅ LGTM (covered by #1)
9	Localization & Resources	✅ LGTM
10	Test Isolation	✅ LGTM
11	Assertion Quality	✅ LGTM
12	Flakiness Patterns	✅ LGTM
13	Test Completeness	⚠️ 1 MODERATE
14	Data-Driven Test Coverage	N/A
15	Code Structure & Simplification	✅ LGTM
16	Naming & Conventions	✅ LGTM
17	Documentation Accuracy	✅ LGTM
18	Analyzer & Code Fix Quality	N/A
19	IPC Wire Compatibility	N/A
20	Build Infrastructure & Dependencies	✅ LGTM
21	Scope & PR Discipline	✅ LGTM
22	PowerShell Scripting Hygiene	N/A

✅ 17/18 applicable dimensions clean.

---

Findings

• Algorithmic Correctness (MODERATE) — ArePublicKeyTokensEqual(byte[] left, byte[] right) in TestSourceHandler.cs line 142 dereferences both parameters unconditionally. AssemblyName.GetPublicKeyToken() returns null for unsigned assemblies, producing a NullReferenceException that is silently caught and converted to the conservative null → true path (false-positive discovery) rather than the correct false. See inline comment for the fix (annotate byte[]? and add a null guard at the top of the helper).

• Test Completeness (MODERATE) — The new SuspendCodeCoverage class (Utilities/SuspendCodeCoverage.cs) has no unit tests. Three behaviours are testable and could silently regress on .NET Framework TFMs without coverage: (1) constructor saves the previous env-var value and sets "TRUE", (2) Dispose() restores the previous value (null → delete), (3) the double-dispose guard prevents a second restoration. Suggested location: a new SuspendCodeCoverageTests.cs in MSTestAdapter.PlatformServices.UnitTests, guarded with #if NETFRAMEWORK.

---

Notable positives

• The ApartmentStateSetting enum ordering (MTA=0, STA=1) correctly matches the old PlatformApartmentState numeric values, preserving parse compatibility for numeric run-settings strings — and the load-bearing comment explaining this is clear.
• SuspendCodeCoverage.Dispose correctly passes null (the captured previous value when the env var was absent) to SetEnvironmentVariable, which is the documented way to delete the variable — no resource-leak risk.
• The System.Configuration explicit reference is correctly scoped to $(NetFrameworkMinimum) only — confirmed that MSTestAdapter.PlatformServices ships exactly one .NET Framework TFM (net462), so no framework TFM is missed.
• ObjectModelDecouplingTests correctly uses AwesomeAssertions (required by this project's BannedSymbols.txt), uses IndexOf instead of string.Contains(string, StringComparison) for .NET Framework compat, and verifies the compile-time manifest references — the right API for the stated contract.

[Copilot]

Pull request overview

This PR is the final slice of a multi-PR initiative to make Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices platform-agnostic. It removes the Microsoft.TestPlatform.ObjectModel package reference from the production assembly, moving all remaining VSTest coupling up into the MSTest.TestAdapter layer. The work is a faithful, no-behavior-change decoupling, locked in by a new guard test.

I verified the key correctness claims: the new local ApartmentStateSetting { MTA, STA } enum exactly mirrors VSTest's PlatformApartmentState (MTA=0, STA=1, confirmed from the vstest source), so Enum.TryParse numeric-string compatibility is preserved on both the runsettings-XML and config paths; System.Configuration usage is entirely #if NETFRAMEWORK-guarded with net462 being the only netfx TFM; the UWP ObjectModel reference is consistent with VSTestBridge being excluded for UwpMinimum; and the guard test's assumption holds (MSTest's framework assemblies are named MSTest.TestFramework*, which don't contain "TestPlatform").

Changes:

• Remove the Microsoft.TestPlatform.ObjectModel package reference from PlatformServices; replace the last VSTest enum consumer with a local neutral ApartmentStateSetting, and add a direct System.Configuration framework reference on .NET Framework.
• Add explicit Microsoft.TestPlatform.ObjectModel references where the transitive path is now gone (MSTest.TestAdapter for UWP; the Desktop integration test project which uses XmlRunSettingsUtilities directly).
• Add ObjectModelDecouplingTests guard asserting the compiled PlatformServices assembly references no *TestPlatform* assembly; harden TestSourceHandler public-key-token comparison against null/empty tokens.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/ApartmentStateSetting.cs	New neutral enum replacing VSTest PlatformApartmentState, with load-bearing member order documented
src/Adapter/MSTestAdapter.PlatformServices/RunConfigurationSettings.cs	Parse apartment state via local enum on both XML and config paths
src/Adapter/MSTestAdapter.PlatformServices/MSTestAdapter.PlatformServices.csproj	Remove ObjectModel package ref; add netfx-only System.Configuration reference
src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHandler.cs	Make public-key-token comparison null/empty-safe
src/Adapter/MSTest.TestAdapter/MSTest.TestAdapter.csproj	Add explicit ObjectModel reference for UWP (VSTestBridge excluded there)
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/ObjectModelDecouplingTests.cs	New guard test enforcing the platform-neutral contract
test/IntegrationTests/PlatformServices.Desktop.IntegrationTests/PlatformServices.Desktop.IntegrationTests.csproj	Add direct ObjectModel package ref for XmlRunSettingsUtilities

Review details

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

[github-actions]

🧪 Test quality grade — PR #9633

Grade	Test	Notes
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenDiffers	Boundary test: differing non-null token; byte-array magic explained by comment; clean AAA. No issues found.
A (90–100)	mod DesktopTestSourceTests. IsAssemblyReferencedShouldReturnFalseIfPublicKeyTokenIsMissing	Boundary test: null public-key-token case; .Should().BeFalse() is complete for a bool return. No issues found.
A (90–100)	new ObjectModelDecouplingTests. PlatformServicesAssemblyShouldNotReferenceAnyTestPlatformAssembly	Contract guard via reflection with a well-messaged .Should().BeEmpty(). 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. · 61.4 AIC · ⌖ 11.1 AIC · ⊞ 9.5K · [◷]( · ◷)