Skip to content

Commit 3b20039

Browse files
CharliePooleCharliePoole
authored
Merge pull request #1710 from nunit/issue-491
Fix Invalid XML from TestFixtureBuilder
2 parents 8420694 + ec8c79b commit 3b20039

File tree

4 files changed

+211
-167
lines changed

4 files changed

+211
-167
lines changed

‎src/NUnitEngine/nunit.engine.tests/Services/TestFilterBuilderTests.cs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public void CreateBuilder()
1818
public void EmptyFilter()
1919
{
2020
TestFilter filter = builder.GetFilter();
21-
Assert.That(filter.Text, Is.EqualTo("<filter></filter>"));
21+
Assert.That(filter.Text, Is.EqualTo("<filter />"));
2222
}
2323

2424
[Test]
@@ -28,7 +28,7 @@ public void OneTestSelected()
2828
TestFilter filter = builder.GetFilter();
2929

3030
Assert.That(filter.Text, Is.EqualTo(
31-
"<filter><test>My.Test.Name</test></filter>"));
31+
"<filter><test><![CDATA[My.Test.Name]]></test></filter>"));
3232
}
3333

3434
[Test]
@@ -38,7 +38,7 @@ public void OneTestSelected_XmlEscape()
3838
TestFilter filter = builder.GetFilter();
3939

4040
Assert.That(filter.Text, Is.EqualTo(
41-
"<filter><test>My.Test.Name&lt;T&gt;(&quot;abc&quot;)</test></filter>"));
41+
"<filter><test><![CDATA[My.Test.Name<T>(\"abc\")]]></test></filter>"));
4242
}
4343

4444
[Test]
@@ -50,7 +50,7 @@ public void ThreeTestsSelected()
5050
TestFilter filter = builder.GetFilter();
5151

5252
Assert.That(filter.Text, Is.EqualTo(
53-
"<filter><or><test>My.First.Test</test><test>My.Second.Test</test><test>My.Third.Test</test></or></filter>"));
53+
"<filter><test><![CDATA[My.First.Test]]></test><test><![CDATA[My.Second.Test]]></test><test><![CDATA[My.Third.Test]]></test></filter>"));
5454
}
5555

5656
[Test]
@@ -59,7 +59,7 @@ public void WhereClause()
5959
builder.SelectWhere("cat==Dummy");
6060
TestFilter filter = builder.GetFilter();
6161

62-
Assert.That(filter.Text, Is.EqualTo("<filter><cat>Dummy</cat></filter>"));
62+
Assert.That(filter.Text, Is.EqualTo("<filter><cat><![CDATA[Dummy]]></cat></filter>"));
6363
}
6464

6565
[Test]
@@ -69,7 +69,7 @@ public void WhereClause_XmlEscape()
6969
TestFilter filter = builder.GetFilter();
7070

7171
Assert.That(filter.Text, Is.EqualTo(
72-
"<filter><test>My.Test.Name&lt;T&gt;(&quot;abc&quot;)</test></filter>"));
72+
"<filter><test><![CDATA[My.Test.Name<T>(\"abc\")]]></test></filter>"));
7373
}
7474

7575
[Test]
@@ -80,7 +80,7 @@ public void OneTestAndWhereClause()
8080
TestFilter filter = builder.GetFilter();
8181

8282
Assert.That(filter.Text, Is.EqualTo(
83-
"<filter><test>My.Test.Name</test><not><cat>Slow</cat></not></filter>"));
83+
"<filter><test><![CDATA[My.Test.Name]]></test><not><cat><![CDATA[Slow]]></cat></not></filter>"));
8484
}
8585
}
8686
}

‎src/NUnitEngine/nunit.engine.tests/Services/TestSelectionParserTests.cs‎

