Use custom argument parsers for the test args/run settings #1
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| using System.CommandLine; | ||
| using System.CommandLine.Invocation; | ||
| using System.CommandLine.Parsing; | ||
| using System.IO; | ||
| using System.Linq; | ||
| using Microsoft.DotNet.Tools; | ||
| using Microsoft.DotNet.Tools.Test; | ||
|
|
@@ -17,12 +18,132 @@ internal static class TestCommandParser | |
| { | ||
| public static readonly string DocsLink = "https://aka.ms/dotnet-test"; | ||
|
|
||
| /// <summary> | ||
| /// Parser delegate that only accepts a token that is a | ||
| /// * project or solution file | ||
| /// * directory that contains a project or solution | ||
| /// * .dll or .exe file | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// S.CL usage note - OnlyTake(0) signals that this token should be returned to the | ||
| /// token stream for the next argument. In this way we prevent initial tokens that | ||
| /// are not relevant from being captured in this argument, since the syntax is a bit | ||
| /// ambiguous. | ||
| /// </remarks> | ||
| private static ParseArgument<string> IsProjectOrSln = | ||
| (ctx) => | ||
| { | ||
| bool HasProjectOrSolution(DirectoryInfo dir) => | ||
| dir.EnumerateFiles("*.*proj").Any() || dir.EnumerateFiles(".sln").Any(); | ||
|
|
||
| if (ctx.Tokens.Count == 0) | ||
| { | ||
| ctx.OnlyTake(0); | ||
| return null; | ||
| } | ||
| else | ||
| { | ||
| var tokenValue = ctx.Tokens[0].Value; | ||
| var ext = System.IO.Path.GetExtension(tokenValue); | ||
| if (ext.EndsWith("proj") || ext.EndsWith(".sln") || ext.EndsWith(".dll") || ext.EndsWith(".exe")) | ||
| { | ||
| ctx.OnlyTake(1); | ||
| return tokenValue; | ||
| } | ||
| else | ||
| { | ||
| var path = System.IO.Path.GetFullPath(tokenValue); | ||
| var dir = new System.IO.DirectoryInfo(path); | ||
| if (dir.Exists && HasProjectOrSolution(dir)) | ||
| { | ||
| ctx.OnlyTake(1); | ||
| return tokenValue; | ||
| } | ||
|
|
||
| ctx.OnlyTake(0); | ||
| return null; | ||
Tunduk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| }; | ||
|
|
||
| public static readonly Argument<string> SlnOrProjectArgument = new Argument<string>(CommonLocalizableStrings.SolutionOrProjectArgumentName, parse: IsProjectOrSln) | ||
| { | ||
| Description = CommonLocalizableStrings.SolutionOrProjectArgumentDescription, | ||
| Arity = ArgumentArity.ZeroOrOne | ||
| }; | ||
|
|
||
| public static (bool success, string key, string value) TryParseRunSetting (string token) { | ||
| var parts = token.Split('='); | ||
| if (parts.Length == 2) { | ||
| return (true, parts[0], parts[1]); | ||
| } | ||
| return (false, null, null); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// A parser that takes from the start of the token stream until the the first argument that could be a RunSetting | ||
| /// </summary> | ||
| private static ParseArgument<string[]> ParseUntilFirstPotentialRunSetting = ctx => | ||
| { | ||
| if (ctx.Tokens.Count == 0) | ||
| { | ||
| ctx.OnlyTake(0); | ||
| return Array.Empty<string>(); | ||
| } | ||
| var tokens = new List<string>(); | ||
| foreach (var token in ctx.Tokens) { | ||
| var (success, key, value) = TryParseRunSetting(token.Value); | ||
| if (success) { | ||
| break; | ||
| } | ||
| else | ||
| { | ||
| tokens.Add(token.Value); | ||
| } | ||
| } | ||
| ctx.OnlyTake(tokens.Count); | ||
| return tokens.ToArray(); | ||
| }; | ||
|
|
||
| public static readonly Argument<string[]> ForwardedArgs = new(LocalizableStrings.AdditionalArguments, parse: ParseUntilFirstPotentialRunSetting, description: LocalizableStrings.AdditionalArgumentsDescription) | ||
| { | ||
| Arity = ArgumentArity.ZeroOrMore | ||
| }; | ||
|
|
||
| private static ParseArgument<(string key, string value)[]> ParseRunSettings = ctx => | ||
|
Owner
There was a problem hiding this comment. Is it ok that we don't check double dash? Seems like we can pass
Author
There was a problem hiding this comment. 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?
Owner
There was a problem hiding this comment. I tried to run Unfortunately I don't use
Owner
There was a problem hiding this comment. My bad. System.CommandLine was updated.
Owner
There was a problem hiding this comment. I reproduced problem. Something odd happens when we pass 3 arguments to command and call command with 1 token. I'll create an issue for System.CommandLine team tomorrow |
||
| { | ||
| if (ctx.Tokens.Count == 0) | ||
| { | ||
| ctx.OnlyTake(0); | ||
| return Array.Empty<(string key, string value)>(); | ||
| } | ||
| var settings = new List<(string key, string value)>(ctx.Tokens.Count); | ||
| var consumed = 0; | ||
| foreach (var token in ctx.Tokens) | ||
| { | ||
| var parts = token.Value.Split('=', 2); | ||
| if (parts.Length == 2) | ||
| { | ||
| consumed += 1; | ||
| settings.Add((parts[0], parts[1])); | ||
| } | ||
| else | ||
| { | ||
| //$"Argument '{token.Value}' could not be parsed as a RunSetting. Use a key/value pair separated with an equals character, like 'foo=bar'"; | ||
| ctx.ErrorMessage = String.Format(LocalizableStrings.CouldNotParseRunSetting, token.Value); | ||
| ctx.OnlyTake(consumed); | ||
| return null; | ||
| } | ||
| } | ||
| ctx.OnlyTake(consumed); | ||
| return settings.ToArray(); | ||
| }; | ||
|
|
||
| public static readonly Argument<(string key, string value)[]> InlineRunSettings = new(LocalizableStrings.RunSettings, parse: ParseRunSettings, description: LocalizableStrings.RunSettingsDescription) | ||
| { | ||
| Arity = ArgumentArity.ZeroOrMore | ||
| }; | ||
|
|
||
| public static readonly Option<string> SettingsOption = new ForwardedOption<string>(new string[] { "-s", "--settings" }, LocalizableStrings.CmdSettingsDescription) | ||
| { | ||
| ArgumentHelpName = LocalizableStrings.CmdSettingsFile | ||
|
|
@@ -159,6 +280,8 @@ private static Command ConstructCommand() | |
| command.AddOption(CommonOptions.VerbosityOption); | ||
| command.AddOption(CommonOptions.ArchitectureOption); | ||
| command.AddOption(CommonOptions.OperatingSystemOption); | ||
| command.AddArgument(ForwardedArgs); | ||
| command.AddArgument(InlineRunSettings); | ||
|
|
||
| command.SetHandler(TestCommand.Run); | ||
|
|
||
|
|
||
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need
testto 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 standardUseParseErrorReporting()middleware, but I didn't think that was necessary for your---removal PR at this time. It can come next :)