Skip to content

Update Microsoft.NETTestSdkVersion and related packages - #11178

Closed
Amaury Levé (Evangelink) wants to merge 4 commits into
mainfrom
Evangelink-patch-2
Closed

Amaury Levé (Evangelink) wants to merge 4 commits into
mainfrom
Evangelink-patch-2

Conversation

@Evangelink

Copy link
Copy Markdown
Member

No description provided.

Copilot AI balanced review requested due to automatic review settings September 10, 2026 13:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The version alignment is complete, valid, and leaves no stale property references.

Review tier: Balanced
Findings: None

What changed in this PR

Aligns the .NET test SDK and related Test Platform packages on version 18.10.0.

Changes:

  • Updates MicrosoftNETTestSdkVersion to 18.10.0.
  • Removes the redundant Filter.Source version property.
File Description
Directory.Packages.props Centralizes related Test Platform packages on version 18.10.0.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 10, 2026 18:09
@github-actions

This comment has been minimized.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Direct TestPlatform references in acceptance tests still resolve version 18.9.0.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 Medium severity

New issues introduced by this change (1)
Severity Finding
Medium severity Directory.Packages.props — The version split is not propagated to acceptance-test consumers that reference TestPlatform…

Comment thread Directory.Packages.props Outdated
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Final test-triage resolution — Azure Pipelines build 1591340

Status: final (completed aggregate build, evidenceIncomplete: false, analysisMode: full).

This confirms and extends the preliminary finding for this build. All three legs (Linux Release, MacOS Release, Windows Release) fail deterministically with the same signature: a VSTest-driven dotnet test invocation against a net10.0 (and, on Windows, additionally net462) target whose test host process crashes immediately at startup, frequently surfaced as MSB4181: The "VSTestTask" task returned false but did not log an error followed by Test host process crashed. Affected acceptance tests, identical across all three OS legs:

  • AspireSdkTests.EnableAspireProperty_WhenUsingVSTest_AllowsToRunAspireTests
  • DotnetTestCliTests.DotnetTest_Should_Execute_Tests (net10.0, Debug and Release)
  • PlaywrightSdkTests.EnablePlaywrightProperty_WhenUsingVSTest_AllowsToRunPlaywrightTests (net10.0)
  • SdkTests.RunTests_With_VSTest (net10.0, Debug and Release; Windows additionally net10.0;net462)
  • TelemetryTests.VSTest_RunTests_Succeeds (net10.0)
  • TelemetryTests.VSTest_DiscoverTests_Succeeds (net10.0)

No retries recovered any of these; each fails once and stays failed on every leg. The identical signature also recurred on this PR's prior build (1590874's preliminary comment flagged it on the Linux leg alone; this build confirms it now reproduces on macOS and Windows too), so this is a stable regression tied to the current revision rather than a one-off environmental flake.

Root cause: this PR only changes Directory.Packages.props, splitting MicrosoftTestPlatformVersion (bumped to 18.10.0) from MicrosoftNETTestSdkVersion (kept at 18.9.0). An existing Copilot review comment on this PR (on Directory.Packages.props:45) already identifies that this split is incomplete: AcceptanceTestBase, ModernUwpTests, VSTestConsoleLocator, and AppDomainTests still resolve Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform/Microsoft.TestPlatform.TestHost via the unchanged MicrosoftNETTestSdkVersion (18.9.0) instead of the new MicrosoftTestPlatformVersion (18.10.0). This version skew between the VSTest console/runner packages and the test-host binaries actually resolved for dotnet test is consistent with the observed test-host crash-at-startup signature.

Because this failure is caused by and confined to this PR's own incomplete package-version split — not a pre-existing or general codebase defect — no new issue is warranted; the fix is to complete the MicrosoftTestPlatformVersion propagation as already requested in the pending review comment, rather than track it separately.

Next step: apply the outstanding review-comment fix (route AcceptanceTestBase/ModernUwpTests/VSTestConsoleLocator/AppDomainTests to MicrosoftTestPlatformVersion for direct Microsoft.TestPlatform* references, keeping MicrosoftNETTestSdkVersion only for Microsoft.NET.Test.Sdk), then re-run the pipeline to confirm the VSTestTask/test-host crashes clear on all three OS legs.

