From 194712d1fb281f033391beb4f4e49d0f0d5bf379 Mon Sep 17 00:00:00 2001 From: KonH Date: Tue, 7 Jul 2020 21:05:50 +0700 Subject: [PATCH 1/6] Test cases covers tuple issue --- .../NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs b/test/NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs index 7308dc7..09ed8e8 100644 --- a/test/NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs +++ b/test/NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs @@ -30,6 +30,10 @@ public class TestCaseNameParserTests // See nunit.testlogger #66. [DataRow("z.y.x.ape.bar(a\\b)", "z.y.x", "ape", "bar(a\\b)")] + + // Test with tuple arguments + [DataRow("z.a.b((0,1))", "z", "a", "b((0,1))")] + [DataRow("z.a.b((\"arg\",1))", "z", "a", "b((\"arg\",1))")] public void Parse_ParsesAllParsableInputs_WithoutConsoleOutput(string testCaseName, string expectedNamespace, string expectedType, string expectedMethod) { var expected = new Tuple(expectedType, expectedMethod); From da985a1839639a962837708b934c76142d308d8d Mon Sep 17 00:00:00 2001 From: KonH Date: Tue, 7 Jul 2020 21:48:23 +0700 Subject: [PATCH 2/6] Proposed solution to track extra parentheses Instead of treat additional parentheses as an error in place, ensure that they are balanced (e.g., two closed and two open) --- src/NUnit.Xml.TestLogger/TestCaseNameParser.cs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/NUnit.Xml.TestLogger/TestCaseNameParser.cs b/src/NUnit.Xml.TestLogger/TestCaseNameParser.cs index fcfa261..a4989ad 100644 --- a/src/NUnit.Xml.TestLogger/TestCaseNameParser.cs +++ b/src/NUnit.Xml.TestLogger/TestCaseNameParser.cs @@ -56,6 +56,7 @@ public static ParsedName Parse(string fullyQualifedName) var step = NameParseStep.FindMethod; var state = NameParseState.Default; + var parenthesisCount = 0; var output = new List(); @@ -71,7 +72,12 @@ public static ParsedName Parse(string fullyQualifedName) } else if (state == NameParseState.Default) { - if (thisChar == '(' || thisChar == '"' || thisChar == '\\') + if (thisChar == '(') + { + parenthesisCount--; + } + + if (thisChar == '"') { throw new Exception("Found invalid characters"); } @@ -125,7 +131,7 @@ public static ParsedName Parse(string fullyQualifedName) { if (thisChar == ')') { - throw new Exception("Found invalid characters"); + parenthesisCount++; } if (thisChar == '(') @@ -157,6 +163,11 @@ public static ParsedName Parse(string fullyQualifedName) } } + if (parenthesisCount != 0) + { + throw new Exception($"Unbalanced count of parentheses found ({parenthesisCount})"); + } + // We are done. If we are finding type, set that variable. // Otherwise, ther was some issue, so leave the type blank. if (step == NameParseStep.FindNamespace) From 63f2a07455cda7c1c4fee9a262590ee7b612852b Mon Sep 17 00:00:00 2001 From: KonH Date: Thu, 24 Sep 2020 22:04:02 +0700 Subject: [PATCH 3/6] Add more complex tuple test cases Refers to comment here - https://github.com/spekt/nunit.testlogger/pull/65#discussion_r483847094 --- test/NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs b/test/NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs index 09ed8e8..ceada00 100644 --- a/test/NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs +++ b/test/NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs @@ -34,6 +34,8 @@ public class TestCaseNameParserTests // Test with tuple arguments [DataRow("z.a.b((0,1))", "z", "a", "b((0,1))")] [DataRow("z.a.b((\"arg\",1))", "z", "a", "b((\"arg\",1))")] + [DataRow("z.a.b((0,1),(2,3))", "z", "a", "b((0,1),(2,3))")] + [DataRow("z.a.b((0,(0,1)),(0,1))", "z", "a", "b((0,(0,1)),(0,1))")] public void Parse_ParsesAllParsableInputs_WithoutConsoleOutput(string testCaseName, string expectedNamespace, string expectedType, string expectedMethod) { var expected = new Tuple(expectedType, expectedMethod); From 6a5e48304531f69698bb6eb9c396629a5f24a96c Mon Sep 17 00:00:00 2001 From: KonH Date: Fri, 25 Sep 2020 20:56:05 +0700 Subject: [PATCH 4/6] Add more test cases for non-parsable inputs --- .../NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs b/test/NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs index ceada00..e48408e 100644 --- a/test/NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs +++ b/test/NUnit.Xml.TestLogger.UnitTests/TestCaseNameParserTests.cs @@ -100,6 +100,10 @@ public void Parse_ParsesAllParsableInputsWithoutNamespace_WithConsoleOutput(stri [DataRow("z.y.x.")] [DataRow("z.y.x.)")] [DataRow("z.y.x.\"\")")] + [DataRow("z.a.b((0,1)")] + [DataRow("z.a.b((0,1)))")] + [DataRow("z.a.b((0,(0,1))")] + [DataRow("z.a.b((0,(0,1))))")] public void Parse_FailsGracefullyOnNonParsableInputs_WithConsoleOutput(string testCaseName) { var expectedConsole = string.Format( From 402528ca3b25e73a97103dbfd72d598784495472 Mon Sep 17 00:00:00 2001 From: KonH Date: Fri, 25 Sep 2020 20:58:35 +0700 Subject: [PATCH 5/6] Closing parenthesis should be treated as error only when it is not part of tuple argument --- src/NUnit.Xml.TestLogger/TestCaseNameParser.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NUnit.Xml.TestLogger/TestCaseNameParser.cs b/src/NUnit.Xml.TestLogger/TestCaseNameParser.cs index a4989ad..3e43aa1 100644 --- a/src/NUnit.Xml.TestLogger/TestCaseNameParser.cs +++ b/src/NUnit.Xml.TestLogger/TestCaseNameParser.cs @@ -83,7 +83,7 @@ public static ParsedName Parse(string fullyQualifedName) } else if (thisChar == ')') { - if (output.Count > 0) + if ((output.Count > 0) && (parenthesisCount == 0)) { throw new Exception("The closing parenthesis we detected wouldn't be the last character in the output string. This isn't acceptable because we aren't in a string"); } From 141e96caf7effac5fef147b71f4d3fe802cf2295 Mon Sep 17 00:00:00 2001 From: KonH Date: Sun, 27 Sep 2020 13:40:55 +0700 Subject: [PATCH 6/6] Treat backslashes as errors only if they not inside parenthesis block --- src/NUnit.Xml.TestLogger/TestCaseNameParser.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/NUnit.Xml.TestLogger/TestCaseNameParser.cs b/src/NUnit.Xml.TestLogger/TestCaseNameParser.cs index 3e43aa1..6105203 100644 --- a/src/NUnit.Xml.TestLogger/TestCaseNameParser.cs +++ b/src/NUnit.Xml.TestLogger/TestCaseNameParser.cs @@ -77,7 +77,12 @@ public static ParsedName Parse(string fullyQualifedName) parenthesisCount--; } - if (thisChar == '"') + // Backslashes allowed only inside parenthesis block + if ((thisChar == '\\') && (parenthesisCount == 0)) + { + throw new Exception("Found invalid characters"); + } + else if (thisChar == '"') { throw new Exception("Found invalid characters"); }