Use custom argument parsers for the test args/run settings

[baronfel]

This builds on your changes to hopefully remove all of the ---aware logic from the CLI, replacing it with hopefully more user-friendly syntax while not harming backwards compatibility. The idea is that we push a lot of the argument parsing into custom ParseArgument delegates for the two arguments that we're attempting to parse - args to forward to the running application, and runsettings. If we have those captured at parse time, the execution-time logic gets much simpler.

[baronfel]

we need test to be part of the new error reporting as well if we want the nice parser errors to be shown to the user. now that we do explicit forwarding of args to commands like MSbuild, we could potentially remove this in favor of the standard UseParseErrorReporting() middleware, but I didn't think that was necessary for your ---removal PR at this time. It can come next :)

[baronfel]

I think this logic is a lot more understandable - let me know what you think

[Tunduk]

Really better.
But I think that we can remove ?? Array.Empty<string>() part

[Tunduk]

Is it ok that we don't check double dash? Seems like we can pass dotnet test --logger "trx;logfilename=custom.trx" RunConfiguration.ResultDirectory=

[baronfel]

we can actually test this now - I should add some tests where we take certain suspicious command lines, run them through the parser, and then do assertions on the parsed arguments. Do you have any specific command lines you'd recommend testing against?

[Tunduk]

I tried to run GivenDotnetTestBuildsAndRunsTestFromCsproj.ItAcceptsMultipleLoggersAsCliArguments test and it failed. This test was my pain and I remembered it =)
I think problem that System.CommandLine wasn't deployed with dotnet/command-line-api#1759 fix. And I can't find new version in nuget.

Unfortunately I don't use dotnet test very often, so nothing comes to mind =(
But I think that there are only 3 combinations that we should check.

[Tunduk]

My bad. System.CommandLine was updated.

[Tunduk]

I reproduced problem. Something odd happens when we pass 3 arguments to command and call command with 1 token.
If we call with >1 token then we will get exception from System.CommandLine.
Like this.

I'll create an issue for System.CommandLine team tomorrow
test.txt

[Tunduk]

@baronfel just FYI dotnet/command-line-api#1779

[Tunduk]

@baronfel Hi! System.CommandLine team has resolved the issue. I updated the branch with last commit.
You wanted to add some tests. Should I wait for yours tests or I can merge PR?

[baronfel]

Hi @Tunduk - I would like to add some testing for the new parser before this gets merged, but I am currently slammed on personal things at the moment. Do you know of any good examples for commands that use dotnet test in the wild? I'm thinking that it should be relatively easy to do a series of tests like this:

[Theory]
[InlineData(new[]{ "test", "a", b", "\"c=d\""}, "a", new[]{"b"}, new[]{"c=d"})]
public void Can_parse_arbitrary_test_command_line(string[] inputs, string? expectedSlnOrProj, string[] expectedForwardedArgs, string[] expectedRunSettings) {
	var parseResult = TestCommandParser.GetCommand().Parse(inputs);
	parseResult.GetValueForArgument(TestCommandParser.SolutionOrProjectArgument).Should().Equal(expectedSlnOrProj);
	parseResult.GetValueForArgument(TestCommandParser.SolutionOrProjectArgument).Should().SequenceEquals(expectedForwardedArgs);
	parseResult.GetValueForArgument(TestCommandParser.InlineRunSettings).Should().SequenceEquals(expectedRunSettings);
}

and so on.

[Tunduk]

Hi @baronfel!

If you busy I can add some tests. You are right, this tests shouldn't take a lot of time.
Unfortunately, I don't know good examples but it should be easy to find/think up.
I think we can merge this PR and I'll add tests in the original PR. What do you think?

[baronfel]

That sounds great to me :)