Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR removes the dependency on the external Polyfill NuGet package and instead inlines polyfill source code via src/Polyfills/**/*.cs, updating call sites to be compatible across TFMs.
Changes:
- Replaced
PackageReference Include="Polyfill"withCompile Include="$(RepoRoot)src/Polyfills/**/*.cs"across many projects. - Added a local
src/Polyfillssource set (attributes, helpers, and compatibility shims) plus some per-TFM#iffallbacks. - Updated various code paths to avoid APIs unavailable on non-
NETCOREAPPTFMs (e.g.,CancelAsync,FlushAsync(CancellationToken),SaveAsync,Enum.Parse<T>,String.Contains(string, comparison)).
Reviewed changes
Copilot reviewed 170 out of 170 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Utilities/TestFramework.ForTestingMSTest/TestFramework.ForTestingMSTest.csproj | Replace Polyfill package with compiled local polyfills. |
| test/Utilities/Microsoft.Testing.TestInfrastructure/ProjectSystem.cs | Adjust TFM argument validation (remove Polyfill Ensure). |
| test/Utilities/Microsoft.Testing.TestInfrastructure/Microsoft.Testing.TestInfrastructure.csproj | Replace Polyfill package with compiled local polyfills. |
| test/Utilities/Automation.CLI/Automation.CLI.csproj | Replace Polyfill package with compiled local polyfills. |
| test/UnitTests/TestFramework.UnitTests/TestFramework.UnitTests.csproj | Replace Polyfill package with compiled local polyfills. |
| test/UnitTests/Microsoft.Testing.Platform.UnitTests/Microsoft.Testing.Platform.UnitTests.csproj | Replace Polyfill package with compiled local polyfills. |
| test/UnitTests/Microsoft.Testing.Platform.UnitTests/Helpers/CountDownEventTests.cs | Add non-NETCOREAPP cancellation fallback. |
| test/UnitTests/Microsoft.Testing.Extensions.VSTestBridge.UnitTests/Microsoft.Testing.Extensions.VSTestBridge.UnitTests.csproj | Replace Polyfill package with compiled local polyfills. |
| test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxTests.cs | Make TrxReportEngine ctor usage conditional on TFM. |
| test/UnitTests/Microsoft.Testing.Extensions.UnitTests/Microsoft.Testing.Extensions.UnitTests.csproj | Replace Polyfill package with compiled local polyfills. |
| test/UnitTests/MSTestAdapter.UnitTests/MSTestAdapter.UnitTests.csproj | Replace Polyfill package with compiled local polyfills. |
| test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/MSTestAdapter.PlatformServices.UnitTests.csproj | Replace Polyfill package with compiled local polyfills. |
| test/UnitTests/MSTest.Analyzers.UnitTests/MSTest.Analyzers.UnitTests.csproj | Replace Polyfill package with compiled local polyfills. |
| test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests.csproj | Replace Polyfill package with compiled local polyfills. |
| test/IntegrationTests/MSTest.IntegrationTests/MSTest.IntegrationTests.csproj | Replace Polyfill package with compiled local polyfills. |
| test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MSTest.Acceptance.IntegrationTests.csproj | Replace Polyfill package with compiled local polyfills. |
| src/TestFramework/TestFramework/TestFramework.csproj | Inline polyfills and exclude specific polyfill subsets via constants. |
| src/TestFramework/TestFramework/Logger.cs | Replace Polyfill Ensure checks with explicit ArgumentNullException. |
| src/TestFramework/TestFramework/Internal/TestDataSourceUtilities.cs | Use string overload of string.Join for compatibility. |
| src/TestFramework/TestFramework/Attributes/TestMethod/SingleThreadedSTASynchronizationContext.cs | Replace OperatingSystem.IsWindows() with RuntimeInformation check. |
| src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs | Guard ParamCollectionAttribute usage for pre-NET9 TFMs. |
| src/TestFramework/TestFramework/Assertions/CollectionAssert.cs | Use HashSet for uniqueness check. |
| src/TestFramework/TestFramework/Assertions/Assert.ThrowsException.cs | Replace Polyfill Ensure checks with explicit ArgumentNullException. |
| src/TestFramework/TestFramework/Assertions/Assert.That.cs | Remove tuple deconstruction; make string checks explicit. |
| src/TestFramework/TestFramework/Assertions/Assert.Contains.cs | Add non-NETCOREAPP fallback for string contains by comparison. |
| src/TestFramework/TestFramework/Assertions/Assert.AreEqual.cs | Replace Polyfill Ensure checks with explicit ArgumentNullException. |
| src/TestFramework/TestFramework.Extensions/TestFramework.Extensions.csproj | Replace Polyfill package with compiled local polyfills. |
| src/TestFramework/TestFramework.Extensions/RuntimeTypeHelper.cs | Remove Polyfill Ensure guard. |
| src/TestFramework/TestFramework.Extensions/PrivateType.cs | Replace Polyfill Ensure checks with explicit ArgumentNullException. |
| src/TestFramework/TestFramework.Extensions/PrivateObject.cs | Replace Polyfill Ensure checks with explicit ArgumentNullException. |
| src/TestFramework/TestFramework.Extensions/Attributes/WinUITestTargetAttribute.cs | Replace Polyfill Ensure checks with explicit ArgumentNullException. |
| src/Polyfills/UnreachableException.cs | Add local polyfill for UnreachableException (or type forward). |
| src/Polyfills/UnconditionalSuppressMessageAttribute.cs | Add local polyfill attribute (or type forward). |
| src/Polyfills/StackTraceHiddenAttribute.cs | Add local polyfill attribute (or type forward). |
| src/Polyfills/RequiredMemberAttribute.cs | Add local polyfill attribute (or type forward). |
| src/Polyfills/Range.cs | Add local polyfill for System.Range (or type forward). |
| src/Polyfills/ProcessExtensions.cs | Add local polyfill for Process.WaitForExitAsync. |
| src/Polyfills/PlatformAttributes.cs | Add local polyfills for platform attributes (or type forwards). |
| src/Polyfills/OperatingSystem.cs | Attempt to polyfill OperatingSystem.Is* checks. |
| src/Polyfills/NullableAttribtues.cs | Add local nullable attribute polyfills (or type forwards). |
| src/Polyfills/ModuleInitializerAttribute.cs | Add local polyfill attribute (or type forward). |
| src/Polyfills/IsExternalInit.cs | Add local polyfill for IsExternalInit (or type forward). |
| src/Polyfills/InterpolatedStringHandlerAttribute.cs | Add local polyfill attribute (or type forward). |
| src/Polyfills/InterpolatedStringHandlerArgumentAttribute.cs | Add local polyfill attribute (or type forward). |
| src/Polyfills/Index.cs | Add local polyfill for System.Index (or type forward). |
| src/Polyfills/HashHelpers.cs | Add local HashHelpers.Combine used by polyfilled Range/Index. |
| src/Polyfills/ExperimentalAttribute.cs | Add local polyfill attribute (or type forward). |
| src/Polyfills/Ensure.cs | Add local Ensure helpers used by call sites. |
| src/Polyfills/EmbeddedAttribute.cs | Add local EmbeddedAttribute used by embedded polyfills. |
| src/Polyfills/DynamicallyAccessedMembersAttribute.cs | Add local trimming annotation polyfill (or type forward). |
| src/Polyfills/DynamicallyAccessedMemberTypes.cs | Add local enum polyfill (or type forward). |
| src/Polyfills/CompilerLoweringPreserveAttribute.cs | Add local polyfill attribute (or type forward). |
| src/Polyfills/CompilerFeatureRequiredAttribute.cs | Add local polyfill attribute (or type forward). |
| src/Polyfills/CallerArgumentExpressionAttribute.cs | Add local polyfill attribute (or type forward). |
| src/Platform/Microsoft.Testing.Platform/Tools/ToolsManager.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/TestHostOrchestrator/TestHostOrchestratorManager.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/TestHostControllers/TestHostControllersManager.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/TestHost/TestHostManager.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/Telemetry/TelemetryManager.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/Services/ServiceProviderExtensions.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/Services/ExecutableInfo.cs | Use string overload of string.Join for compatibility. |
| src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/TcpMessageHandler.cs | Add non-NETCOREAPP flush fallback. |
| src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/ServerModePerCallOutputDevice.cs | Drain buffered messages via TryTake instead of Clear. |
| src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs | Remove newer APIs (TryAdd/deconstruction) for compatibility. |
| src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/PerRequestServerDataConsumerService.cs | Remove Ensure call. |
| src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Serializers/HandshakeMessageSerializer.cs | Remove tuple deconstruction for compatibility. |
| src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Serializers/DiscoveredTestMessagesSerializer.cs | Replace Ensure with ApplicationStateGuard.Unreachable(). |
| src/Platform/Microsoft.Testing.Platform/Requests/TreeNodeFilter/TreeNodeFilter.cs | Replace Ensure with explicit null-guard + length checks. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs | Avoid Split allocation in architecture parsing. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TestProgressStateAwareTerminal.cs | Use Lock on NET9+ otherwise object. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.cs | Use Lock on NET9+ otherwise object. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/OutputDeviceManager.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/Microsoft.Testing.Platform.csproj | Replace Polyfill package with compiled local polyfills and add Using Include="Polyfills". |
| src/Platform/Microsoft.Testing.Platform/Messages/TestNodeProperties.cs | Replace AppendJoin usage with manual loop. |
| src/Platform/Microsoft.Testing.Platform/Messages/PropertyBag.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/Messages/PropertyBag.Property.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/Messages/MessageBusProxy.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/Messages/AsynchronousMessageBus.cs | Avoid deconstruction; add NETCOREAPP/non-NETCOREAPP dictionary add patterns. |
| src/Platform/Microsoft.Testing.Platform/Logging/LoggingManager.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/Logging/LoggerFactoryProxy.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/Logging/LoggerFactoryExtensions.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeClient.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform/Hosts/ServerTestHost.cs | Replace CancelAsync usage with sync cancel + suppression. |
| src/Platform/Microsoft.Testing.Platform/Helpers/TimeSpanParser.cs | Use string overload for StartsWith. |
| src/Platform/Microsoft.Testing.Platform/Helpers/TaskExtensions.cs | Add local non-NETCOREAPP Task.WaitAsync polyfill. |
| src/Platform/Microsoft.Testing.Platform/Helpers/System/SystemProcess.cs | Add non-NETCOREAPP Kill fallback. |
| src/Platform/Microsoft.Testing.Platform/Helpers/System/SystemFileSystem.cs | Add non-NETCOREAPP file move/read async fallbacks. |
| src/Platform/Microsoft.Testing.Platform/Helpers/System/SystemEnvironment.cs | Add non-NETCOREAPP ProcessId fallback. |
| src/Platform/Microsoft.Testing.Platform/Helpers/Sha256Hasher.cs | Add non-NETCOREAPP hashing + hex lower fallbacks. |
| src/Platform/Microsoft.Testing.Platform/Helpers/ExtensionValidationHelper.cs | Replace Ensure with explicit null-guards. |
| src/Platform/Microsoft.Testing.Platform/Extensions/CompositeExtensionsFactory.cs | Replace Ensure with explicit null-guards; use Lock on NET9+. |
| src/Platform/Microsoft.Testing.Platform/Configurations/EnvironmentVariablesConfigurationProvider.cs | Replace NotNullOrEmpty with null + empty split checks. |
| src/Platform/Microsoft.Testing.Platform/Configurations/ConfigurationExtensions.cs | Replace Ensure with ApplicationStateGuard.Unreachable(). |
| src/Platform/Microsoft.Testing.Platform/CommandLine/Parser.cs | Use string overloads for StartsWith/EndsWith. |
| src/Platform/Microsoft.Testing.Platform/Builder/TestApplication.cs | Simplify debugger attach branch + add non-NETCOREAPP enum parsing. |
| src/Platform/Microsoft.Testing.Platform/BannedSymbols.txt | Remove bans related to ArgumentNullException usage. |
| src/Platform/Microsoft.Testing.Platform.MSBuild/Tasks/InvokeTestingPlatformTask.cs | Add non-NETCOREAPP enum parsing + use Lock on NET9+. |
| src/Platform/Microsoft.Testing.Platform.MSBuild/Tasks/ConfigurationFileTask.cs | Make injected dependency non-nullable with explicit null-guard. |
| src/Platform/Microsoft.Testing.Platform.MSBuild/Microsoft.Testing.Platform.MSBuild.csproj | Replace Polyfill package with compiled local polyfills. |
| src/Platform/Microsoft.Testing.Platform.AI/Microsoft.Testing.Platform.AI.csproj | Replace Polyfill package with compiled local polyfills. |
| src/Platform/Microsoft.Testing.Extensions.VSTestBridge/VSTestBridgedTestFrameworkBase.cs | Replace Ensure with explicit null-guard. |
| src/Platform/Microsoft.Testing.Extensions.VSTestBridge/Microsoft.Testing.Extensions.VSTestBridge.csproj | Inline polyfills; remove global Polyfills using item group. |
| src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportExtensions.cs | Add conditional using Polyfills for non-NETCOREAPP. |
| src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportEngine.cs | Make ctor + async save conditional on TFM. |
| src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxProcessLifetimeHandler.cs | Update TrxReportEngine usage for conditional ctor. |
| src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxModeHelpers.cs | Add conditional using Polyfills; adjust OperatingSystem browser check. |
| src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxDataConsumer.cs | Update TrxReportEngine usage for conditional ctor. |
| src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxCompareTool.cs | Add non-NETCOREAPP XML load fallback. |
| src/Platform/Microsoft.Testing.Extensions.TrxReport/Microsoft.Testing.Extensions.TrxReport.csproj | Replace Polyfill package with compiled local polyfills; remove global using. |
| src/Platform/Microsoft.Testing.Extensions.TrxReport/BannedSymbols.txt | Remove bans related to ArgumentNullException usage. |
| src/Platform/Microsoft.Testing.Extensions.TrxReport.Abstractions/TrxReportProperties.cs | Replace AppendJoin usage with manual loop. |
| src/Platform/Microsoft.Testing.Extensions.TrxReport.Abstractions/Microsoft.Testing.Extensions.TrxReport.Abstractions.csproj | Replace Polyfill package with compiled local polyfills; remove global using. |
| src/Platform/Microsoft.Testing.Extensions.TrxReport.Abstractions/BannedSymbols.txt | Remove bans related to ArgumentNullException usage. |
| src/Platform/Microsoft.Testing.Extensions.Telemetry/Microsoft.Testing.Extensions.Telemetry.csproj | Inline polyfills via compile include. |
| src/Platform/Microsoft.Testing.Extensions.Telemetry/AppInsightsProvider.cs | Remove tuple deconstruction for compatibility. |
| src/Platform/Microsoft.Testing.Extensions.Retry/RetryExtensions.cs | Add conditional using Polyfills for non-NETCOREAPP. |
| src/Platform/Microsoft.Testing.Extensions.Retry/Microsoft.Testing.Extensions.Retry.csproj | Replace Polyfill package with compiled local polyfills. |
| src/Platform/Microsoft.Testing.Extensions.Retry/BannedSymbols.txt | Remove bans related to ArgumentNullException usage. |
| src/Platform/Microsoft.Testing.Extensions.OpenTelemetry/Microsoft.Testing.Extensions.OpenTelemetry.csproj | Inline polyfills via compile include. |
| src/Platform/Microsoft.Testing.Extensions.MSBuild/Microsoft.Testing.Extensions.MSBuild.csproj | Replace Polyfill package with compiled local polyfills. |
| src/Platform/Microsoft.Testing.Extensions.MSBuild/MSBuildExtensions.cs | Add conditional using Polyfills for non-NETCOREAPP. |
| src/Platform/Microsoft.Testing.Extensions.HotReload/Microsoft.Testing.Extensions.HotReload.csproj | Replace Polyfill package with compiled local polyfills. |
| src/Platform/Microsoft.Testing.Extensions.HotReload/HotReloadHandler.cs | Add conditional using Polyfills for non-NETCOREAPP. |
| src/Platform/Microsoft.Testing.Extensions.HotReload/BannedSymbols.txt | Remove bans related to ArgumentNullException usage. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/WindowsMiniDumpWriteDump.cs | Exclude Windows-only code for non-NETCOREAPP builds. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Microsoft.Testing.Extensions.HangDump.csproj | Replace Polyfill package with compiled local polyfills. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Helpers/IProcessExtensions.cs | Add conditional using Polyfills for non-NETCOREAPP. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/HangDumpExtensions.cs | Add conditional using Polyfills for non-NETCOREAPP. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/BannedSymbols.txt | Remove bans related to ArgumentNullException usage. |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Microsoft.Testing.Extensions.CrashDump.csproj | Replace Polyfill package with compiled local polyfills. |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/BannedSymbols.txt | Remove bans related to ArgumentNullException usage. |
| src/Platform/Microsoft.Testing.Extensions.AzureFoundry/Microsoft.Testing.Extensions.AzureFoundry.csproj | Replace Polyfill package with compiled local polyfills. |
| src/Platform/Microsoft.Testing.Extensions.AzureDevOpsReport/Microsoft.Testing.Extensions.AzureDevOpsReport.csproj | Inline polyfills alongside RootFinder source include. |
| src/Platform/Microsoft.Testing.Extensions.AzureDevOpsReport/BannedSymbols.txt | Remove bans related to ArgumentNullException usage. |
| src/Analyzers/MSTest.Analyzers/TestContextShouldBeValidAnalyzer.cs | Make StartsWith usage explicit and ordinal. |
| src/Analyzers/MSTest.Analyzers/RoslynAnalyzerHelpers/ArrayBuilder.cs | Remove tuple deconstruction for compatibility. |
| src/Analyzers/MSTest.Analyzers/MSTest.Analyzers.csproj | Replace Polyfill package with compiled local polyfills. |
| src/Analyzers/MSTest.Analyzers/FlowTestContextCancellationTokenAnalyzer.cs | Make StartsWith usage explicit and ordinal. |
| src/Analyzers/MSTest.Analyzers/DoNotUseShadowingAnalyzer.cs | Avoid GetValueOrDefault usage for compatibility. |
| src/Analyzers/MSTest.Analyzers/DataRowShouldBeValidAnalyzer.cs | Replace IsAssignableTo usage with IsAssignableFrom patterns. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/MSTest.Analyzers.CodeFixes.csproj | Replace Polyfill package with compiled local polyfills. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/FlowTestContextCancellationTokenFixer.cs | Replace Ensure check with explicit null check. |
| src/Adapter/MSTestAdapter.PlatformServices/Utilities/XmlUtilities.cs | Replace Ensure with explicit null-guard. |
| src/Adapter/MSTestAdapter.PlatformServices/Utilities/DeploymentUtilityBase.cs | Remove Ensure checks in deployment loops. |
| src/Adapter/MSTestAdapter.PlatformServices/Utilities/DeploymentUtility.cs | Add non-NETCOREAPP ProcessId fallback and use it. |
| src/Adapter/MSTestAdapter.PlatformServices/Utilities/DeploymentItemUtility.cs | Remove tuple deconstruction for compatibility. |
| src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHost.cs | Use string overload of string.Join for compatibility. |
| src/Adapter/MSTestAdapter.PlatformServices/Services/TestDataSource.cs | Add non-NETCOREAPP enum parsing fallback. |
| src/Adapter/MSTestAdapter.PlatformServices/Services/SettingsProvider.cs | Replace Ensure with explicit null-guard. |
| src/Adapter/MSTestAdapter.PlatformServices/Services/MSTestAdapterSettings.cs | Replace Ensure with explicit null-guard. |
| src/Adapter/MSTestAdapter.PlatformServices/RunConfigurationSettings.cs | Remove StringSyntax annotation usage and add explicit null guard. |
| src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/UnitTestElement.cs | Replace Ensure with explicit null-guard. |
| src/Adapter/MSTestAdapter.PlatformServices/MSTestSettings.cs | Remove StringSyntax annotations; add non-NETCOREAPP enum helpers. |
| src/Adapter/MSTestAdapter.PlatformServices/MSTestAdapter.PlatformServices.csproj | Inline polyfills; exclude OS polyfill via constants. |
| src/Adapter/MSTestAdapter.PlatformServices/Helpers/TestDataSourceHelpers.cs | Replace IsAssignableTo usage with IsAssignableFrom patterns. |
| src/Adapter/MSTestAdapter.PlatformServices/Helpers/RunSettingsUtilities.cs | Remove StringSyntax annotations to reduce dependency. |
| src/Adapter/MSTestAdapter.PlatformServices/Helpers/ReflectHelper.cs | Replace Ensure with explicit null-guard; simplify return-type match. |
| src/Adapter/MSTestAdapter.PlatformServices/Extensions/MethodInfoExtensions.cs | Replace helper usage with reflection-based generic parameter check. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs | Replace Ensure with explicit null-guards. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TypeCache.cs | Adjust caching patterns for non-NETCOREAPP builds. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.cs | Replace CancelAsync usage with sync cancel + suppression. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestExecutionManager.cs | Remove tuple deconstruction for compatibility. |
| src/Adapter/MSTestAdapter.PlatformServices/AssemblyResolver.cs | Switch lock type based on TFM; replace Ensure checks. |
| src/Adapter/MSTest.TestAdapter/VSTestAdapter/MSTestExecutor.cs | Replace Ensure checks with explicit null + empty validation. |
| src/Adapter/MSTest.TestAdapter/MSTest.TestAdapter.csproj | Replace Polyfill package with compiled local polyfills. |
| src/Adapter/MSTest.Engine/MSTest.Engine.csproj | Replace Polyfill package with compiled local polyfills. |
| src/Adapter/MSTest.Engine/Engine/TestArgumentsManager.cs | Change dictionary registration logic. |
| Directory.Packages.props | Remove centralized Polyfill package version. |
| Directory.Build.props | Remove Polyfill package configuration properties. |
Comments suppressed due to low confidence (3)
src/Polyfills/OperatingSystem.cs:1
- The
extension(OperatingSystem)block is not valid C# in currently supported language versions and will fail to compile. Additionally, even if implemented as extension methods, callers cannot useOperatingSystem.IsBrowser()on TFMs whereSystem.OperatingSystemlacks these static members. Replace this with a normal helper API (e.g.,Polyfill.IsBrowser()/PlatformGuards.IsBrowser()) and update call sites to use that helper under#if !NETCOREAPP, or directly useRuntimeInformation.IsOSPlatform(OSPlatform.Create("BROWSER"))in the call sites.
src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxModeHelpers.cs:1 - On non-
NETCOREAPPTFMs,System.OperatingSystemdoes not exposeIsBrowser(), so this will not compile unless the polyfill injects an actualOperatingSystem.IsBrowserstatic member (which C# cannot do). Use an explicit#if NETCOREAPPsplit here (NETCOREAPP:!OperatingSystem.IsBrowser(), else:!RuntimeInformation.IsOSPlatform(OSPlatform.Create("BROWSER"))) or call a dedicated helper method from the polyfills that is callable on all TFMs.
src/Polyfills/NullableAttribtues.cs:1 - The filename has a typo: rename
NullableAttribtues.cstoNullableAttributes.csto avoid confusion and improve discoverability/maintenance.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 172 out of 172 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (9)
src/Polyfills/OperatingSystem.cs:1
extension(OperatingSystem)is not valid C# syntax in current stable language versions and will fail to compile (unless the repo is explicitly using preview extension-members support). Replace this with a C#-supported approach (e.g., a normal static helper class in thePolyfillsnamespace, or explicit helper methods that callers invoke), and update call sites accordingly.
src/Polyfills/Ensure.cs:1Ensure.NotEmpty<T>returns whenvalueis null, which defeats the purpose of an argument guard and can mask null input bugs. This should throwArgumentNullException(or otherwise be renamed to reflect that null is allowed).
test/Utilities/Microsoft.Testing.TestInfrastructure/ProjectSystem.cs:1- This is validating a constructor argument, so
InvalidOperationExceptionis the wrong exception type. PreferArgumentNullExceptionwhentfmsis null andArgumentException(orArgumentOutOfRangeException) when it’s empty; that keeps errors consistent and actionable for callers.
test/Utilities/Microsoft.Testing.TestInfrastructure/ProjectSystem.cs:1 - This is validating a constructor argument, so
InvalidOperationExceptionis the wrong exception type. PreferArgumentNullExceptionwhentfmsis null andArgumentException(orArgumentOutOfRangeException) when it’s empty; that keeps errors consistent and actionable for callers.
src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportEngine.cs:1 - The constructor has different parameter lists across TFMs. That forces
#ifat every call site (as seen in tests/consumers) and increases maintenance cost. Consider keeping a single signature across TFMs (e.g., always acceptCancellationTokenbut ignore it on non-NETCOREAPP), and only conditionalize the implementation (SaveAsyncvsSave).
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Helpers/CountDownEventTests.cs:1 CancellationTokenSource.CancelAsync()is not available on allNETCOREAPPTFMs (it was introduced much later than .NET Core itself). UsingNETCOREAPPhere is too broad and can break compilation when targeting earlier .NET versions. Use a more specific preprocessor symbol (e.g.,NET8_0_OR_GREATER) or avoidCancelAsyncentirely if consistent behavior is required.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Configuration/ConfigurationExtensionsTests.cs:1- The test name indicates an
ArgumentNullException, but the assertion now expectsInvalidOperationException. Please rename the test to reflect the new expected exception to keep test intent accurate.
src/Polyfills/NullableAttribtues.cs:1 - The filename
NullableAttribtues.cscontains a typo and should be renamed toNullableAttributes.csfor clarity and discoverability.
src/Polyfills/UnconditionalSuppressMessageAttribute.cs:1 - The XML doc
crefcontains an extra trailing space (MessageId), which can break doc tooling and links. Update it toMessageId.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 177 out of 177 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (5)
src/Polyfills/OperatingSystem.cs:1
- The
extension(OperatingSystem)block is not valid C# syntax in released language versions and will fail compilation. If the goal is to provideOperatingSystem.IsBrowser()-style APIs on TFMs where those static methods don't exist, consider instead introducing a polyfill type and consuming it via an alias (e.g.,using OperatingSystem = Polyfills.OperatingSystemPolyfill;) or refactoring call sites to use a dedicated helper (e.g.,PlatformOperatingSystem.IsBrowser()), gated by#if !NETCOREAPP.
src/Polyfills/IsExternalInit.cs:1 - This assembly attribute is inconsistent with the other type-forwarding files and is likely to fail to compile because
TypeForwardedTois not fully qualified andIsExternalInitis not namespace-qualified. Align with the rest of the polyfills by usingSystem.Runtime.CompilerServices.TypeForwardedToandtypeof(System.Runtime.CompilerServices.IsExternalInit)so it doesn't depend on global usings.
src/TestFramework/TestFramework.Extensions/TestFramework.Extensions.csproj:1 - MSBuild will treat the second
DefineConstantselement as an override, which likely dropsTRACEfrom the final constants. Combine these into a singleDefineConstantsentry (or append in one place) so bothTRACEand the new exclusion constants are preserved.
src/Polyfills/EmbeddedAttribute.cs:1 - Several projects in this PR (e.g., analyzer/code-fix projects) reference
Microsoft.CodeAnalysis.*packages. DefiningMicrosoft.CodeAnalysis.EmbeddedAttributein source can cause CS0433 type conflicts if the referenced Roslyn assemblies also define this type. A robust fix is to exclude this file from compilation in Roslyn-based projects (e.g., via<Compile Remove=\".../EmbeddedAttribute.cs\" />) so those projects use the Roslyn-provided attribute, while still compiling it into projects that don't reference Roslyn.
src/Polyfills/Ensure.cs:1 Ensure.NotEmpty<T>currently returns whenvalueis null, which makes the method name misleading and can silently accept null inputs in call sites that previously relied onEnsure.NotNullOrEmpty(...)throwing. Consider changing the null branch to throwArgumentNullException(name)to match typical guard semantics and avoid behavior regressions.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 178 out of 178 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (5)
src/Polyfills/OperatingSystem.cs:1
- The
extension(OperatingSystem)block is not valid C# syntax in mainstream language versions and will fail compilation. Replace this with regular APIs (e.g., a static helper class, or extension methods likepublic static bool IsBrowser(this OperatingSystem _)), and update call sites accordingly (or avoid needing these methods by usingRuntimeInformation.IsOSPlatform(...)directly).
src/Platform/Microsoft.Testing.Platform/Helpers/System/SystemEnvironment.cs:1 - The
fieldkeyword used as a backing store is a newer/preview C# feature and can break builds if the repo isn't compiling with that language version. Use an explicit private backing field (e.g.,_processId) instead offieldto keep this compatible with stable C# compilers.
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs:1 - This changes behavior from the previous
TryAdd(non-throwing) toAdd(throws if the key already exists) on NETCOREAPP. Since the old code explicitly usedTryAdd, it implies"node-type"may already be present in some cases; in that scenario this will now throw at runtime. Prefer keeping the non-throwing behavior by usingTryAdd(or aContainsKeyguard) on NETCOREAPP as well.
test/Utilities/Microsoft.Testing.TestInfrastructure/ProjectSystem.cs:1 - This is validating a constructor argument, so
InvalidOperationExceptionis not the most appropriate exception type. UseArgumentNullExceptionfortfms is nullandArgumentException(orArgumentOutOfRangeException) for an empty array so callers get standard parameter-validation semantics.
src/Polyfills/Ensure.cs:1 NotEmptyreturning successfully whenvalue is nullmakes the guard ineffective and is inconsistent with typical “not empty” contracts (and with the previous usage pattern ofEnsure.NotNullOrEmpty). This can permit null arguments to flow further and cause harder-to-debug failures later. Consider throwingArgumentNullExceptionhere (and similarly updatingNotEmpty(string value, ...)to throw on null) to preserve expected guard behavior.
09d3ffe to
528eaf4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 178 out of 178 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
src/Polyfills/OperatingSystem.cs:1
- The
extension(OperatingSystem)block is not valid C# syntax in current, broadly supported language versions and is likely to fail compilation for non-NETCOREAPP targets now that Polyfill is no longer provided via a package/tooling layer. Consider replacing this with regular helper methods (e.g.,Polyfills.Polyfill.IsBrowser()etc.) and updating the non-NETCOREAPP call sites to use that helper, or otherwise remove/replace this file with portableRuntimeInformation.IsOSPlatform(...)checks directly at call sites.
src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs:1 - This changes behavior from the previous
TryAddlogic: on NETCOREAPP,properties.Add(...)will now throw ifnode-typeis already present. To preserve the prior behavior (do nothing if already set), useTryAddon NETCOREAPP as well, and keep the guardedContainsKeyfallback only for TFMs whereTryAddisn't available.
src/Polyfills/Ensure.cs:1 Ensure.NotEmpty<T>silently returns whenvalueis null, which is surprising for anEnsure-style guard and can hide null-argument bugs. If the intent is to enforce both non-null and non-empty, this should throwArgumentNullExceptionwhenvalueis null. If the intent is only to enforce non-empty when non-null, consider renaming to something likeNotEmptyIfNotNullto make the contract explicit.
test/Utilities/Microsoft.Testing.TestInfrastructure/ProjectSystem.cs:1- Invalid/absent
tfmsis an argument validation issue, but this throwsInvalidOperationExceptionwith a message that doesn't include the parameter name. PreferArgumentNullException(nameof(tfms))when null is passed, andArgumentException(withparamName: nameof(tfms)) when the params array is empty, to align with .NET argument-validation conventions and improve diagnosability.
src/Polyfills/NullableAttribtues.cs:1 - The filename
NullableAttribtues.csappears to be misspelled; consider renaming toNullableAttributes.csto avoid confusion and improve discoverability.
Fixes #7596