🤖 Automated content by GitHub Copilot. Generated by the Pipeline Test Triage workflow. · auto · 140.9 AIC · ⌖ 2.09 AIC · ⊞ 8.9K

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 11, 2026 10:46
@github-actions

This comment has been minimized.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The publish cleanup risks deleting required assemblies, and Test Platform asset resolution still uses the outdated version source.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 1 High severity · 1 Medium severity

New issues introduced by this change (1)
Severity Finding
High severity src/​Adapter/​MSTest.TestAdapter/​buildTransitive/​common/​MSTest.TestAdapter.targets — Wildcard cleanup deletes legitimate self-contained runtime files
Pre-existing issues (1)
Severity Finding
Medium severity Directory.Packages.props — The version split is not propagated to acceptance-test consumers that reference TestPlatform… View comment

Comment thread src/Adapter/MSTest.TestAdapter/buildTransitive/common/MSTest.TestAdapter.targets Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 11, 2026 11:01
@github-actions

Copy link
Copy Markdown
Contributor

🧵 Parallel-safety audit — PR #11178

Nothing audited here touches process-global state, shared filesystem paths, or
[ResourceLock] / [DoNotParallelize] declarations. Nothing to flag for
parallel-safety.

The two changed test files (RunnerTests.cs, BrowserWasmExecutionTests.cs) each only add a
private, pure string-template helper (ExcludeMicrosoftCodeCoverageAssets, built on
string.Replace via PatchCodeWithReplace) used to patch in-memory source code before it is
handed to TestAsset.GenerateAssetAsync, which writes it into a fresh, GUID-unique
TempDirectory per call — so there is no shared-path collision even though the same asset-name
constants are reused across methods. No environment variable, current-directory, console, or
static-field mutation was introduced, and no [ResourceLock] / [Parallelize] /
[DoNotParallelize] declaration changed. The third changed file, Directory.Packages.props, is
only a NuGet package-version bump and does not affect any assembly's parallelization state.

Audited MSTest.Acceptance.IntegrationTests at scope MethodLevel (assembly attribute in
Program.cs), workers CPU count (Workers = 0).
Audited Microsoft.Testing.Platform.Acceptance.IntegrationTests at scope MethodLevel (assembly
attribute in Program.cs), workers CPU count (Workers = 0).

Re-run with /parallel-audit.

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 78.5 AIC · ⌖ 3.7 AIC · ⊞ 23.9K · [◷]( · )

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified.

Review tier: Lite
Findings: None

Issues resolved since last review (2)
Severity Finding
High severity src/​Adapter/​MSTest.TestAdapter/​buildTransitive/​common/​MSTest.TestAdapter.targets — Wildcard cleanup deletes legitimate self-contained runtime files View resolved comment
Medium severity Directory.Packages.props — The version split is not propagated to acceptance-test consumers that reference TestPlatform… View resolved comment

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Expert test review — PR #11178

Both changed test files land their edits entirely inside asset-generator helper methods or in a single one-line swap of the injected MSBuild source string — no assertion logic, control flow, or test intent changed.

GradeTestMutationNotesHow to improve
A (90–100) mod RunnerTests.
EnableMSTestRunner_True_
Will_Run_Standalone
N/A Only change is wrapping the injected source with ExcludeMicrosoftCodeCoverageAssets; binlog capability check plus AssertOutputContainsSummary remain intact and unaffected.
A (90–100) mod RunnerTests.
EnableMSTestRunner_True_
WithCustomEntryPoint_
Will_Run_Standalone
N/A Same one-line source-wrapping change; AssertOutputContainsSummary assertion on pass/fail/skip counts is unchanged.

No [TestMethod]-decorated methods in BrowserWasmExecutionTests.cs fall inside this PR's changed line ranges — every touched line there is inside private, non-test asset-generator helpers (GenerateBrowserWasm*AssetAsync, ExcludeMicrosoftCodeCoverageAssets) that only wire the new Microsoft.CodeCoverage ExcludeAssets="all" package reference into generated test-asset projects. That plumbing is out of scope for per-test grading.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 132.9 AIC · ⌖ 2.44 AIC · ⊞ 15.9K · [◷]( · )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants