Handle test parser forwarded args

[baronfel]

Description

A tech debt PR for the CLI's command line parsing resulted in a regression where unknown tokens were no longer forwarded along in the dotnet test command. This is behavior we want, since commands that forward to MSBuild shouldn't have to embed the entire MSBuild CLI syntax in their own parsers. As a result of this regression, users were unable to pass MSBuild arguments directly, instead having to use workarounds like setting environment variables instead. This was reported in microsoft/vstest#4014.

Regression

Yes, this worked in preview 7.

Risk

Low, this uses the same mechanism we have in other locations in the codebase to forward the tokens along

Testing

• manual testing
• automated tests to cover msbuild argument forwarding for this command

Details

Closes microsoft/vstest#4014

With the switch away from the legacy -- handling, users of System.CommandLine have two options for handling unrecognized arguments:

• adding an explicit argument to capture them and doing something explicit with them, or
• using the ParseResult.UnmatchedTokens property to get unmatched tokens (tokens that the parser doesn't explicitly have handling for) and do something explicit with them

In this PR I've taken the former latter approach because the former approach results in a help output that is too noisy.

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

[baronfel]

The new test output has some duplication that I can't track down the source of:

Description:
  .NET Test Driver                                                                                                                                                                                                                                                                                                                                                                                                                Usage:
  dotnet test [<PROJECT | SOLUTION> [<FORWARDED_ARGS>...]] [options] [[--] <additional arguments>...]]

Arguments:
  <PROJECT | SOLUTION>  The project or solution file to operate on. If a file is not specified, the command will search the current directory for one.
  <FORWARDED_ARGS>      Arguments to forward to MSBuild.

Options:
  -s, --settings <SETTINGS_FILE>                             The settings file to use when running tests.
  -t, --list-tests                                           List the discovered tests instead of running the tests.
  -e, --environment <NAME="VALUE">                           Sets the value of an environment variable.
                                                             Creates the variable if it does not exist, overrides if it does.
                                                             This will force the tests to be run in an isolated process.
                                                             This argument can be specified multiple times to provide multiple variables.

                                                             Examples:
                                                             -e VARIABLE=abc
                                                             -e VARIABLE="value with spaces"
                                                             -e VARIABLE="value;seperated with;semicolons"
                                                             -e VAR1=abc -e VAR2=def -e VAR3=ghi
  --filter <EXPRESSION>                                      Run tests that match the given expression.
                                                                                                     Examples:
                                                                                                     Run tests with priority set to 1: --filter "Priority = 1"
                                                                                                     Run a test with the specified full name: --filter "FullyQualifiedName=Namespace.ClassName.MethodName"
                                                                                                     Run tests that contain the specified name: --filter "FullyQualifiedName~Namespace.Class"
                                                                                                     See https://aka.ms/vstest-filtering for more information on filtering support.
  --test-adapter-path <ADAPTER_PATH>                         The path to the custom adapters to use for the test run.
  -l, --logger <LOGGER>                                      The logger to use for test results.
                                                                                                     Examples:
                                                                                                     Log in trx format using a unique file name: --logger trx
                                                                                                     Log in trx format using the specified file name: --logger "trx;LogFileName=<TestResults.trx>"
                                                                                                     See https://aka.ms/vstest-report for more information on logger arguments.
  -o, --output <OUTPUT_DIR>                                  The output directory to place built artifacts in.
  -d, --diag <LOG_FILE>                                      Enable verbose logging to the specified file.
  --no-build                                                 Do not build the project before testing. Implies --no-restore.
  --results-directory <RESULTS_DIR>                          The directory where the test results will be placed.
                                                             The specified directory will be created if it does not exist.
  --collect <DATA_COLLECTOR_NAME>                            The friendly name of the data collector to use for the test run.
                                                                                                     More info here: https://aka.ms/vstest-collect
  --blame                                                    Runs the tests in blame mode. This option is helpful in isolating problematic tests that cause the test host to crash or hang.
                                                             When a crash is detected, it creates an sequence file in TestResults/guid/guid_Sequence.xml that captures the order of tests that were run before
                                                             the crash.
                                                             Based on the additional settings, hang dump or crash dump can also be collected.
                                                             Example:
                                                               Timeout the test run when test takes more than the default timeout of 1 hour, and collect crash dump when the test host exits unexpectedly.
                                                               (Crash dumps require additional setup, see below.)
                                                               dotnet test --blame-hang --blame-crash
                                                             Example:
                                                               Timeout the test run when a test takes more than 20 minutes and collect hang dump.
                                                               dotnet test --blame-hang-timeout 20min
  --blame-crash                                              Runs the tests in blame mode and enables collecting crash dump when testhost exits unexpectedly.
                                                             This option is currently only supported on Windows, and requires procdump.exe and procdump64.exe to be available in PATH.
                                                             Or PROCDUMP_PATH environment variable to be set, and point to a directory that contains procdump.exe and procdump64.exe.
                                                             The tools can be downloaded here: https://docs.microsoft.com/sysinternals/downloads/procdump
                                                             Implies --blame.
  --blame-crash-dump-type <blame-crash-dump-type>            The type of crash dump to be collected. Implies --blame-crash.
  --blame-crash-collect-always <blame-crash-collect-always>  Enables collecting crash dump on expected as well as unexpected testhost exit.
  --blame-hang                                               Run the tests in blame mode and enables collecting hang dump when test exceeds the given timeout. Implies --blame-hang.
  --blame-hang-dump-type <blame-hang-dump-type>              The type of crash dump to be collected. When None, is used then test host is terminated on timeout, but no dump is collected. Implies --blame-hang.
  --blame-hang-timeout <TIMESPAN>                            Per-test timeout, after which hang dump is triggered and the testhost process is terminated.
                                                             The timeout value is specified in the following format: 1.5h / 90m / 5400s / 5400000ms. When no unit is used (e.g. 5400000), the value is assumed
                                                             to be in milliseconds.
                                                             When used together with data driven tests, the timeout behavior depends on the test adapter used. For xUnit and NUnit the timeout is renewed after
                                                             every test case,
                                                             For MSTest, the timeout is used for all testcases.
                                                             This option is currently supported only on Windows together with netcoreapp2.1 and newer. And on Linux with netcoreapp3.1 and newer. OSX and UWP
                                                             are not supported.
  --nologo                                                   Run test(s), without displaying Microsoft Testplatform banner
  -c, --configuration <CONFIGURATION>                        The configuration to use for running tests. The default for most projects is 'Debug'.
  -f, --framework <FRAMEWORK>                                The target framework to run tests for. The target framework must also be specified in the project file.
  -r, --runtime <RUNTIME_IDENTIFIER>                         The target runtime to test for.
  --no-restore                                               Do not restore the project before building.
  --interactive                                              Allows the command to stop and wait for user input or action (for example to complete authentication).
  -v, --verbosity <LEVEL>                                    Set the MSBuild verbosity level. Allowed values are q[uiet], m[inimal], n[ormal], d[etailed], and diag[nostic].
  -a, --arch <arch>                                          The target architecture.
  --os <os>                                                  The target operating system.
  -?, -h, --help                                             Show command line help.


