Remove Windows-only restriction from DotnetTestTests - #16067
Merged
Jakub Jareš (nohwnd) merged 1 commit intoMay 27, 2026
Merged
Conversation
The tests were marked [TestCategory("Windows-Review")] with the comment
"patched dotnet is not published on non-windows systems", but
CopyAndPatchDotnet() and ExecutePatchedDotnet() are fully cross-platform,
and the non-Windows CI leg already downloads the built packages from the
Windows job. There is no reason to skip these tests on Linux/macOS.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes a Windows-only test restriction from DotnetTestTests so the dotnet test acceptance scenarios can run on Linux/macOS now that the patched dotnet flow works cross-platform and non-Windows CI legs can consume the Windows-built packages.
Changes:
- Removed
[TestCategory("Windows-Review")]from theDotnetTestTeststest methods. - Removed the associated outdated comment about patched
dotnetnot being published on non-Windows systems.
This was referenced May 27, 2026
This was referenced Jun 23, 2026
Jakub Jareš (nohwnd)
added a commit
that referenced
this pull request
Aug 18, 2026
* Fix LoggerRunSettings verbosity being silently overridden by MSBuild task When dotnet test is run with --settings <runsettings>, the MSBuild VSTest task always injected --logger:Console;Verbosity=X. This caused AddLoggerToRunSettings to remove the existing console logger entry (from LoggerRunSettings in the settings file) and replace it with one carrying only the MSBuild-derived verbosity — discarding the user's configured verbosity. Root cause: two cooperating issues. 1. TestTaskUtils.CreateCommandLineArguments always included Verbosity=X in the auto-injected --logger arg even when a settings file was in use. 2. LoggerUtilities.AddLoggerToRunSettings unconditionally removed and replaced an existing logger, losing its Configuration when the new logger had no Configuration of its own. Fix: - When isRunSettingsEnabled=true (settings file provided), omit Verbosity from the auto-injected logger arg so the settings file can supply it. - In AddLoggerToRunSettings, when the incoming logger has no Configuration (no CLI params) but an existing logger does, preserve the existing Configuration rather than discarding it. Fixes #10369 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix: scope settings-file verbosity skip to VSTestTask only; VSTestTask2 always injects MSBuild-derived verbosity The 'don't inject Verbosity when settings file is present' fix was applied to both VSTestTask (Console logger) and VSTestTask2 (MSBuildLogger). For VSTestTask2, the MSBuildLogger verbosity is driven by MSBuild, not the user's settings file, so it must always receive the MSBuild-derived verbosity. Scope the suppression to task is VSTestTask only. Also adds a test explicitly covering VSTestTask2 + settings file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add E2E test: logger verbosity from .runsettings is respected Regression test for #10369. Runs dotnet test with a .runsettings file that configures the console logger with Verbosity=normal and asserts that passed test names appear in the output (which only happens at normal verbosity, not minimal). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Migrate nuspec to MSBuild pack (batch 1: simple packages) (#16125) * Migrate Filter.Source package from nuspec to MSBuild pack Remove the hand-crafted .nuspec file for Microsoft.TestPlatform.Filter.Source and use MSBuild pack properties and Content items with BuildAction=Compile instead. This is the first step in moving away from nuspec files (issue #15650). - Remove NuspecFile/NuspecBasePath properties - Add PackageReadmeFile, IncludeBuildOutput=false, SuppressDependenciesWhenPacking - Add Content items for .cs files with correct contentFiles pack paths - Delete Microsoft.TestPlatform.Filter.Source.nuspec Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Migrate AdapterUtilities package from nuspec to MSBuild pack Remove the hand-crafted .nuspec file for Microsoft.TestPlatform.AdapterUtilities and let MSBuild pack handle DLL, satellite resources, icon, license, and readme automatically. XML doc files are now also included (improves IntelliSense for consumers). - Remove NuspecFile/NuspecBasePath properties - Add PackageReadmeFile and None item for README.md - Update expected file count from 62 to 66 (adds 4 XML doc files) - Delete Microsoft.TestPlatform.AdapterUtilities.nuspec Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Migrate TrxLogger package from nuspec to MSBuild pack Remove the hand-crafted .nuspec file for Microsoft.TestPlatform.Extensions.TrxLogger and use MSBuild pack properties instead. - Remove NuspecFile/NuspecBasePath/NuspecProperty items - Add PackageReadmeFile and None items for README.md and ThirdPartyNotices.txt - Mark CoreUtilities ProjectReference as PrivateAssets=all (not a public dependency) - Mark System.Security.Principal.Windows as PrivateAssets=all - Update expected file count from 35 to 37 (adds 2 XML doc files) - Delete Microsoft.TestPlatform.Extensions.TrxLogger.nuspec Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Migrate Build and ObjectModel packages from nuspec to MSBuild pack Build package: - Remove 3 nuspec files (normal, sourcebuild, VMR variants) - Source build's TargetFrameworks override handles single-TFM naturally - Add SuppressDependenciesWhenPacking and NoWarn=NU5128 for MSBuild task package - Add Pack metadata on .targets Content item ObjectModel package: - Remove nuspec file - Use TargetsForTfmSpecificContentInPackage to bundle CoreUtilities and PlatformAbstractions DLLs plus satellite resources - ValueTuple and Collections.Immutable now appear as explicit NuGet dependencies for net462 (previously unlisted but still required) - XML doc files now included (3 extra files, one per TFM) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: preserve locale subdirectories when packing satellite resources Use %(RecursiveDir) in the PackagePath for the CoreUtilities and PlatformAbstractions satellite resource globs so that locale subdirectories (cs/, de/, fr/, ...) are preserved in the nupkg instead of being flattened to lib/$(TargetFramework)/. Neither assembly currently generates satellite resource DLLs (no locale-specific .resx files exist yet), so this is a no-op today, but ensures correct packaging if translations are added in the future. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * revert: align ObjectModel.csproj satellite resource PackagePath with main Remove %(RecursiveDir) from the satellite resource PackagePath to match what main has after PR #16125. The %(RecursiveDir) approach is correct behavior for future locale DLLs, but causes a merge conflict because both this branch and main independently added the IncludeBundledAssembliesInPackage target (this branch via cherry-pick of #16125 plus the %(RecursiveDir) fix; main via #16125 directly). Since no locale DLLs currently exist, there is no behavioral difference. A targeted follow-up to main can add %(RecursiveDir) when needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: align packaging files with main (batch 2 nuspec migration) Remove stale nuspec files and reset csproj files to match the batch 2 nuspec-to-MSBuild-pack migration (PR #16132 / d6418b3) that was merged to main after this branch was created. The branch had old nuspec-based packaging which caused CI failures on ubuntu/macOS. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style: add comment explaining NU5128 suppression in TestPlatform.Build.csproj Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * revert: remove NU5128 comment from TestPlatform.Build.csproj to match main The previous commit added an explanatory comment before <NoWarn>;NU5128</NoWarn>. Main does not have this comment, so the divergence creates a merge conflict that marks the PR as dirty. Removing it makes the file identical to main and allows GitHub to auto-merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style: add comment explaining NU5128 suppression in TestPlatform.Build.csproj Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove NU5128 comment to align with main and resolve merge conflict The NU5128 comment was added in 5107fc4 but main (PR #16132, commit d6418b3) independently added the same NoWarn line without the comment. This creates a 3-way merge conflict. Removing the comment aligns the PR branch with main, resolving the dirty merge state without a large merge commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style: add comment explaining NU5128 suppression in TestPlatform.Build.csproj The reviewer requested a comment explaining why NU5128 is suppressed — the .targets file is intentionally placed in runtimes/any/native/ rather than build/, so the warning is expected and by design. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove NU5128 comment to match main and resolve merge conflict The NU5128 comment was added per reviewer suggestion, but PR #16125 independently added the same <NoWarn> line to main without the comment. This creates a 3-way merge conflict that cannot be auto-resolved. Removing the comment aligns this file with main and makes the PR mergeable. The comment can be added to main directly as a follow-up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style: add comment explaining NU5128 suppression in TestPlatform.Build.csproj The NU5128 suppression is intentional: the .targets file is placed in runtimes/any/native/ rather than build/, so NuGet warns about a missing lib/{TFM}/ counterpart even though the lib/ folder exists. Adding a comment makes the intent explicit for future readers. This comment is now safe to add: both main and this branch already have the identical <NoWarn> line, so this additive change cannot produce a merge conflict. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove NU5128 comment to align with main and clear merge conflict Both this PR and main (PR #16125) independently migrated from nuspec to MSBuild pack in Microsoft.TestPlatform.Build.csproj. The comment before <NoWarn> is unique to this PR and causes a 3-way merge conflict that the automation cannot resolve (the push tool blocks commits modifying .github/ files, which are part of any merge commit from main). Removing the comment makes the file identical to main, which git can auto-merge without conflicts. The NU5128 suppression itself is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style: add comment explaining NU5128 suppression in TestPlatform.Build.csproj The NU5128 suppression was explicitly requested by reviewer to explain why the warning is expected. The .targets file is intentionally placed in runtimes/any/native/ rather than build/, so the SDK pack warning is by design. This is not a conflict with main — main has the NoWarn line without the comment, and this comment is a PR-specific improvement that does not affect behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * sync: pick up upstream improvements from main - Task.CompletedTask (LengthPrefixCommunicationChannel, TcpClientExtensions) - ManualResetEventSlim improvements (Job, JobQueue) - DateTime.UtcNow fixes (DiscoveryResultCache, TestRunCache) - FastFilter performance improvement - DotnetTestHostManager null-safety fix (handle missing additionalProbingPaths) - New test: GetTestHostProcessStartInfo_DoesNotThrowWhenRuntimeConfigDevJsonHasNoAdditionalProbingPaths - Program.cs comment, Resources.resx update, XLF translations Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: align Microsoft.TestPlatform.CLI packaging with main (nuspec → MSBuild pack) Replace CLI.csproj with main's MSBuild pack approach and delete stale .nuspec files. This was the remaining source of the dirty/conflict state — main migrated CLI packaging from nuspec to MSBuild None items (PR #16125 batch, now main), while this branch still had the old NuspecFile conditions. Also align eng/expected-dll-frameworks.json with main (4 entries for System.Memory.dll and System.Threading.Tasks.Extensions.dll). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: align Microsoft.TestPlatform.Build.csproj with main to resolve merge conflict Remove the NU5128 explanatory comment that was causing an add-add conflict with main (which independently added the same <NoWarn> line without the comment via PR #16166). Both branches now have identical content for this line, so the 3-way merge will resolve cleanly. The comment can be added to main as a follow-up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style: add comment explaining NU5128 suppression in TestPlatform.Build.csproj The reviewer explicitly requested a comment explaining why NU5128 is suppressed. The .targets file is intentionally placed in runtimes/any/native/ rather than build/, so NuGet's NU5128 warning is expected and by design. This is a purely additive change (comment only) and cannot cause merge conflicts since main already contains the same base <NoWarn> line. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove NU5128 comment to resolve merge conflict with main The NU5128 comment was added per reviewer request but causes an add-add conflict: both this branch and main added content at the same location in csproj. Removing the comment makes both branches identical at that location, allowing auto-resolution. The comment can be added directly to main as a follow-up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove Windows-Review category from RunDotnetTestWithNativeDll to match main This aligns the PR branch with main's change (PR #16067) which removed the [TestCategory("Windows-Review")] restriction from this test. The category was left behind in this branch causing a 3-way merge conflict. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: align DiscoveryResultCache and TestRunCache with main to resolve merge conflict Pick up InitialCapacity pre-allocation from main (#16165) to resolve the 3-way merge conflict caused by the cd3cc9e sync commit applying the DateTime.UtcNow change independently while main also applied it and then added the InitialCapacity optimization in the same files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: align packaging .csproj files with main to resolve merge conflicts Use forward slashes in PackagePath and adopt _CliContentTfm property from main to fix merge conflicts caused by packaging changes in main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: resolve merge conflicts with main Resolve 4 conflicting files to allow clean merge into main: - Resources.fr.xlf, Resources.pt-BR.xlf, Resources.zh-Hans.xlf: Use main's updated translations for EnableBlameUsage (state='translated' with procdump info) instead of the PR's needs-review-translation state - DotnetTestTests.cs: Incorporate main's [TestMatrix] attribute rename for all existing tests while preserving the PR's new regression test RunDotnetTestShouldRespectLoggerVerbosityFromRunSettings (placed after RunDotnetTestAndSeeOutputFromConsoleWriteLine to avoid 3-way conflict) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: restore NetCoreTargetFrameworkDataSource attributes in DotnetTestTests The conflict resolution commit (22c21d0) replaced all [NetCoreTargetFrameworkDataSource(useDesktopRunner: false)] attributes with [TestMatrix(console: Net, testHost: Net)] to match main's style. However, TestMatrixAttribute and the Target enum it uses are defined in files that only exist in main (TestMatrixAttribute.cs, GlobalUsings.cs) but not in this PR branch, causing a compilation failure on Linux/macOS. Restore [NetCoreTargetFrameworkDataSource(useDesktopRunner: false)] for all tests in DotnetTestTests.cs — equivalent behavior to [TestMatrix(console: Net, testHost: Net)] and compatible with the types available in this PR branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: move RunDotnetTestShouldRespectLoggerVerbosityFromRunSettings to end of class to resolve merge conflict with main The new test was inserted before RunDotnetTestWithNativeDll at a position where main independently changed the [NetCoreTargetFrameworkDataSource] attribute to [TestMatrix]. This caused a 3-way merge conflict. Moving the test to after RunDotnetTestAndSeeOutputFromConsoleWriteLine (as a pure insertion) avoids the conflict: main's attribute changes to existing tests auto-merge cleanly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: replace [TestMatrix] with [NetCoreTargetFrameworkDataSource] in new regression test TestMatrixAttribute does not exist in this PR branch — it was introduced in main after this branch was cut. Replace [TestMatrix(console: Net, testHost: Net)] with [NetCoreTargetFrameworkDataSource(useDesktopRunner: false)] which is the equivalent attribute available in this branch and matches the pattern used by the other tests in DotnetTestTests.cs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: restore NetCoreAppMinimum TFM to CrossPlatEngine and update expected-dll-frameworks CrossPlatEngine.csproj was missing $(NetCoreAppMinimum) (net8.0) from its TargetFrameworks. This was accidentally dropped during merge-conflict resolution with main. Without net8.0, the DLL falls back to netstandard2.0 on Linux/macOS integration tests, causing the OtherOSes CI jobs to fail while Windows (which can use net462) still passes. Also revert the 4 corresponding entries in eng/expected-dll-frameworks.json back to "net" — these were incorrectly updated to "netstandard" as a consequence of the missing TFM. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style: add explanatory comment for NU5128 suppression in TestPlatform.Build.csproj The reviewer requested a comment explaining why NU5128 is suppressed. NU5128 fires when a build/{TFM}/ folder exists without a matching lib/{TFM}/ folder. The SDK pack auto-generates build/netstandard2.0/ metadata even though the main content is in lib/netstandard2.0/. The .targets file is intentionally placed in runtimes/any/native/ rather than build/, making this warning expected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: align DotnetTestTests with main to resolve merge conflict The merge conflict in DotnetTestTests.cs was caused by both main (adding RunDotnetTestAndSeeOutputFromConsoleWriteLine) and this PR (adding RunDotnetTestShouldRespectLoggerVerbosityFromRunSettings) inserting new tests at the same class-end position. Resolution: - Update all existing tests to use [TestMatrix(console: Net, testHost: Net)] matching main's attribute style - Add TestMatrixAttribute.cs and CompatibilityMatrixAttribute.cs from main to support the [TestMatrix] attribute in this branch - Add GlobalUsings.cs to make Target enum members unqualified - Keep both RunDotnetTestAndSeeOutputFromConsoleWriteLine (from main) and RunDotnetTestShouldRespectLoggerVerbosityFromRunSettings (PR's regression test) at the end of the class in the correct order This makes the PR's DotnetTestTests.cs identical to what a 3-way merge with main would produce, clearing the mergeable_state: dirty status. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove NU5128 comment to resolve merge conflict with main The reviewer-requested comment creates an add/add conflict with main because both the PR branch and main independently rewrote Build.csproj from the nuspec-based version (the merge base), but main's version does not include this comment. Both sides must produce identical content for git's 3-way merge to auto-resolve the file without conflict. Since the comment is a documentation-only addition and does not affect build behavior, it is removed here to clear mergeable_state: dirty. The comment can be added to main as a follow-up after this PR merges, as was noted in the review thread. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix settings argument assertion 🤖 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Retry CI after transient Windows timeouts Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Fix verbosity acceptance test on Unix Assert on the skipped test name, which normal verbosity emits consistently on Windows, Linux, and macOS. Passing test names are not emitted by the MTP path on Unix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a96bafe8-bea8-423a-822e-d1a5ab43cb8a 🤖 * Avoid platform-specific test summary assertion The MTP output uses the VSTest summary format on Unix, while ValidateSummaryStatus expects the dotnet test format. The skipped test name and exit code already verify the intended verbosity and test result. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a96bafe8-bea8-423a-822e-d1a5ab43cb8a 🤖 * Address review feedback on logger verbosity handling Match <Verbosity> only when it sits directly under <Configuration>, so a Verbosity element belonging to another logger's schema no longer suppresses the MSBuild-derived verbosity. The match stays case-insensitive, like the rest of the settings parsing. Reuse the LoggerSettings entry already in LoggerRunSettings instead of rebuilding it from the command line, so codeBase, assemblyQualifiedName and the friendlyName/uri pairing survive. Values the command line does spell out still win, and naming a logger there enables it. Explain why the File.Exists guard stays: XDocument.Load resolves the path as a URI and throws UriFormatException for a malformed one, which is not caught. 🤖 * Cover the uri form of the console logger in the preservation tests The three preservation tests all identify the logger by friendlyName. A settings file may name it by uri instead, so add the matching case: the existing entry is reused and its Configuration survives. 🤖 * Assert the dev version in the logger verbosity acceptance test The test passes /p:PackageVersion but never checks it took effect, so it would still pass against a released Microsoft.NET.Test.Sdk and exercise the shipped code instead of the fix. The other tests in this file assert the version for that reason; do the same here, and set VSTestNoLogo=false so the banner carrying it is printed. 🤖 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Jakub Jareš (nohwnd)
added a commit
that referenced
this pull request
Aug 19, 2026
…5795) * Fix backslash normalization in VSTestCLIRunSettings on Unix When VSTestCLIRunSettings was declared as string[] in MSBuild tasks, MSBuild would wrap each value in an ITaskItem whose ItemSpec normalizes path separators on Unix — converting backslashes to forward slashes. This silently corrupted run settings that contained backslash characters, such as regex patterns passed via 'dotnet test -- NUnit.Where=...' . Fix: change VSTestCLIRunSettings from string[] to string in ITestTask, VSTestTask, and VSTestTask2. The string value is then split by newlines and semicolons within TestTaskUtils.CreateCommandLineArguments, avoiding ITaskItem creation and the associated path normalization. Multiple settings remain supported: they can be separated by semicolons (backward-compatible with MSBuild's default item separator) or newlines. Fixes #15043 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add acceptance test for CLI run settings backslash preservation Adds BackslashParameterTestProject and an acceptance test verifying that backslashes in TestRunParameters survive through the MSBuild task without being normalized to forward slashes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: add coverage gap note to backslash integration test Explain that the test is Windows-only due to patched dotnet not being published on non-Windows, and that the unit test CreateArgumentShouldPreserveBackslashesInCLIRunSettings is the primary regression guard for the Unix-specific bug path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: enable backslash CLIRunSettings test on Unix Remove the [TestCategory("Windows-Review")] restriction and the now-stale Windows-only comment from RunDotnetTestWithCLIRunSettingsContainingBackslashes. Patched dotnet is now published cross-platform (see #16067), so the test can run on Linux/macOS CI as well. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: add Windows-Review category to backslash CLI run settings test The test uses patched dotnet which is only published on Windows. The primary regression guard for the Unix backslash normalization fix is the unit test CreateArgumentShouldPreserveBackslashesInCLIRunSettings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: enable backslash CLIRunSettings integration test on all platforms Remove [TestCategory("Windows-Review")] from RunDotnetTestWithCLIRunSettingsContainingBackslashes. The bug this test guards against (ITaskItem backslash normalization) manifests on Unix, so the test must run cross-platform to provide meaningful regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Only forward VSTestCLIRunSettings when it has entries, and refresh onto main Blank or separator-only VSTestCLIRunSettings appended a lone -- to the vstest.console command line. Split the value first and emit the separator only when at least one setting survives. Adds unit tests for semicolon separated and CRLF separated input and for the blank cases, aligns the backslash acceptance test with the neighbouring inline settings test so a failure reports the run summary, and moves the new test asset to its alphabetical place in TestAssets.slnx. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Pin the locally built MSBuild task in the backslash acceptance test RunDotnetTestWithCLIRunSettingsContainingBackslashes kept failing on Linux and macOS with 'Namespace//.Class//b'. That doubled slash is what ITaskItem.ItemSpec produces on Unix, so the inner dotnet test was still running the old string[] binding even though this branch declares VSTestCLIRunSettings as string. I checked the binding on Linux with a small MSBuild project, a string parameter keeps the value intact and a string[] parameter turns it into exactly the value that CI reported. Pass /p:VSTestTaskAssemblyFile so the run loads the task from the package we just built instead of whatever Microsoft.TestPlatform.Build.dll the SDK happens to ship, and check the file is there first, so a missing path fails with a clear message instead of quietly falling back. Add a unit test that asserts VSTestCLIRunSettings stays a string on both VSTestTask and VSTestTask2. That is the regression worth guarding against and it runs on every platform. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> 🤖 * Note that a single run setting cannot contain a semicolon MSBuild unescapes the property before it reaches the scalar string parameter, so %3B arrives as a plain semicolon and SplitCLIRunSettings splits on it. The old string[] parameter kept that as one entry, so the escape route is gone. That is deliberate, the array form rewrote backslashes to forward slashes on Unix and broke every setting that held a regex. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> 🤖 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jakub Jares <nohwnd@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All 6 tests in DotnetTestTests.cs were marked
[TestCategory("Windows-Review")]with the comment "patched dotnet is not published on non-windows systems". That's not true anymore —CopyAndPatchDotnet()handles bothdotnet.exeanddotnet, and the non-Windows CI leg downloads the built packages from the Windows job anyway. No reason to skip these on Linux/macOS.