Fix forward-compat crash loading old 2.x extensions on .NET Framework (#9710)#9739
Conversation
…#9710) A newer Microsoft.Testing.Platform combined with an older (2.x) extension whose netstandard2.0 asset was compiled against the platform's internal Polyfills.Polyfill helper (shared via InternalsVisibleTo) threw MissingMethodException in AppInsightsProvider.IngestLoopAsync on .NET Framework, crashing the process. - Restore the exact Polyfills.Polyfill members old 2.x extensions reference (Deconstruct, Contains x2, AppendJoin x2, CopyToAsync, SaveAsync), guarded to the non-.NET-Core TFMs so they land only in the netstandard2.0 platform asset that is loaded on .NET Framework. The type is already internal with IVT to the extensions; [Embedded] only hides it from new compile-time references, so the CLR still resolves old baked MemberRefs at runtime. - Add TelemetryForwardCompatibilityTests reproducing the crash: current platform + oldest supported 2.x telemetry extension (2.0.0) on net462 with telemetry enabled. Verified red without the shim (exit code -532462766) and green with it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
🧪 Test quality grade — PR #9739
This advisory comment was generated automatically. Grades are heuristic
|
There was a problem hiding this comment.
Pull request overview
Fixes #9710, where a newer Microsoft.Testing.Platform crashes with MissingMethodException on .NET Framework when loading an older (2.x) extension. Old 2.x extension assemblies (Telemetry, TrxReport, VSTestBridge) were compiled against the platform's internal Polyfills.Polyfill helpers and resolve those MemberRefs from the platform assembly at runtime via [InternalsVisibleTo]. Newer platform code stopped using those helpers, so they were dropped — breaking the netstandard2.0 load path. The fix restores the exact member set, guarded to the non-netcore asset, and adds a regression test.
Changes:
- Restore the
Polyfills.Polyfillmembers old 2.x extensions reference (Deconstruct,Contains×2,AppendJoin×2,CopyToAsync,SaveAsync) in a new#if !NETCOREAPPfile, with a prominent comment warning against re-deleting them as "unused." - Add
TelemetryForwardCompatibilityTestsreproducing the crash with the oldest supported 2.x telemetry extension (2.0.0) onnet462with telemetry enabled.
Show a summary per file
| File | Description |
|---|---|
src/Polyfills/PolyfillBackwardCompatibility.cs |
New netstandard2.0/.NET Framework-only file restoring the internal Polyfill members old 2.x extensions bind to at runtime, preventing MissingMethodException. |
test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/TelemetryForwardCompatibilityTests.cs |
New acceptance test reproducing #9710 (newest platform + oldest 2.x telemetry extension on net462, telemetry enabled) and guarding against regression. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Medium
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
✅ 15/15 dimensions clean — no findings.
| Dimension | Severity | Verdict |
|---|---|---|
| Algorithmic Correctness | MAJOR | LGTM |
| Threading & Concurrency | BLOCKING | LGTM |
| Public API & Binary Compatibility | BLOCKING | LGTM |
| Cross-TFM Compatibility | MAJOR | LGTM |
| Defensive Coding | MAJOR | LGTM |
| Flakiness Patterns | BLOCKING | LGTM |
| Test Isolation | MAJOR | LGTM |
| Assertion Quality | MAJOR | LGTM |
| Test Completeness & Coverage | MAJOR | LGTM |
| Code Structure & Simplification | MODERATE | LGTM |
| Naming & Conventions | NIT | LGTM |
| Documentation Accuracy | NIT | LGTM |
| Performance & Allocations | MAJOR | LGTM |
| Resource & IDisposable Management | MAJOR | LGTM |
| Scope & PR Discipline | MODERATE | LGTM |
Summary: Clean, well-scoped fix. The polyfill implementations are correct and binary-compatible with old 2.x extension MemberRef tokens. The #if !NETCOREAPP guard correctly targets the netstandard2.0 build (which is what .NET Framework loads). The acceptance test faithfully reproduces the crash conditions (net462 + telemetry enabled + oldest 2.x extension) and the IMPORTANT comment provides strong protection against future accidental removal.
|
/backport to rel/4.3 |
|
Started backporting to rel/4.3: https://github.com/microsoft/testfx/actions/runs/28952108089 |
…#9710) by @Evangelink in #9739 (backport to rel/4.3) (#9740) Co-authored-by: Amaury Levé <amauryleve@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Summary
Fixes #9710. A newer
Microsoft.Testing.Platformcombined with an older (2.x) extension crashes withMissingMethodExceptionon .NET Framework:Root cause
Old 2.x extension assemblies (Telemetry, TrxReport, TrxReport.Abstractions, VSTestBridge) were compiled against the platform's internal
Polyfills.Polyfillhelper, which they access from the platform assembly via[InternalsVisibleTo](they don't embed their own copy). Their baked-in member references are resolved by the runtime against the platform assembly. Newer platform code stopped using those helpers, so they were dropped — and loading an old extension'snetstandard2.0/ .NET Framework asset now throws. The.NETassets bind these to the BCL and are unaffected, which is why the original report crashed onnet472whilenet8.0passed.Fix
src/Polyfills/PolyfillBackwardCompatibility.cs— restore the exactPolyfills.Polyfillmembers old 2.x extensions reference (Deconstruct,Contains×2,AppendJoin×2,CopyToAsync,SaveAsync), guarded#if !NETCOREAPPso they land only in thenetstandard2.0platform asset (the one loaded on .NET Framework). The type is alreadyinternalwith IVT to these extensions;[Embedded]only hides it from new compile-time references — the CLR still resolves old bakedMemberRefs at runtime. A prominent comment warns against re-deleting them as "unused polyfills."Test
TelemetryForwardCompatibilityTests— reproduces the crash: current platform + oldest supported 2.x telemetry extension (2.0.0) on net462 with telemetry enabled (which is what forcesAppInsightsProvider.IngestLoopAsync— the crashing method — to actually run). The existingForwardCompatibilityTestsmissed this because it pinned2.2.1(no longer references the platformPolyfill) and ran telemetry opted out.Verification (
build.cmd -pack+ acceptance run)-532462766(0xE0434352, CLR unhandled exception) crashing inAppInsightsProvider. Faithfully reproduces System.MissingMethodException after upgrade to 2.3.0 #9710.Notes for reviewers
Polyfillsurface was confirmed by decompiling the shipped2.0.0netstandard2.0nupkgs, so the shim covers every extension that referenced it (not just Telemetry). Happy to trim it to telemetry-only (Deconstruct) if preferred, but restoring the full set makes any old 2.x extension load correctly on .NET Framework.