Additional Arguments:
  Arguments passed to the application that is being run.

The 'additional arguments' portion seems to have resources defined, but I don't see where those resources are being accessed.

[baronfel]

Those are defined in the core Localized resources of System.CommandLine, so unless we remove them for all commands there's no getting around it.

[baronfel]

Using the unmatched tokens approach is perhaps cleaner - it doesn't pollute the help output, and it is easy to union the unmatched tokens on the total list of known arguments. The final result seems to work well:

2:39:05 ❯ dotnet test .\demo.csproj /p:Configuration=Release -bl
D:\Code\dotnet-sdk\.dotnet\sdk\7.0.100-preview.7.22377.5\MSBuild.dll -nologo -bl -distributedlogger:Microsoft.DotNet.Tools.MSBuild.MSBuildLogger,D:\Code\dotnet-sdk\.dotnet\sdk\7.0.100-preview.7.22377.5\dotnet.dll*Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingLogger,D:\Code\dotnet-sdk\.dotnet\sdk\7.0.100-preview.7.22377.5\dotnet.dll -maxcpucount -nodereuse:false -restore -target:VSTest -verbosity:m /p:Configuration=Release -property:VSTestArtifactsProcessingMode=collect -property:VSTestSessionCorrelationId=307080_ee2e0d4f-c859-4f43-860e-f21092c97c4b .\demo.csproj

In this example you can see both the /p property and the -bl flag forwarded to the test target.

[baronfel]

@nohwnd I pinged you for review because you're familiar with everything going on here.

[nohwnd]

@baronfel I will be leaving for paternity leave some time soon, @Evangelink will take over this for me. We would like to get it into 7.0.1xx servicing.

[baronfel]

@nohwnd congratulations and best of luck! I'll work with @Evangelink to get this in. A little slammed ATM with .NET Conf prep, but we'll make it happen.

[Evangelink]

I will check with team tomorrow for help but I cannot run/debug tests from VS nor from command line using dotnet although I am following the steps described in https://github.com/dotnet/sdk/blob/main/documentation/project-docs/developer-guide.md.

So I pushed only a partial change in case you have some time to push forward. Otherwise, I will resume my work tomorrow.

[Evangelink]

@baronfel @nohwnd PR is fixed. I did a few more manual tests and everything is working as expected.

I am wondering if it could be worth to add a few tests were we expect msbuild to fail by passing some extra invalid arguments. WDYT?

[Evangelink]

@baronfel I added some nitpicks, could you please apply them?