Lines changed: 42 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -9,57 +9,57 @@ namespace NUnit.Engine.Services
99
public class TestSelectionParserTests
1010
{
1111
// Category Filter
12-
[TestCase("cat=Urgent", "<cat>Urgent</cat>")]
13-
[TestCase("cat==Urgent", "<cat>Urgent</cat>")]
14-
[TestCase("cat!=Urgent", "<not><cat>Urgent</cat></not>")]
15-
[TestCase("cat =~ Urgent", "<cat re='1'>Urgent</cat>")]
16-
[TestCase("cat !~ Urgent", "<not><cat re='1'>Urgent</cat></not>")]
17-
[TestCase("cat = Urgent || cat = High", "<or><cat>Urgent</cat><cat>High</cat></or>")]
12+
[TestCase("cat=Urgent", "<cat><![CDATA[Urgent]]></cat>")]
13+
[TestCase("cat==Urgent", "<cat><![CDATA[Urgent]]></cat>")]
14+
[TestCase("cat!=Urgent", "<not><cat><![CDATA[Urgent]]></cat></not>")]
15+
[TestCase("cat =~ Urgent", "<cat re=\"1\"><![CDATA[Urgent]]></cat>")]
16+
[TestCase("cat !~ Urgent", "<not><cat re=\"1\"><![CDATA[Urgent]]></cat></not>")]
17+
[TestCase("cat = Urgent || cat = High", "<or><cat><![CDATA[Urgent]]></cat><cat><![CDATA[High]]></cat></or>")]
1818
// Property Filter
19-
[TestCase("Priority == High", "<prop name='Priority'>High</prop>")]
20-
[TestCase("Priority != Urgent", "<not><prop name='Priority'>Urgent</prop></not>")]
21-
[TestCase("Author =~ Jones", "<prop name='Author' re='1'>Jones</prop>")]
22-
[TestCase("Author !~ Jones", "<not><prop name='Author' re='1'>Jones</prop></not>")]
19+
[TestCase("Priority == High", "<prop name=\"Priority\"><![CDATA[High]]></prop>")]
20+
[TestCase("Priority != Urgent", "<not><prop name=\"Priority\"><![CDATA[Urgent]]></prop></not>")]
21+
[TestCase("Author =~ Jones", "<prop name=\"Author\" re=\"1\"><![CDATA[Jones]]></prop>")]
22+
[TestCase("Author !~ Jones", "<not><prop name=\"Author\" re=\"1\"><![CDATA[Jones]]></prop></not>")]
2323
// Name Filter
24-
[TestCase("name='SomeTest'", "<name>SomeTest</name>")]
24+
[TestCase("name='SomeTest'", "<name><![CDATA[SomeTest]]></name>")]
2525
// Method Filter
26-
[TestCase("method=TestMethod", "<method>TestMethod</method>")]
27-
[TestCase("method=Test1||method=Test2||method=Test3", "<or><method>Test1</method><method>Test2</method><method>Test3</method></or>")]
26+
[TestCase("method=TestMethod", "<method><![CDATA[TestMethod]]></method>")]
27+
[TestCase("method=Test1||method=Test2||method=Test3", "<or><method><![CDATA[Test1]]></method><method><![CDATA[Test2]]></method><method><![CDATA[Test3]]></method></or>")]
2828
// Namespace Filter
29-
[TestCase("namespace=Foo", "<namespace>Foo</namespace>")]
30-
[TestCase("namespace=Foo.Bar", "<namespace>Foo.Bar</namespace>")]
31-
[TestCase("namespace=Foo||namespace=Bar", "<or><namespace>Foo</namespace><namespace>Bar</namespace></or>")]
32-
[TestCase("namespace=Foo.Bar||namespace=Bar.Baz", "<or><namespace>Foo.Bar</namespace><namespace>Bar.Baz</namespace></or>")]
29+
[TestCase("namespace=Foo", "<namespace><![CDATA[Foo]]></namespace>")]
30+
[TestCase("namespace=Foo.Bar", "<namespace><![CDATA[Foo.Bar]]></namespace>")]
31+
[TestCase("namespace=Foo||namespace=Bar", "<or><namespace><![CDATA[Foo]]></namespace><namespace><![CDATA[Bar]]></namespace></or>")]
32+
[TestCase("namespace=Foo.Bar||namespace=Bar.Baz", "<or><namespace><![CDATA[Foo.Bar]]></namespace><namespace><![CDATA[Bar.Baz]]></namespace></or>")]
3333
// Test Filter
34-
[TestCase("test='My.Test.Fixture.Method(42)'", "<test>My.Test.Fixture.Method(42)</test>")]
35-
[TestCase("test='My.Test.Fixture.Method(\"xyz\")'", "<test>My.Test.Fixture.Method(&quot;xyz&quot;)</test>")]
36-
[TestCase("test='My.Test.Fixture.Method(\"abc\\'s\")'", "<test>My.Test.Fixture.Method(&quot;abc&apos;s&quot;)</test>")]
37-
[TestCase("test='My.Test.Fixture.Method(\"x&y&z\")'", "<test>My.Test.Fixture.Method(&quot;x&amp;y&amp;z&quot;)</test>")]
38-
[TestCase("test='My.Test.Fixture.Method(\"<xyz>\")'", "<test>My.Test.Fixture.Method(&quot;&lt;xyz&gt;&quot;)</test>")]
39-
[TestCase("test == namespace.class(1).test1(1)", "<test>namespace.class(1).test1(1)</test>")]
40-
[TestCase("test == \"namespace.class(1).test1(1)\"", "<test>namespace.class(1).test1(1)</test>")]
41-
[TestCase("test == 'namespace.class(1).test1(1)'", "<test>namespace.class(1).test1(1)</test>")]
42-
[TestCase("test =~ \"(namespace\\.test1\\(1\\)|namespace\\.test2\\(2\\))\"", "<test re='1'>(namespace.test1(1)|namespace.test2(2))</test>")]
43-
[TestCase("test =~ '(namespace\\.test1\\(1\\)|namespace\\.test2\\(2\\))'", "<test re='1'>(namespace.test1(1)|namespace.test2(2))</test>")]
44-
[TestCase("test =~ /(namespace\\.test1\\(1\\)|namespace\\.test2\\(2\\))/", "<test re='1'>(namespace.test1(1)|namespace.test2(2))</test>")]
45-
[TestCase("test =~ \"(namespace1|namespace2)\\.test1\"", "<test re='1'>(namespace1|namespace2).test1</test>")]
46-
[TestCase("test =~ '(namespace1|namespace2)\\.test1'", "<test re='1'>(namespace1|namespace2).test1</test>")]
47-
[TestCase("test =~ /(namespace1|namespace2)\\.test1/", "<test re='1'>(namespace1|namespace2).test1</test>")]
48-
[TestCase("test='My.Test.Fixture.Method(\" A \\\\\" B \\\\\" C \")'", "<test>My.Test.Fixture.Method(&quot; A \\&quot; B \\&quot; C &quot;)</test>")]
34+
[TestCase("test='My.Test.Fixture.Method(42)'", "<test><![CDATA[My.Test.Fixture.Method(42)]]></test>")]
35+
[TestCase("test='My.Test.Fixture.Method(\"xyz\")'", "<test><![CDATA[My.Test.Fixture.Method(\"xyz\")]]></test>")]
36+
[TestCase("test='My.Test.Fixture.Method(\"abc\\'s\")'", "<test><![CDATA[My.Test.Fixture.Method(\"abc's\")]]></test>")]
37+
[TestCase("test='My.Test.Fixture.Method(\"x&y&z\")'", "<test><![CDATA[My.Test.Fixture.Method(\"x&y&z\")]]></test>")]
38+
[TestCase("test='My.Test.Fixture.Method(\"<xyz>\")'", "<test><![CDATA[My.Test.Fixture.Method(\"<xyz>\")]]></test>")]
39+
[TestCase("test == namespace.class(1).test1(1)", "<test><![CDATA[namespace.class(1).test1(1)]]></test>")]
40+
[TestCase("test == \"namespace.class(1).test1(1)\"", "<test><![CDATA[namespace.class(1).test1(1)]]></test>")]
41+
[TestCase("test == 'namespace.class(1).test1(1)'", "<test><![CDATA[namespace.class(1).test1(1)]]></test>")]
42+
[TestCase("test =~ \"(namespace\\.test1\\(1\\)|namespace\\.test2\\(2\\))\"", "<test re=\"1\"><![CDATA[(namespace.test1(1)|namespace.test2(2))]]></test>")]
43+
[TestCase("test =~ '(namespace\\.test1\\(1\\)|namespace\\.test2\\(2\\))'", "<test re=\"1\"><![CDATA[(namespace.test1(1)|namespace.test2(2))]]></test>")]
44+
[TestCase("test =~ /(namespace\\.test1\\(1\\)|namespace\\.test2\\(2\\))/", "<test re=\"1\"><![CDATA[(namespace.test1(1)|namespace.test2(2))]]></test>")]
45+
[TestCase("test =~ \"(namespace1|namespace2)\\.test1\"", "<test re=\"1\"><![CDATA[(namespace1|namespace2).test1]]></test>")]
46+
[TestCase("test =~ '(namespace1|namespace2)\\.test1'", "<test re=\"1\"><![CDATA[(namespace1|namespace2).test1]]></test>")]
47+
[TestCase("test =~ /(namespace1|namespace2)\\.test1/", "<test re=\"1\"><![CDATA[(namespace1|namespace2).test1]]></test>")]
48+
[TestCase("test='My.Test.Fixture.Method(\" A \\\\\" B \\\\\" C \")'", "<test><![CDATA[My.Test.Fixture.Method(\" A \\\" B \\\" C \")]]></test>")]
4949
// And Filter
50-
[TestCase("cat==Urgent && test=='My.Tests'", "<and><cat>Urgent</cat><test>My.Tests</test></and>")]
51-
[TestCase("cat==Urgent and test=='My.Tests'", "<and><cat>Urgent</cat><test>My.Tests</test></and>")]
50+
[TestCase("cat==Urgent && test=='My.Tests'", "<and><cat><![CDATA[Urgent]]></cat><test><![CDATA[My.Tests]]></test></and>")]
51+
[TestCase("cat==Urgent and test=='My.Tests'", "<and><cat><![CDATA[Urgent]]></cat><test><![CDATA[My.Tests]]></test></and>")]
5252
// Or Filter
53-
[TestCase("cat==Urgent || test=='My.Tests'", "<or><cat>Urgent</cat><test>My.Tests</test></or>")]
54-
[TestCase("cat==Urgent or test=='My.Tests'", "<or><cat>Urgent</cat><test>My.Tests</test></or>")]
53+
[TestCase("cat==Urgent || test=='My.Tests'", "<or><cat><![CDATA[Urgent]]></cat><test><![CDATA[My.Tests]]></test></or>")]
54+
[TestCase("cat==Urgent or test=='My.Tests'", "<or><cat><![CDATA[Urgent]]></cat><test><![CDATA[My.Tests]]></test></or>")]
5555
// Mixed And Filter with Or Filter
56-
[TestCase("cat==Urgent || test=='My.Tests' && cat == high", "<or><cat>Urgent</cat><and><test>My.Tests</test><cat>high</cat></and></or>")]
57-
[TestCase("cat==Urgent && test=='My.Tests' || cat == high", "<or><and><cat>Urgent</cat><test>My.Tests</test></and><cat>high</cat></or>")]
58-
[TestCase("cat==Urgent && (test=='My.Tests' || cat == high)", "<and><cat>Urgent</cat><or><test>My.Tests</test><cat>high</cat></or></and>")]
59-
[TestCase("cat==Urgent && !(test=='My.Tests' || cat == high)", "<and><cat>Urgent</cat><not><or><test>My.Tests</test><cat>high</cat></or></not></and>")]
56+
[TestCase("cat==Urgent || test=='My.Tests' && cat == high", "<or><cat><![CDATA[Urgent]]></cat><and><test><![CDATA[My.Tests]]></test><cat><![CDATA[high]]></cat></and></or>")]
57+
[TestCase("cat==Urgent && test=='My.Tests' || cat == high", "<or><and><cat><![CDATA[Urgent]]></cat><test><![CDATA[My.Tests]]></test></and><cat><![CDATA[high]]></cat></or>")]
58+
[TestCase("cat==Urgent && (test=='My.Tests' || cat == high)", "<and><cat><![CDATA[Urgent]]></cat><or><test><![CDATA[My.Tests]]></test><cat><![CDATA[high]]></cat></or></and>")]
59+
[TestCase("cat==Urgent && !(test=='My.Tests' || cat == high)", "<and><cat><![CDATA[Urgent]]></cat><not><or><test><![CDATA[My.Tests]]></test><cat><![CDATA[high]]></cat></or></not></and>")]
6060
// Not Filter
61-
[TestCase("!(test!='My.Tests')", "<not><not><test>My.Tests</test></not></not>")]
62-
[TestCase("!(cat!=Urgent)", "<not><not><cat>Urgent</cat></not></not>")]
61+
[TestCase("!(test!='My.Tests')", "<not><not><test><![CDATA[My.Tests]]></test></not></not>")]
62+
[TestCase("!(cat!=Urgent)", "<not><not><cat><![CDATA[Urgent]]></cat></not></not>")]
6363
public void TestParser(string input, string output)
6464
{
6565
Assert.That(TestSelectionParser.Parse(input), Is.EqualTo(output));

‎src/NUnitEngine/nunit.engine/Services/TestFilterBuilder.cs‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt
22

33
using System.Collections.Generic;
4+
using System.IO;
45
using System.Text;
6+
using System.Xml;
57

68
// Missing XML Docs
79
#pragma warning disable 1591
@@ -37,34 +39,28 @@ public void SelectWhere(string whereClause)
3739
/// <returns>A TestFilter.</returns>
3840
public TestFilter GetFilter()
3941
{
40-
var filter = new StringBuilder("<filter>");
42+
var filter = new StringBuilder();
43+
var xmlWriter = XmlWriter.Create(filter, new XmlWriterSettings { OmitXmlDeclaration = true });
4144

42-
if (_testList.Count > 0)
45+
xmlWriter.WriteStartElement("filter");
46+
47+
// NOTE: Top level "filter" element works like "or"
48+
foreach (string test in _testList)
4349
{
44-
if (_testList.Count > 1)
45-
filter.Append("<or>");
46-
foreach (string test in _testList)
47-
filter.AppendFormat("<test>{0}</test>", XmlEscape(test));
48-
if (_testList.Count > 1)
49-
filter.Append("</or>");
50+
xmlWriter.WriteStartElement("test");
51+
xmlWriter.WriteCData(test);
52+
xmlWriter.WriteEndElement();
5053
}
5154

55+
// We pass our XmlWriter to TestSelectionParser so it can parse
56+
// the where clause and leave the result where we will find it.
5257
if (_whereClause is not null)
53-
filter.Append(TestSelectionParser.Parse(_whereClause));
58+
TestSelectionParser.Parse(_whereClause, xmlWriter);
5459

55-
filter.Append("</filter>");
60+
xmlWriter.WriteEndElement();
61+
xmlWriter.Close();
5662

5763
return new TestFilter(filter.ToString());
5864
}
59-
60-
private static string XmlEscape(string text)
61-
{
62-
return text
63-
.Replace("&", "&amp;")
64-
.Replace("\"", "&quot;")
65-
.Replace("<", "&lt;")
66-
.Replace(">", "&gt;")
67-
.Replace("'", "&apos;");
68-
}
6965
}
7066
}

0 commit comments

Comments
 (0)