From 3e32e2c57bcf363fb7d3f759ff09018ed692d568 Mon Sep 17 00:00:00 2001 From: William Li Date: Thu, 9 Apr 2020 22:47:43 -0700 Subject: [PATCH 1/6] Remove unnecessary test This test is testing CSC behavior. It does not test SDK code. The behaivor is also changed due to https://github.com/dotnet/roslyn/pull/42769 --- .../GivenThereAreDefaultItems.cs | 40 ------------------- 1 file changed, 40 deletions(-) diff --git a/src/Tests/Microsoft.NET.Build.Tests/GivenThereAreDefaultItems.cs b/src/Tests/Microsoft.NET.Build.Tests/GivenThereAreDefaultItems.cs index 7b073664a77c..bc6a290ba5a7 100644 --- a/src/Tests/Microsoft.NET.Build.Tests/GivenThereAreDefaultItems.cs +++ b/src/Tests/Microsoft.NET.Build.Tests/GivenThereAreDefaultItems.cs @@ -527,46 +527,6 @@ public void It_gives_an_error_message_if_duplicate_compile_items_are_included() .And.HaveStdOutContaining("EnableDefaultCompileItems"); } - [Fact] - public void It_gives_the_correct_error_if_duplicate_compile_items_are_included_and_default_items_are_disabled() - { - var testProject = new TestProject() - { - Name = "DuplicateCompileItems", - TargetFrameworks = "netstandard1.6", - IsSdkProject = true - }; - - var testAsset = _testAssetsManager.CreateTestProject(testProject, "DuplicateCompileItemsWithDefaultItemsDisabled") - .WithProjectChanges(project => - { - var ns = project.Root.Name.Namespace; - - project.Root.Element(ns + "PropertyGroup").Add( - new XElement(ns + "EnableDefaultCompileItems", "false")); - - var itemGroup = new XElement(ns + "ItemGroup"); - project.Root.Add(itemGroup); - itemGroup.Add(new XElement(ns + "Compile", new XAttribute("Include", @"**\*.cs"))); - itemGroup.Add(new XElement(ns + "Compile", new XAttribute("Include", @"DuplicateCompileItems.cs"))); - }); - - var buildCommand = new BuildCommand(Log, Path.Combine(testAsset.TestRoot, testProject.Name)); - - WriteFile(Path.Combine(buildCommand.ProjectRootPath, "Class1.cs"), "public class Class1 {}"); - - buildCommand - .Execute() - .Should() - .Fail() - .And.HaveStdOutContaining("DuplicateCompileItems.cs") - // Class1.cs wasn't included multiple times, so it shouldn't be mentioned - .And.NotHaveStdOutMatching("Class1.cs") - // Default items weren't enabled, so the error message should come from the C# compiler and shouldn't include the information about default compile items - .And.HaveStdOutContaining("MSB3105") - .And.NotHaveStdOutMatching("EnableDefaultCompileItems"); - } - [Fact] public void Implicit_package_references_are_overridden_by_PackageReference_includes_in_the_project_file() { From 3572a114569aa4b5b766057a5148b3b919228426 Mon Sep 17 00:00:00 2001 From: nohwnd Date: Wed, 29 Jul 2020 15:01:26 +0200 Subject: [PATCH 2/6] Add --blame options to dotnet vstest and dotnet test + dll --- .../dotnet-test/VSTestArgumentConverter.cs | 207 +++++++++++++++++- .../VSTestArgumentConverterTests.cs | 47 ++-- 2 files changed, 233 insertions(+), 21 deletions(-) diff --git a/src/Cli/dotnet/commands/dotnet-test/VSTestArgumentConverter.cs b/src/Cli/dotnet/commands/dotnet-test/VSTestArgumentConverter.cs index 0c228199141a..4bc7b94ddc23 100644 --- a/src/Cli/dotnet/commands/dotnet-test/VSTestArgumentConverter.cs +++ b/src/Cli/dotnet/commands/dotnet-test/VSTestArgumentConverter.cs @@ -50,6 +50,15 @@ public class VSTestArgumentConverter "--interactive" }; + private readonly string _blame = "--blame"; + private readonly string _blameCrash = "--blame-crash"; + private readonly string _blameCrashDumpType = "--blame-crash-dump-type"; + private readonly string _blameCrashCollectAlways = "--blame-crash-collect-always"; + private readonly string _blameHang = "--blame-hang"; + private readonly string _blameHangDumpType = "--blame-hang-dump-type"; + private readonly string _blameHangTimeout = "--blame-hang-timeout"; + + /// /// Converts the given arguments to vstest parsable arguments /// @@ -62,6 +71,17 @@ public List Convert(string[] args, out List ignoredArgs) ignoredArgs = new List(); string activeArgument = null; + + bool blame = false; + + bool collectCrashDump = false; + string collectCrashDumpType = null; + bool collectCrashDumpAlways = false; + + bool collectHangDump = false; + string collectHangDumpType = null; + string collectHangDumpTimeout = null; + foreach (var arg in args) { if (arg == "--") @@ -101,6 +121,53 @@ public List Convert(string[] args, out List ignoredArgs) continue; } + if (Eq(argValues[0], _blame)) + { + blame = true; + } + + // Any blame-crash param implies that we collect crash dump + if (Eq(argValues[0], _blameCrash)) + { + blame = true; + collectCrashDump = true; + } + + if (Eq(argValues[0], _blameCrashCollectAlways)) + { + blame = true; + collectCrashDump = true; + collectCrashDumpAlways = true; + } + + if (Eq(argValues[0], _blameCrashDumpType)) + { + blame = true; + collectCrashDump = true; + collectCrashDumpType = argValues[1]; + } + + // Any blame-hang param implies that we collect hang dump + if (Eq(argValues[0], _blameHang)) + { + blame = true; + collectHangDump = true; + } + + if (Eq(argValues[0], _blameHangDumpType)) + { + blame = true; + collectHangDump = true; + collectHangDumpType = argValues[1]; + } + + if (Eq(argValues[0], _blameHangTimeout)) + { + blame = true; + collectHangDump = true; + collectHangDumpTimeout = argValues[1]; + } + // Check if the argument is shortname if (ArgumentMapping.TryGetValue(argValues[0].ToLower(), out var longName)) { @@ -111,10 +178,33 @@ public List Convert(string[] args, out List ignoredArgs) } else { - activeArgument = arg.ToLower(); - if (ArgumentMapping.TryGetValue(activeArgument, out var value)) + if (Eq(arg, _blame)) + { + blame = true; + } + else if (Eq(arg, _blameCrash)) + { + blame = true; + collectCrashDump = true; + } + else if (Eq(arg, _blameCrashCollectAlways)) + { + blame = true; + collectCrashDump = true; + collectCrashDumpAlways = true; + } + else if (Eq(arg, _blameHang)) + { + blame = true; + collectHangDump = true; + } + else { - activeArgument = value; + activeArgument = arg.ToLower(); + if (ArgumentMapping.TryGetValue(activeArgument, out var value)) + { + activeArgument = value; + } } } } @@ -129,6 +219,44 @@ public List Convert(string[] args, out List ignoredArgs) ignoredArgs.Add(activeArgument); ignoredArgs.Add(arg); } + else if (Eq(activeArgument, _blame)) + { + blame = true; + } + else if (Eq(activeArgument, _blameCrash)) + { + blame = true; + collectCrashDump = true; + } + else if (Eq(activeArgument, _blameCrashCollectAlways)) + { + blame = true; + collectCrashDump = true; + collectCrashDumpAlways = true; + } + else if (Eq(activeArgument, _blameCrashDumpType)) + { + blame = true; + collectCrashDump = true; + collectCrashDumpType = arg; + } + else if (Eq(activeArgument, _blameHang)) + { + blame = true; + collectHangDump = true; + } + else if (Eq(activeArgument, _blameHangDumpType)) + { + blame = true; + collectHangDump = true; + collectHangDumpType = arg; + } + else if (Eq(activeArgument, _blameHangTimeout)) + { + blame = true; + collectHangDump = true; + collectHangDumpTimeout = arg; + } else { newArgList.Add(string.Concat(activeArgument, ":", arg)); @@ -138,7 +266,30 @@ public List Convert(string[] args, out List ignoredArgs) } else { - newArgList.Add(arg); + if (Eq(arg, _blame)) + { + blame = true; + } + else if (Eq(arg, _blameCrash)) + { + blame = true; + collectCrashDump = true; + } + else if (Eq(arg, _blameCrashCollectAlways)) + { + blame = true; + collectCrashDump = true; + collectCrashDumpAlways = true; + } + else if (Eq(arg, _blameHang)) + { + blame = true; + collectHangDump = true; + } + else + { + newArgList.Add(arg); + } } } @@ -154,6 +305,49 @@ public List Convert(string[] args, out List ignoredArgs) } } + if (blame) + { + string crashDumpArgs = null; + string hangDumpArgs = null; + + if (collectCrashDump) + { + crashDumpArgs = "CollectDump"; + if (collectCrashDumpAlways) + { + crashDumpArgs += ";CollectAlways=true"; + } + + if (!string.IsNullOrWhiteSpace(collectCrashDumpType)) + { + crashDumpArgs += $";DumpType={collectCrashDumpType}"; + } + } + + if (collectHangDump) + { + hangDumpArgs = "CollectHangDump"; + if (!string.IsNullOrWhiteSpace(collectHangDumpType)) + { + hangDumpArgs += $";DumpType={collectHangDumpType}"; + } + + if (!string.IsNullOrWhiteSpace(collectHangDumpTimeout)) + { + hangDumpArgs += $";TestTimeout={collectHangDumpTimeout}"; + } + } + + if (collectCrashDump || collectHangDump) + { + newArgList.Add($@"--blame:""{string.Join(";", crashDumpArgs, hangDumpArgs).Trim(';')}"""); + } + else + { + newArgList.Add("--blame"); + } + } + return newArgList; } @@ -171,5 +365,10 @@ private void UpdateVerbosity(string verbosity, List newArgList) } newArgList.Add(verbosityString + verbosity); } + + private bool Eq(string left, string right) + { + return string.Equals(left, right, System.StringComparison.OrdinalIgnoreCase); + } } } diff --git a/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs b/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs index a140b1150511..2e2821885363 100644 --- a/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs +++ b/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs @@ -21,7 +21,7 @@ public void ConvertArgsShouldConvertValidArgsIntoVSTestParsableArgs(string input // Act var convertedArgs = new VSTestArgumentConverter().Convert(args, out var ignoredArgs); - convertedArgs.Should().BeEquivalentTo(convertedArgs); + convertedArgs.Should().BeEquivalentTo(expectedArgs); ignoredArgs.Should().BeEmpty(); } @@ -35,7 +35,7 @@ public void ConvertArgshouldConvertsVerbosityArgsIntoVSTestParsableArgs(string i // Act var convertedArgs = new VSTestArgumentConverter().Convert(args, out var ignoredArgs); - convertedArgs.Should().BeEquivalentTo(convertedArgs); + convertedArgs.Should().BeEquivalentTo(expectedArgs); ignoredArgs.Should().BeEmpty(); } @@ -50,7 +50,7 @@ public void ConvertArgsShouldIgnoreKnownArgsWhileConvertingArgsIntoVSTestParsabl // Act var convertedArgs = new VSTestArgumentConverter().Convert(args, out var ignoredArgs); - convertedArgs.Should().BeEquivalentTo(convertedArgs); + convertedArgs.Should().BeEquivalentTo(expectedArgs); Assert.Equal(expIgnoredArgs, ignoredArgs); } @@ -69,20 +69,33 @@ public static class DataSource { public static IEnumerable ArgTestCases { get; } = new List { - new object[] { "-h", "--help" }, - new object[] { "sometest.dll -s test.settings", "sometest.dll --settings:test.settings" }, - new object[] { "sometest.dll -t", "sometest.dll --listtests" }, - new object[] { "sometest.dll --list-tests", "sometest.dll --listtests" }, - new object[] { "sometest.dll --filter", "sometest.dll --testcasefilter" }, - new object[] { "sometest.dll -l trx", "sometest.dll --logger:trx" }, - new object[] { "sometest.dll -a c:\adapterpath\temp", "sometest.dll --testadapterpath:c:\adapterpath\temp" }, - new object[] { "sometest.dll --test-adapter-path c:\adapterpath\temp", "sometest.dll --testadapterpath:c:\adapterpath\temp" }, - new object[] { "sometest.dll -f net451", "sometest.dll --framework:net451" }, - new object[] { @"sometest.dll -d c:\temp\log.txt", @"sometest.dll --diag:c:\temp\log.txt" }, - new object[] { @"sometest.dll --results-directory c:\temp\", @"sometest.dll --resultsdirectory:c:\temp\" }, - new object[] { @"sometest.dll -s testsettings -t -a c:\path -f net451 -d log.txt --results-directory c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" }, - new object[] { @"sometest.dll -s:testsettings -t -a:c:\path -f:net451 -d:log.txt --results-directory:c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" }, - new object[] { @"sometest.dll --settings testsettings -t --test-adapter-path c:\path --framework net451 --diag log.txt --results-directory c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" } + //new object[] { "-h", "--help" }, + //new object[] { "sometest.dll -s test.settings", "sometest.dll --settings:test.settings" }, + //new object[] { "sometest.dll -t", "sometest.dll --listtests" }, + //new object[] { "sometest.dll --list-tests", "sometest.dll --listtests" }, + //new object[] { "sometest.dll --filter", "sometest.dll --testcasefilter" }, + //new object[] { "sometest.dll -l trx", "sometest.dll --logger:trx" }, + //new object[] { "sometest.dll -a c:\adapterpath\temp", "sometest.dll --testadapterpath:c:\adapterpath\temp" }, + //new object[] { "sometest.dll --test-adapter-path c:\adapterpath\temp", "sometest.dll --testadapterpath:c:\adapterpath\temp" }, + //new object[] { "sometest.dll -f net451", "sometest.dll --framework:net451" }, + //new object[] { @"sometest.dll -d c:\temp\log.txt", @"sometest.dll --diag:c:\temp\log.txt" }, + //new object[] { @"sometest.dll --results-directory c:\temp\", @"sometest.dll --resultsdirectory:c:\temp\" }, + //new object[] { @"sometest.dll -s testsettings -t -a c:\path -f net451 -d log.txt --results-directory c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" }, + //new object[] { @"sometest.dll -s:testsettings -t -a:c:\path -f:net451 -d:log.txt --results-directory:c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" }, + //new object[] { @"sometest.dll --settings testsettings -t --test-adapter-path c:\path --framework net451 --diag log.txt --results-directory c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" } + new object[] { @"sometest.dll --blame", @"sometest.dll --blame" }, + new object[] { @"sometest.dll --blame-crash", @"sometest.dll --blame:""CollectDump""" }, + new object[] { @"sometest.dll --blame-crash-dump-type full", @"sometest.dll --blame:""CollectDump;DumpType=full""" }, + new object[] { @"sometest.dll --blame-crash-collect-always", @"sometest.dll --blame:""CollectDump;CollectAlways=true""" }, + new object[] { @"sometest.dll --blame --blame-crash-dump-type full --blame-crash-collect-always", @"sometest.dll --blame:""CollectDump;CollectAlways=true;DumpType=full""" }, + new object[] { @"sometest.dll --blame-hang", @"sometest.dll --blame:""CollectHangDump""" }, + new object[] { @"sometest.dll --blame-hang-dump-type full", @"sometest.dll --blame:""CollectHangDump;DumpType=full""" }, + new object[] { @"sometest.dll --blame-hang-timeout 10min", @"sometest.dll --blame:""CollectHangDump;TestTimeout=10min""" }, + new object[] { @"sometest.dll --blame --blame-hang-dump-type full --blame-hang-timeout 10min", @"sometest.dll --blame:""CollectHangDump;DumpType=full;TestTimeout=10min""" }, + new object[] { @"sometest.dll --blame --blame-hang-dump-type full --blame-hang-timeout 10min --blame-crash-dump-type mini --blame-crash-collect-always", @"sometest.dll --blame:""CollectDump;CollectAlways=true;DumpType=mini;CollectHangDump;DumpType=full;TestTimeout=10min""" }, + + + }; public static IEnumerable VerbosityTestCases { get; } = new List From 505a2a0825a1609ed8aa52b379a544472f6aa279 Mon Sep 17 00:00:00 2001 From: nohwnd Date: Wed, 29 Jul 2020 15:04:07 +0200 Subject: [PATCH 3/6] Uncomment other test cases --- .../VSTestArgumentConverterTests.cs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs b/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs index 2e2821885363..0d683bbc0d70 100644 --- a/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs +++ b/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs @@ -69,20 +69,20 @@ public static class DataSource { public static IEnumerable ArgTestCases { get; } = new List { - //new object[] { "-h", "--help" }, - //new object[] { "sometest.dll -s test.settings", "sometest.dll --settings:test.settings" }, - //new object[] { "sometest.dll -t", "sometest.dll --listtests" }, - //new object[] { "sometest.dll --list-tests", "sometest.dll --listtests" }, - //new object[] { "sometest.dll --filter", "sometest.dll --testcasefilter" }, - //new object[] { "sometest.dll -l trx", "sometest.dll --logger:trx" }, - //new object[] { "sometest.dll -a c:\adapterpath\temp", "sometest.dll --testadapterpath:c:\adapterpath\temp" }, - //new object[] { "sometest.dll --test-adapter-path c:\adapterpath\temp", "sometest.dll --testadapterpath:c:\adapterpath\temp" }, - //new object[] { "sometest.dll -f net451", "sometest.dll --framework:net451" }, - //new object[] { @"sometest.dll -d c:\temp\log.txt", @"sometest.dll --diag:c:\temp\log.txt" }, - //new object[] { @"sometest.dll --results-directory c:\temp\", @"sometest.dll --resultsdirectory:c:\temp\" }, - //new object[] { @"sometest.dll -s testsettings -t -a c:\path -f net451 -d log.txt --results-directory c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" }, - //new object[] { @"sometest.dll -s:testsettings -t -a:c:\path -f:net451 -d:log.txt --results-directory:c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" }, - //new object[] { @"sometest.dll --settings testsettings -t --test-adapter-path c:\path --framework net451 --diag log.txt --results-directory c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" } + new object[] { "-h", "--help" }, + new object[] { "sometest.dll -s test.settings", "sometest.dll --settings:test.settings" }, + new object[] { "sometest.dll -t", "sometest.dll --listtests" }, + new object[] { "sometest.dll --list-tests", "sometest.dll --listtests" }, + new object[] { "sometest.dll --filter", "sometest.dll --testcasefilter" }, + new object[] { "sometest.dll -l trx", "sometest.dll --logger:trx" }, + new object[] { "sometest.dll -a c:\adapterpath\temp", "sometest.dll --testadapterpath:c:\adapterpath\temp" }, + new object[] { "sometest.dll --test-adapter-path c:\adapterpath\temp", "sometest.dll --testadapterpath:c:\adapterpath\temp" }, + new object[] { "sometest.dll -f net451", "sometest.dll --framework:net451" }, + new object[] { @"sometest.dll -d c:\temp\log.txt", @"sometest.dll --diag:c:\temp\log.txt" }, + new object[] { @"sometest.dll --results-directory c:\temp\", @"sometest.dll --resultsdirectory:c:\temp\" }, + new object[] { @"sometest.dll -s testsettings -t -a c:\path -f net451 -d log.txt --results-directory c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" }, + new object[] { @"sometest.dll -s:testsettings -t -a:c:\path -f:net451 -d:log.txt --results-directory:c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" }, + new object[] { @"sometest.dll --settings testsettings -t --test-adapter-path c:\path --framework net451 --diag log.txt --results-directory c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" } new object[] { @"sometest.dll --blame", @"sometest.dll --blame" }, new object[] { @"sometest.dll --blame-crash", @"sometest.dll --blame:""CollectDump""" }, new object[] { @"sometest.dll --blame-crash-dump-type full", @"sometest.dll --blame:""CollectDump;DumpType=full""" }, From 7bf979fdb1cf026cdd0e9ff86c8543bf04359740 Mon Sep 17 00:00:00 2001 From: nohwnd Date: Wed, 29 Jul 2020 15:10:08 +0200 Subject: [PATCH 4/6] Fix comma --- .../VSTestArgumentConverterTests.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs b/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs index 0d683bbc0d70..5b138c68fd7d 100644 --- a/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs +++ b/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs @@ -69,20 +69,20 @@ public static class DataSource { public static IEnumerable ArgTestCases { get; } = new List { - new object[] { "-h", "--help" }, - new object[] { "sometest.dll -s test.settings", "sometest.dll --settings:test.settings" }, - new object[] { "sometest.dll -t", "sometest.dll --listtests" }, - new object[] { "sometest.dll --list-tests", "sometest.dll --listtests" }, - new object[] { "sometest.dll --filter", "sometest.dll --testcasefilter" }, - new object[] { "sometest.dll -l trx", "sometest.dll --logger:trx" }, - new object[] { "sometest.dll -a c:\adapterpath\temp", "sometest.dll --testadapterpath:c:\adapterpath\temp" }, - new object[] { "sometest.dll --test-adapter-path c:\adapterpath\temp", "sometest.dll --testadapterpath:c:\adapterpath\temp" }, - new object[] { "sometest.dll -f net451", "sometest.dll --framework:net451" }, + new object[] { @"-h", "--help" }, + new object[] { @"sometest.dll -s test.settings", @"sometest.dll --settings:test.settings" }, + new object[] { @"sometest.dll -t", @"sometest.dll --listtests" }, + new object[] { @"sometest.dll --list-tests", @"sometest.dll --listtests" }, + new object[] { @"sometest.dll --filter", @"sometest.dll --testcasefilter" }, + new object[] { @"sometest.dll -l trx", @"sometest.dll --logger:trx" }, + new object[] { @"sometest.dll -a c:\adapterpath\temp", @"sometest.dll --testadapterpath:c:\adapterpath\temp" }, + new object[] { @"sometest.dll --test-adapter-path c:\adapterpath\temp", @"sometest.dll --testadapterpath:c:\adapterpath\temp" }, + new object[] { @"sometest.dll -f net451", @"sometest.dll --framework:net451" }, new object[] { @"sometest.dll -d c:\temp\log.txt", @"sometest.dll --diag:c:\temp\log.txt" }, new object[] { @"sometest.dll --results-directory c:\temp\", @"sometest.dll --resultsdirectory:c:\temp\" }, new object[] { @"sometest.dll -s testsettings -t -a c:\path -f net451 -d log.txt --results-directory c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" }, new object[] { @"sometest.dll -s:testsettings -t -a:c:\path -f:net451 -d:log.txt --results-directory:c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" }, - new object[] { @"sometest.dll --settings testsettings -t --test-adapter-path c:\path --framework net451 --diag log.txt --results-directory c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" } + new object[] { @"sometest.dll --settings testsettings -t --test-adapter-path c:\path --framework net451 --diag log.txt --results-directory c:\temp\", @"sometest.dll --settings:testsettings --listtests --testadapterpath:c:\path --framework:net451 --diag:log.txt --resultsdirectory:c:\temp\" }, new object[] { @"sometest.dll --blame", @"sometest.dll --blame" }, new object[] { @"sometest.dll --blame-crash", @"sometest.dll --blame:""CollectDump""" }, new object[] { @"sometest.dll --blame-crash-dump-type full", @"sometest.dll --blame:""CollectDump;DumpType=full""" }, From 5371ccd094c4a86507b0b1f9bda89eb37220989c Mon Sep 17 00:00:00 2001 From: nohwnd Date: Wed, 29 Jul 2020 15:57:37 +0200 Subject: [PATCH 5/6] Refactor to recude code repetition --- .../dotnet-test/VSTestArgumentConverter.cs | 337 +++++++++--------- 1 file changed, 161 insertions(+), 176 deletions(-) diff --git a/src/Cli/dotnet/commands/dotnet-test/VSTestArgumentConverter.cs b/src/Cli/dotnet/commands/dotnet-test/VSTestArgumentConverter.cs index 4bc7b94ddc23..1211e6b06f91 100644 --- a/src/Cli/dotnet/commands/dotnet-test/VSTestArgumentConverter.cs +++ b/src/Cli/dotnet/commands/dotnet-test/VSTestArgumentConverter.cs @@ -6,6 +6,7 @@ namespace Microsoft.DotNet.Cli using System; using System.Collections.Generic; using System.Linq; + using static BlameArgs; /// /// Converts the given arguments to vstest parsable arguments @@ -50,15 +51,6 @@ public class VSTestArgumentConverter "--interactive" }; - private readonly string _blame = "--blame"; - private readonly string _blameCrash = "--blame-crash"; - private readonly string _blameCrashDumpType = "--blame-crash-dump-type"; - private readonly string _blameCrashCollectAlways = "--blame-crash-collect-always"; - private readonly string _blameHang = "--blame-hang"; - private readonly string _blameHangDumpType = "--blame-hang-dump-type"; - private readonly string _blameHangTimeout = "--blame-hang-timeout"; - - /// /// Converts the given arguments to vstest parsable arguments /// @@ -71,16 +63,7 @@ public List Convert(string[] args, out List ignoredArgs) ignoredArgs = new List(); string activeArgument = null; - - bool blame = false; - - bool collectCrashDump = false; - string collectCrashDumpType = null; - bool collectCrashDumpAlways = false; - - bool collectHangDump = false; - string collectHangDumpType = null; - string collectHangDumpTimeout = null; + BlameArgs blame = new BlameArgs(); foreach (var arg in args) { @@ -121,51 +104,10 @@ public List Convert(string[] args, out List ignoredArgs) continue; } - if (Eq(argValues[0], _blame)) - { - blame = true; - } - - // Any blame-crash param implies that we collect crash dump - if (Eq(argValues[0], _blameCrash)) - { - blame = true; - collectCrashDump = true; - } - - if (Eq(argValues[0], _blameCrashCollectAlways)) - { - blame = true; - collectCrashDump = true; - collectCrashDumpAlways = true; - } - - if (Eq(argValues[0], _blameCrashDumpType)) - { - blame = true; - collectCrashDump = true; - collectCrashDumpType = argValues[1]; - } - - // Any blame-hang param implies that we collect hang dump - if (Eq(argValues[0], _blameHang)) + if (blame.IsBlameArg(argValues[0])) { - blame = true; - collectHangDump = true; - } - - if (Eq(argValues[0], _blameHangDumpType)) - { - blame = true; - collectHangDump = true; - collectHangDumpType = argValues[1]; - } - - if (Eq(argValues[0], _blameHangTimeout)) - { - blame = true; - collectHangDump = true; - collectHangDumpTimeout = argValues[1]; + blame.UpdateBlame(argValues[0], argValues[1]); + continue; } // Check if the argument is shortname @@ -178,25 +120,9 @@ public List Convert(string[] args, out List ignoredArgs) } else { - if (Eq(arg, _blame)) - { - blame = true; - } - else if (Eq(arg, _blameCrash)) + if (blame.IsBlameSwitch(arg)) { - blame = true; - collectCrashDump = true; - } - else if (Eq(arg, _blameCrashCollectAlways)) - { - blame = true; - collectCrashDump = true; - collectCrashDumpAlways = true; - } - else if (Eq(arg, _blameHang)) - { - blame = true; - collectHangDump = true; + blame.UpdateBlame(arg, null); } else { @@ -219,43 +145,9 @@ public List Convert(string[] args, out List ignoredArgs) ignoredArgs.Add(activeArgument); ignoredArgs.Add(arg); } - else if (Eq(activeArgument, _blame)) - { - blame = true; - } - else if (Eq(activeArgument, _blameCrash)) - { - blame = true; - collectCrashDump = true; - } - else if (Eq(activeArgument, _blameCrashCollectAlways)) + else if (blame.IsBlameArg(activeArgument)) { - blame = true; - collectCrashDump = true; - collectCrashDumpAlways = true; - } - else if (Eq(activeArgument, _blameCrashDumpType)) - { - blame = true; - collectCrashDump = true; - collectCrashDumpType = arg; - } - else if (Eq(activeArgument, _blameHang)) - { - blame = true; - collectHangDump = true; - } - else if (Eq(activeArgument, _blameHangDumpType)) - { - blame = true; - collectHangDump = true; - collectHangDumpType = arg; - } - else if (Eq(activeArgument, _blameHangTimeout)) - { - blame = true; - collectHangDump = true; - collectHangDumpTimeout = arg; + blame.UpdateBlame(activeArgument, arg); } else { @@ -266,25 +158,9 @@ public List Convert(string[] args, out List ignoredArgs) } else { - if (Eq(arg, _blame)) + if (blame.IsBlameArg(arg)) { - blame = true; - } - else if (Eq(arg, _blameCrash)) - { - blame = true; - collectCrashDump = true; - } - else if (Eq(arg, _blameCrashCollectAlways)) - { - blame = true; - collectCrashDump = true; - collectCrashDumpAlways = true; - } - else if (Eq(arg, _blameHang)) - { - blame = true; - collectHangDump = true; + blame.UpdateBlame(arg, null); } else { @@ -305,47 +181,9 @@ public List Convert(string[] args, out List ignoredArgs) } } - if (blame) + if (blame.Blame) { - string crashDumpArgs = null; - string hangDumpArgs = null; - - if (collectCrashDump) - { - crashDumpArgs = "CollectDump"; - if (collectCrashDumpAlways) - { - crashDumpArgs += ";CollectAlways=true"; - } - - if (!string.IsNullOrWhiteSpace(collectCrashDumpType)) - { - crashDumpArgs += $";DumpType={collectCrashDumpType}"; - } - } - - if (collectHangDump) - { - hangDumpArgs = "CollectHangDump"; - if (!string.IsNullOrWhiteSpace(collectHangDumpType)) - { - hangDumpArgs += $";DumpType={collectHangDumpType}"; - } - - if (!string.IsNullOrWhiteSpace(collectHangDumpTimeout)) - { - hangDumpArgs += $";TestTimeout={collectHangDumpTimeout}"; - } - } - - if (collectCrashDump || collectHangDump) - { - newArgList.Add($@"--blame:""{string.Join(";", crashDumpArgs, hangDumpArgs).Trim(';')}"""); - } - else - { - newArgList.Add("--blame"); - } + blame.AddCombinedBlameArgs(newArgList); } return newArgList; @@ -365,10 +203,157 @@ private void UpdateVerbosity(string verbosity, List newArgList) } newArgList.Add(verbosityString + verbosity); } + } + + class BlameArgs + { + public bool Blame = false; + + public bool CollectCrashDump = false; + public string CollectCrashDumpType = null; + public bool CollectCrashDumpAlways = false; + + public bool CollectHangDump = false; + public string CollectHangDumpType = null; + public string CollectHangDumpTimeout = null; + + public static string BlameParam = "--blame"; + public static string BlameCrashParam = "--blame-crash"; + public static string BlameCrashDumpTypeParam = "--blame-crash-dump-type"; + public static string BlameCrashCollectAlwaysParam = "--blame-crash-collect-always"; + public static string BlameHangParam = "--blame-hang"; + public static string BlameHangDumpTypeParam = "--blame-hang-dump-type"; + public static string BlameHangTimeoutParam = "--blame-hang-timeout"; + + // parameters that expect arguments + private readonly string[] _blameArgList = new string[]{ + BlameCrashDumpTypeParam, + + BlameHangDumpTypeParam, + BlameHangTimeoutParam + }; + + // parameters that don't expect any arguments + private readonly string[] _blameSwitchList = new string[]{ + BlameParam, + + BlameCrashParam, + BlameCrashCollectAlwaysParam, + + BlameHangParam, + }; + + + internal bool IsBlameArg(string parameter) + { + return _blameArgList.Contains(parameter) || _blameSwitchList.Contains(parameter); + } + + internal bool IsBlameSwitch(string parameter) + { + return _blameSwitchList.Contains(parameter); + + } + + internal void UpdateBlame(string parameter, string argument) + { + if (Eq(parameter, BlameParam)) + { + Blame = true; + } + + // Any blame-crash param implies that we collect crash dump + if (Eq(parameter, BlameCrashParam)) + { + Blame = true; + CollectCrashDump = true; + } + + if (Eq(parameter, BlameCrashCollectAlwaysParam)) + { + Blame = true; + CollectCrashDump = true; + CollectCrashDumpAlways = true; + } + + if (Eq(parameter, BlameCrashDumpTypeParam)) + { + Blame = true; + CollectCrashDump = true; + CollectCrashDumpType = argument; + } + + // Any Blame-hang param implies that we collect hang dump + if (Eq(parameter, BlameHangParam)) + { + Blame = true; + CollectHangDump = true; + } + + if (Eq(parameter, BlameHangDumpTypeParam)) + { + Blame = true; + CollectHangDump = true; + CollectHangDumpType = argument; + } + + if (Eq(parameter, BlameHangTimeoutParam)) + { + Blame = true; + CollectHangDump = true; + CollectHangDumpTimeout = argument; + } + } private bool Eq(string left, string right) { - return string.Equals(left, right, System.StringComparison.OrdinalIgnoreCase); + return string.Equals(left, right, StringComparison.OrdinalIgnoreCase); + } + + internal void AddCombinedBlameArgs(List newArgList) + { + if (!Blame) + return; + + string crashDumpArgs = null; + string hangDumpArgs = null; + + if (CollectCrashDump) + { + crashDumpArgs = "CollectDump"; + if (CollectCrashDumpAlways) + { + crashDumpArgs += ";CollectAlways=true"; + } + + if (!string.IsNullOrWhiteSpace(CollectCrashDumpType)) + { + crashDumpArgs += $";DumpType={CollectCrashDumpType}"; + } + } + + if (CollectHangDump) + { + hangDumpArgs = "CollectHangDump"; + if (!string.IsNullOrWhiteSpace(CollectHangDumpType)) + { + hangDumpArgs += $";DumpType={CollectHangDumpType}"; + } + + if (!string.IsNullOrWhiteSpace(CollectHangDumpTimeout)) + { + hangDumpArgs += $";TestTimeout={CollectHangDumpTimeout}"; + } + } + + if (CollectCrashDump || CollectHangDump) + { + newArgList.Add($@"--blame:""{string.Join(";", crashDumpArgs, hangDumpArgs).Trim(';')}"""); + } + else + { + newArgList.Add("--blame"); + } } } } From 8c4b15900891304e4f3d178ef825541a30b182ca Mon Sep 17 00:00:00 2001 From: nohwnd Date: Wed, 29 Jul 2020 16:36:08 +0200 Subject: [PATCH 6/6] Ensure that legacy Blame syntax still works --- .../dotnet-test/VSTestArgumentConverter.cs | 48 ++++++++++++++++--- .../VSTestArgumentConverterTests.cs | 3 ++ 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/Cli/dotnet/commands/dotnet-test/VSTestArgumentConverter.cs b/src/Cli/dotnet/commands/dotnet-test/VSTestArgumentConverter.cs index 1211e6b06f91..b4a67bed9016 100644 --- a/src/Cli/dotnet/commands/dotnet-test/VSTestArgumentConverter.cs +++ b/src/Cli/dotnet/commands/dotnet-test/VSTestArgumentConverter.cs @@ -80,6 +80,10 @@ public List Convert(string[] args, out List ignoredArgs) { ignoredArgs.Add(activeArgument); } + else if (blame.IsBlameArg(activeArgument, null)) + { + // do nothing, we process remaining arguments ourselves + } else { newArgList.Add(activeArgument); @@ -104,7 +108,7 @@ public List Convert(string[] args, out List ignoredArgs) continue; } - if (blame.IsBlameArg(argValues[0])) + if (blame.IsBlameArg(argValues[0], argValues[1])) { blame.UpdateBlame(argValues[0], argValues[1]); continue; @@ -123,6 +127,7 @@ public List Convert(string[] args, out List ignoredArgs) if (blame.IsBlameSwitch(arg)) { blame.UpdateBlame(arg, null); + activeArgument = arg; } else { @@ -145,7 +150,7 @@ public List Convert(string[] args, out List ignoredArgs) ignoredArgs.Add(activeArgument); ignoredArgs.Add(arg); } - else if (blame.IsBlameArg(activeArgument)) + else if (blame.IsBlameArg(activeArgument, arg)) { blame.UpdateBlame(activeArgument, arg); } @@ -158,7 +163,7 @@ public List Convert(string[] args, out List ignoredArgs) } else { - if (blame.IsBlameArg(arg)) + if (blame.IsBlameArg(arg, null)) { blame.UpdateBlame(arg, null); } @@ -175,6 +180,9 @@ public List Convert(string[] args, out List ignoredArgs) { ignoredArgs.Add(activeArgument); } + else if( blame.IsBlameArg(activeArgument, null)) { + // do nothing, we process remaining arguments ourselves + } else { newArgList.Add(activeArgument); @@ -208,6 +216,7 @@ private void UpdateVerbosity(string verbosity, List newArgList) class BlameArgs { public bool Blame = false; + public string LegacyBlame = null; public bool CollectCrashDump = false; public string CollectCrashDumpType = null; @@ -244,19 +253,38 @@ class BlameArgs }; - internal bool IsBlameArg(string parameter) + internal bool IsBlameArg(string parameter, string value) + { + return _blameArgList.Any(p => Eq(p, parameter)) || _blameSwitchList.Any(p => Eq(p, parameter)); + } + + private bool IsLegacyBlame(string parameter, string value) + { + // when provided --blame , we do not want to process it any further + // most likely a legacy call, and the param is already in the format that vstest.console expects + return Eq(BlameParam, parameter) && !string.IsNullOrWhiteSpace(value); + } + + internal bool IsBlame(string parameter) { - return _blameArgList.Contains(parameter) || _blameSwitchList.Contains(parameter); + return Eq(BlameParam, parameter); + } internal bool IsBlameSwitch(string parameter) { - return _blameSwitchList.Contains(parameter); + return _blameSwitchList.Any(p => Eq(p, parameter)); } internal void UpdateBlame(string parameter, string argument) { + if (IsLegacyBlame(parameter, argument)) + { + Blame = true; + LegacyBlame = argument; + } + if (Eq(parameter, BlameParam)) { Blame = true; @@ -315,6 +343,14 @@ internal void AddCombinedBlameArgs(List newArgList) if (!Blame) return; + if (!string.IsNullOrWhiteSpace(LegacyBlame)) + { + // when legacy call is detected don't process + // any more parameters + newArgList.Add($"--blame:{LegacyBlame}"); + return; + } + string crashDumpArgs = null; string hangDumpArgs = null; diff --git a/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs b/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs index 5b138c68fd7d..35a9a3c41f3b 100644 --- a/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs +++ b/src/Tests/dotnet.Tests/ParserTests/VSTestArgumentConverterTests.cs @@ -93,6 +93,9 @@ public static class DataSource new object[] { @"sometest.dll --blame-hang-timeout 10min", @"sometest.dll --blame:""CollectHangDump;TestTimeout=10min""" }, new object[] { @"sometest.dll --blame --blame-hang-dump-type full --blame-hang-timeout 10min", @"sometest.dll --blame:""CollectHangDump;DumpType=full;TestTimeout=10min""" }, new object[] { @"sometest.dll --blame --blame-hang-dump-type full --blame-hang-timeout 10min --blame-crash-dump-type mini --blame-crash-collect-always", @"sometest.dll --blame:""CollectDump;CollectAlways=true;DumpType=mini;CollectHangDump;DumpType=full;TestTimeout=10min""" }, + // using the legacy --blame syntax when we provide the parameter that are already in vstest.console format still works + new object[] { @"sometest.dll --blame ""CollectDump;DumpType=full""", @"sometest.dll --blame:""CollectDump;DumpType=full""" }, + new object[] { @"sometest.dll --blame:""CollectDump;DumpType=full""", @"sometest.dll --blame:""CollectDump;DumpType=full""" },