From 1af35ed44618c64fa44d9d33028648ee879d7f06 Mon Sep 17 00:00:00 2001 From: Charlie Poole Date: Sat, 30 Nov 2024 12:32:20 -0800 Subject: [PATCH] Fix exception arising from parsing test name --- .../Services/TestSelectionParserTests.cs | 125 +++++++++++++----- .../Services/TestSelectionParser.cs | 91 ++++++++++++- .../nunit.engine/Services/Tokenizer.cs | 3 +- 3 files changed, 176 insertions(+), 43 deletions(-) diff --git a/src/NUnitEngine/nunit.engine.tests/Services/TestSelectionParserTests.cs b/src/NUnitEngine/nunit.engine.tests/Services/TestSelectionParserTests.cs index 47500507a..334e690f7 100644 --- a/src/NUnitEngine/nunit.engine.tests/Services/TestSelectionParserTests.cs +++ b/src/NUnitEngine/nunit.engine.tests/Services/TestSelectionParserTests.cs @@ -2,6 +2,8 @@ using NUnit.Framework; using System; +using System.Collections; +using System.Collections.Generic; using System.Xml; namespace NUnit.Engine.Tests @@ -16,47 +18,19 @@ public void CreateParser() _parser = new TestSelectionParser(); } - [TestCase("cat=Urgent", "Urgent")] - [TestCase("cat==Urgent", "Urgent")] - [TestCase("cat!=Urgent", "Urgent")] - [TestCase("cat =~ Urgent", "Urgent")] - [TestCase("cat !~ Urgent", "Urgent")] - [TestCase("cat = Urgent || cat = High", "UrgentHigh")] - [TestCase("Priority == High", "High")] - [TestCase("Priority != Urgent", "Urgent")] - [TestCase("Author =~ Jones", "Jones")] - [TestCase("Author !~ Jones", "Jones")] - [TestCase("name='SomeTest'", "SomeTest")] - [TestCase("method=TestMethod", "TestMethod")] - [TestCase("method=Test1||method=Test2||method=Test3", "Test1Test2Test3")] - [TestCase("namespace=Foo", "Foo")] - [TestCase("namespace=Foo.Bar", "Foo.Bar")] - [TestCase("namespace=Foo||namespace=Bar", "FooBar")] - [TestCase("namespace=Foo.Bar||namespace=Bar.Baz", "Foo.BarBar.Baz")] - [TestCase("test='My.Test.Fixture.Method(42)'", "My.Test.Fixture.Method(42)")] - [TestCase("test='My.Test.Fixture.Method(\"xyz\")'", "My.Test.Fixture.Method("xyz")")] - [TestCase("test='My.Test.Fixture.Method(\"abc\\'s\")'", "My.Test.Fixture.Method("abc's")")] - [TestCase("test='My.Test.Fixture.Method(\"x&y&z\")'", "My.Test.Fixture.Method("x&y&z")")] - [TestCase("test='My.Test.Fixture.Method(\"\")'", "My.Test.Fixture.Method("<xyz>")")] - [TestCase("test=='Issue1510.TestSomething(Option1,\"ABC\")'", "Issue1510.TestSomething(Option1,"ABC")")] - [TestCase("cat==Urgent && test=='My.Tests'", "UrgentMy.Tests")] - [TestCase("cat==Urgent and test=='My.Tests'", "UrgentMy.Tests")] - [TestCase("cat==Urgent || test=='My.Tests'", "UrgentMy.Tests")] - [TestCase("cat==Urgent or test=='My.Tests'", "UrgentMy.Tests")] - [TestCase("cat==Urgent || test=='My.Tests' && cat == high", "UrgentMy.Testshigh")] - [TestCase("cat==Urgent && test=='My.Tests' || cat == high", "UrgentMy.Testshigh")] - [TestCase("cat==Urgent && (test=='My.Tests' || cat == high)", "UrgentMy.Testshigh")] - [TestCase("cat==Urgent && !(test=='My.Tests' || cat == high)", "UrgentMy.Testshigh")] - [TestCase("!(test!='My.Tests')", "My.Tests")] - [TestCase("!(cat!=Urgent)", "Urgent")] - public void TestParser(string input, string output) + [TestCaseSource(nameof(UniqueOutputs))] + public void AllOutputsAreValidXml(string output) { - Assert.That(_parser.Parse(input), Is.EqualTo(output)); - XmlDocument doc = new XmlDocument(); Assert.DoesNotThrow(() => doc.LoadXml(output)); } + [TestCaseSource(nameof(ParserTestCases))] + public void TestParser(string input, string output) + { + Assert.That(_parser.Parse(input), Is.EqualTo(output)); + } + [TestCase(null, typeof(ArgumentNullException))] [TestCase("", typeof(TestSelectionParserException))] [TestCase(" ", typeof(TestSelectionParserException))] @@ -65,5 +39,84 @@ public void TestParser_InvalidInput(string input, Type type) { Assert.That(() => _parser.Parse(input), Throws.TypeOf(type)); } + + private static readonly TestCaseData[] ParserTestCases = new[] + { + // Category Filter + new TestCaseData("cat=Urgent", "Urgent"), + new TestCaseData("cat=/Urgent/", "Urgent"), + new TestCaseData("cat='Urgent'", "Urgent"), + new TestCaseData("cat==Urgent", "Urgent"), + new TestCaseData("cat!=Urgent", "Urgent"), + new TestCaseData("cat =~ Urgent", "Urgent"), + new TestCaseData("cat !~ Urgent", "Urgent"), + // Property Filter + new TestCaseData("Priority == High", "High"), + new TestCaseData("Priority != Urgent", "Urgent"), + new TestCaseData("Author =~ Jones", "Jones"), + new TestCaseData("Author !~ Jones", "Jones"), + // Name Filter + new TestCaseData("name='SomeTest'", "SomeTest"), + // Method Filter + new TestCaseData("method=TestMethod", "TestMethod"), + new TestCaseData("method=Test1||method=Test2||method=Test3", "Test1Test2Test3"), + // Namespace Filter + new TestCaseData("namespace=Foo", "Foo"), + new TestCaseData("namespace=Foo.Bar", "Foo.Bar"), + new TestCaseData("namespace=Foo||namespace=Bar", "FooBar"), + new TestCaseData("namespace=Foo.Bar||namespace=Bar.Baz", "Foo.BarBar.Baz"), + // Test Filter + new TestCaseData("test='My.Test.Fixture.Method(42)'", "My.Test.Fixture.Method(42)"), + new TestCaseData("test='My.Test.Fixture.Method(\"xyz\")'", "My.Test.Fixture.Method("xyz")"), + new TestCaseData("test='My.Test.Fixture.Method(\"abc\\'s\")'", "My.Test.Fixture.Method("abc's")"), + new TestCaseData("test='My.Test.Fixture.Method(\"x&y&z\")'", "My.Test.Fixture.Method("x&y&z")"), + new TestCaseData("test='My.Test.Fixture.Method(\"\")'", "My.Test.Fixture.Method("<xyz>")"), + new TestCaseData("test=='Issue1510.TestSomething ( Option1 , \"ABC\" ) '", "Issue1510.TestSomething(Option1,"ABC")"), + new TestCaseData("test=='Issue1510.TestSomething ( Option1 , \"A B C\" ) '", "Issue1510.TestSomething(Option1,"A B C")"), + new TestCaseData("test=/My.Test.Fixture.Method(42)/", "My.Test.Fixture.Method(42)"), + new TestCaseData("test=/My.Test.Fixture.Method(\"xyz\")/", "My.Test.Fixture.Method("xyz")"), + new TestCaseData("test=/My.Test.Fixture.Method(\"abc\\'s\")/", "My.Test.Fixture.Method("abc's")"), + new TestCaseData("test=/My.Test.Fixture.Method(\"x&y&z\")/", "My.Test.Fixture.Method("x&y&z")"), + new TestCaseData("test=/My.Test.Fixture.Method(\"\")/", "My.Test.Fixture.Method("<xyz>")"), + new TestCaseData("test==/Issue1510.TestSomething ( Option1 , \"ABC\" ) /", "Issue1510.TestSomething(Option1,"ABC")"), + new TestCaseData("test==/Issue1510.TestSomething ( Option1 , \"A B C\" ) /", "Issue1510.TestSomething(Option1,"A B C")"), + new TestCaseData("test=My.Test.Fixture.Method(42)", "My.Test.Fixture.Method(42)"), + new TestCaseData("test=My.Test.Fixture.Method(\"xyz\")", "My.Test.Fixture.Method("xyz")"), + new TestCaseData("test=My.Test.Fixture.Method(\"abc\\'s\")", "My.Test.Fixture.Method("abc's")"), + new TestCaseData("test=My.Test.Fixture.Method(\"x&y&z\")", "My.Test.Fixture.Method("x&y&z")"), + new TestCaseData("test=My.Test.Fixture.Method(\"\")", "My.Test.Fixture.Method("<xyz>")"), + new TestCaseData("test==Issue1510.TestSomething ( Option1 , \"ABC\" ) ", "Issue1510.TestSomething(Option1,"ABC")"), + new TestCaseData("test==Issue1510.TestSomething ( Option1 , \"A B C\" ) ", "Issue1510.TestSomething(Option1,"A B C")"), + // And Filter + new TestCaseData("cat==Urgent && test=='My.Tests'", "UrgentMy.Tests"), + new TestCaseData("cat==Urgent and test=='My.Tests'", "UrgentMy.Tests"), + // Or Filter + new TestCaseData("cat==Urgent || test=='My.Tests'", "UrgentMy.Tests"), + new TestCaseData("cat==Urgent or test=='My.Tests'", "UrgentMy.Tests"), + // Mixed And Filter with Or Filter + new TestCaseData("cat = Urgent || cat = High", "UrgentHigh"), + new TestCaseData("cat==Urgent || test=='My.Tests' && cat == high", "UrgentMy.Testshigh"), + new TestCaseData("cat==Urgent && test=='My.Tests' || cat == high", "UrgentMy.Testshigh"), + new TestCaseData("cat==Urgent && (test=='My.Tests' || cat == high)", "UrgentMy.Testshigh"), + new TestCaseData("cat==Urgent && !(test=='My.Tests' || cat == high)", "UrgentMy.Testshigh"), + // Not Filter + new TestCaseData("!(test!='My.Tests')", "My.Tests"), + new TestCaseData("!(cat!=Urgent)", "Urgent") + }; + + private static IEnumerable UniqueOutputs() + { + List alreadyReturned = new List(); + + foreach (var testCase in ParserTestCases) + { + var output = testCase.Arguments[1] as string; + if (!alreadyReturned.Contains(output)) + { + alreadyReturned.Add(output); + yield return output; + } + } + } } } diff --git a/src/NUnitEngine/nunit.engine/Services/TestSelectionParser.cs b/src/NUnitEngine/nunit.engine/Services/TestSelectionParser.cs index 387836edf..8873d172c 100644 --- a/src/NUnitEngine/nunit.engine/Services/TestSelectionParser.cs +++ b/src/NUnitEngine/nunit.engine/Services/TestSelectionParser.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; +using System.Runtime.CompilerServices; using System.Text; // Missing XML Docs @@ -37,6 +38,7 @@ public class TestSelectionParser private static readonly Token[] REL_OPS = new Token[] { EQ_OP1, EQ_OP2, NE_OP, MATCH_OP, NOMATCH_OP }; private static readonly Token EOF = new Token(TokenKind.Eof); + private static readonly Token COMMA = new Token(TokenKind.Symbol, ","); public string Parse(string input) { @@ -116,21 +118,29 @@ public string ParseFilterElement() return ParseExpressionInParentheses(); Token lhs = Expect(TokenKind.Word); + Token op; + Token rhs; switch (lhs.Text) { - case "id": + case "test": + op = Expect(REL_OPS); + rhs = GetTestName(); + return EmitFilterElement(lhs, op, rhs); + case "cat": case "method": case "class": case "name": - case "test": case "namespace": case "partition": - Token op = lhs.Text == "id" - ? Expect(EQ_OPS) - : Expect(REL_OPS); - Token rhs = Expect(TokenKind.String, TokenKind.Word); + op = Expect(REL_OPS); + rhs = Expect(TokenKind.String, TokenKind.Word); + return EmitFilterElement(lhs, op, rhs); + + case "id": + op = Expect(EQ_OPS); + rhs = Expect(TokenKind.String, TokenKind.Word); return EmitFilterElement(lhs, op, rhs); default: @@ -142,6 +152,75 @@ public string ParseFilterElement() } } + // TODO: We do extra work for test names due to the fact that + // Windows drops double quotes from arguments in many situations. + // It would be better to parse the command-line directly but + // that will mean a significant rewrite. + private Token GetTestName() + { + var result = Expect(TokenKind.String, TokenKind.Word); + var sb = new StringBuilder(); + + if (result.Kind == TokenKind.String) + { + int index = result.Text.IndexOf('('); + + if (index < 0) + return result; + + // Remove white space around arguments + string testName = result.Text; + sb = new StringBuilder(testName.Substring(0, index).Trim()); + sb.Append('('); + bool done = false; + + while (++index < testName.Length && !done) + { + char ch = testName[index]; + switch (ch) + { + case '"': + sb.Append(ch); + while (++index < testName.Length && testName[index] != '"') + sb.Append(testName[index]); + sb.Append('"'); + break; + case ' ': + break; + default: + sb.Append(ch); + done = ch == ')'; + break; + } + } + } + else + { + // Word Token - check to see if it's followed by a left parenthesis + if (_tokenizer.LookAhead != LPAREN) return result; + + // We have a "Word" token followed by a left parenthesis + // This may be a testname entered without quotes or one + // using double quotes, which were removed by the shell. + + sb = new StringBuilder(result.Text); + var token = NextToken(); + + while (token != EOF) + { + bool isString = token.Kind == TokenKind.String; + + if (isString) sb.Append('"'); + sb.Append(token.Text); + if (isString) sb.Append('"'); + + token = NextToken(); + } + } + + return new Token(TokenKind.String, sb.ToString()); + } + private static string EmitFilterElement(Token lhs, Token op, Token rhs) { string fmt = null; diff --git a/src/NUnitEngine/nunit.engine/Services/Tokenizer.cs b/src/NUnitEngine/nunit.engine/Services/Tokenizer.cs index e8d4fd4c5..ece264277 100644 --- a/src/NUnitEngine/nunit.engine/Services/Tokenizer.cs +++ b/src/NUnitEngine/nunit.engine/Services/Tokenizer.cs @@ -83,7 +83,7 @@ public class Tokenizer private int _index; private const char EOF_CHAR = '\0'; - private const string WORD_BREAK_CHARS = "=!()&|"; + private const string WORD_BREAK_CHARS = "=!()&| \t,"; private readonly string[] DOUBLE_CHAR_SYMBOLS = new string[] { "==", "=~", "!=", "!~", "&&", "||" }; private Token _lookahead; @@ -130,6 +130,7 @@ private Token GetNextToken() // Single char symbols case '(': case ')': + case ',': GetChar(); return new Token(TokenKind.Symbol, ch) { Pos = pos };