Merge pull request #592 from launchableinc/nunit

Work around a bug in NUnitXML.Logger mishandling nested type
launchableinc · Jul 31, 2023 · 8716e7d · 8716e7d
2 parents 733ad78 + f99e6ac
commit 8716e7d
Show file tree

Hide file tree

Showing 9 changed files with 146 additions and 7 deletions.
diff --git a/launchable/test_runners/nunit.py b/launchable/test_runners/nunit.py
@@ -13,6 +13,25 @@
 # common code between 'subset' & 'record tests' to build up test path from
 # nested <test-suite>s
 
+"""
+Nested class name handling in .NET
+---------------------------------
+
+Nested class 'Zot' in the following example gets the full name "Foo.Bar+Zot":
+
+    namespace Foo {
+        class Bar {
+            class Zot {
+    }}}
+
+This is incontrast to how you refer to this class from the source code. For example,
+"new Foo.Bar.Zot()"
+
+The subset command expects the list of tests to be passed to "nunit --testlist" option,
+and this option expects test names to be in "Foo.Bar+Zot" format.
+
+"""
+
 
 def build_path(e: Element):
     pp = []  # type: TestPath
@@ -22,6 +41,28 @@ def build_path(e: Element):
         # <test-suite>s form a nested tree structure so capture those in path
         pp = pp + [{'type': e.attrs['type'], 'name': e.attrs['name']}]
     if e.name == "test-case":
+        # work around a bug in NUnitXML.Logger.
+        # see nunit-reporter-bug-with-nested-type.xml test case
+        methodname = e.attrs['methodname']
+        idx = methodname.rfind(".")
+        if idx >= 0:
+            # when things are going well, method name cannot contain '.' since it's not a valid character in a symbol.
+            # but when NUnitXML.Logger messes up, it ends up putting the class name and the method name, like
+            # <test-case name="TheTest" fullname="Launchable.NUnit.Test.Outer+Inner.TheTest"
+            #   methodname="Outer+Inner.TheTest" classname="Test"
+
+            pp = pp[0:-1] + [
+                # NUnitXML.Logger mistreats the last portion of the namespace as a test fixture when
+                # it really should be test suite. So we patch that up too. This is going beyond what's minimally required
+                # to make subset work, because type information won't impact how the test path is printed, but
+                # when NUnitXML.Logger eventually fixes this bug, we don't want that to produce different test paths.
+                {'type': 'TestSuite', 'name': pp[-1]['name']},
+                # Here, we need to insert the missing TestFixture=Outer+Inner.
+                # I chose TestFixture because that's what nunit console runner (which we believe is handling it correctly)
+                # chooses as its type.
+                {'type': 'TestFixture', 'name': methodname[0:idx]}
+            ]
+
         pp = pp + [{'type': 'TestCase', 'name': e.attrs['name']}]
 
     if len(pp) > 0:
@@ -32,7 +73,7 @@ def split_filepath(path: str) -> List[str]:
             else:
                 return path.split('\\')
 
-        # "Assembly" type containts full path at a cutomer's environment
+        # "Assembly" type contains full path at a customer's environment
         # remove file path prefix in Assembly
         e.tags['path'] = [
             {**path, 'name': split_filepath(path['name'])[-1]}

diff --git a/tests/data/nunit/nunit-reporter-bug-with-nested-type.json b/tests/data/nunit/nunit-reporter-bug-with-nested-type.json
@@ -0,0 +1,24 @@
+{
+  "events": [
+    {
+      "type": "case",
+      "testPath": [
+        { "type": "Assembly",    "name": "Launchable.NUnit.Test.dll" },
+        { "type": "TestSuite",   "name": "Launchable" },
+        { "type": "TestSuite",   "name": "NUnit" },
+        { "type": "TestSuite",   "name": "Test" },
+        { "type": "TestSuite",   "name": "Outer+Inner" },
+        { "type": "TestCase",    "name": "TheTest" }
+      ],
+      "created_at": "2023-07-28T 22:32:13Z",
+      "duration": 1.6e-05,
+      "status": 1,
+      "stdout": "",
+      "stderr": "",
+      "data": null
+    }
+  ],
+  "testRunner": "nunit",
+  "group": "",
+  "noBuild": false
+}
diff --git a/tests/data/nunit/nunit-reporter-bug-with-nested-type.xml b/tests/data/nunit/nunit-reporter-bug-with-nested-type.xml
@@ -0,0 +1,22 @@
+<!--
+  This report file was created from github.com/launchableinc/nunit 8ce509a573ac3fa84d2b6c22f28c46e044f6e904
+  by running "dotnet test -l:nunit", which triggers an apparent bug in NUnitXML.Logger and produces an incorrect
+  <test-case> that has more than a method name in its @methodname.
+
+  NUnit console runner and its reporter doesn't have this issue, as tested in src/Tests.cs, we need to work around
+  this bug.
+
+  See https://launchableinc.atlassian.net/l/cp/aYKtdjDw for an internal investigation note with more context.
+-->
+<test-run id="2" duration="0.012734" testcasecount="7" total="7" passed="5" failed="0" inconclusive="2" skipped="0" result="Passed" start-time="2023-07-28T 22:32:13Z" end-time="2023-07-28T 22:32:13Z">
+  <test-suite type="Assembly" name="Launchable.NUnit.Test.dll" fullname="/home/kohsuke/ws/launchable/nunit/Launchable.NUnit.Test/bin/Debug/net6.0/Launchable.NUnit.Test.dll" total="7" passed="5" failed="0" inconclusive="2" skipped="0" result="Passed" start-time="2023-07-28T 22:32:13Z" end-time="2023-07-28T 22:32:13Z" duration="0.012734">
+    <test-suite type="TestSuite" name="Launchable" fullname="Launchable" total="1" passed="1" failed="0" inconclusive="0" skipped="0" result="Passed" start-time="2023-07-28T 22:32:13Z" end-time="2023-07-28T 22:32:13Z" duration="1.6E-05">
+      <test-suite type="TestSuite" name="NUnit" fullname="Launchable.NUnit" total="1" passed="1" failed="0" inconclusive="0" skipped="0" result="Passed" start-time="2023-07-28T 22:32:13Z" end-time="2023-07-28T 22:32:13Z" duration="1.6E-05">
+        <test-suite type="TestFixture" name="Test" fullname="Launchable.NUnit.Test" classname="Launchable.NUnit.Test" total="1" passed="1" failed="0" inconclusive="0" skipped="0" result="Passed" start-time="2023-07-28T 22:32:13Z" end-time="2023-07-28T 22:32:13Z" duration="1.6E-05">
+          <test-case name="TheTest" fullname="Launchable.NUnit.Test.Outer+Inner.TheTest" methodname="Outer+Inner.TheTest" classname="Test" result="Passed" start-time="2023-07-28T 22:32:13Z" end-time="2023-07-28T 22:32:13Z" duration="1.6E-05" asserts="0" seed="427950853" />
+        </test-suite>
+      </test-suite>
+    </test-suite>
+    <errors />
+  </test-suite>
+</test-run>
diff --git a/tests/data/nunit/output-linux.xml b/tests/data/nunit/output-linux.xml
@@ -1,8 +1,8 @@
 <?xml version="1.0" encoding="utf-8" standalone="no"?>
-<test-run id="0" runstate="Runnable" testcasecount="6" result="Failed" total="6" passed="5" failed="1" inconclusive="0" skipped="0" asserts="3" engine-version="3.12.0.0" clr-version="3.1.15" start-time="2021-05-25 01:24:39Z" end-time="2021-05-25 01:24:39Z" duration="0.172244">
-  <command-line><![CDATA[/home/kohsuke/ws/launchable/examples/nunit/nunit-console/bin/netcoreapp3.1/nunit3-console.dll --noc ./bin/Debug/netcoreapp3.1/calc.dll]]></command-line>
-  <test-suite type="Assembly" id="1-1010" name="/home/kohsuke/ws/launchable/cli/tests/data/nunit/src/bin/Debug/netcoreapp3.1/calc.dll" fullname="/home/kohsuke/ws/launchable/cli/tests/data/nunit/src/bin/Debug/netcoreapp3.1/calc.dll" runstate="Runnable" testcasecount="6" result="Failed" site="Child" start-time="2021-05-25T01:24:39.4968461Z" end-time="2021-05-25T01:24:39.5361345Z" duration="0.039246" total="6" passed="5" failed="1" warnings="0" inconclusive="0" skipped="0" asserts="3">
-    <environment framework-version="3.13.2.0" clr-version="3.1.15" os-version="Linux 5.4.0-71-generic #79-Ubuntu SMP Wed Mar 24 10:56:57 UTC 2021" platform="Unix" cwd="/home/kohsuke/ws/launchable/cli/tests/data/nunit/src" machine-name="atlas" user="kohsuke" user-domain="atlas" culture="en-US" uiculture="en-US" os-architecture="x64" />
+<test-run id="0" runstate="Runnable" testcasecount="7" result="Failed" total="7" passed="6" failed="1" inconclusive="0" skipped="0" asserts="3" engine-version="3.12.0.0" clr-version="7.0.9" start-time="2023-07-27 17:47:58Z" end-time="2023-07-27 17:47:58Z" duration="0.067628">
+  <command-line><![CDATA[/usr/local/nunit-3.12/bin/netcoreapp3.1/nunit3-console.dll ./bin/Debug/netcoreapp3.1/calc.dll]]></command-line>
+  <test-suite type="Assembly" id="1-1012" name="calc.dll" fullname="/home/kohsuke/ws/launchable/cli/tests/data/nunit/src/bin/Debug/netcoreapp3.1/calc.dll" runstate="Runnable" testcasecount="7" result="Failed" site="Child" start-time="2023-07-27T17:47:58.8118130Z" end-time="2023-07-27T17:47:58.8334356Z" duration="0.021608" total="7" passed="6" failed="1" warnings="0" inconclusive="0" skipped="0" asserts="3">
+    <environment framework-version="3.13.2.0" clr-version="7.0.9" os-version="Linux 5.19.0-41-generic #42~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Tue Apr 18 17:40:00 UTC 2" platform="Unix" cwd="/home/kohsuke/ws/launchable/cli/tests/data/nunit/src" machine-name="nue" user="kohsuke" user-domain="nue" culture="en-US" uiculture="en-US" os-architecture="x64" />
     <settings>
       <setting name="DisposeRunners" value="True" />
       <setting name="WorkDirectory" value="/home/kohsuke/ws/launchable/cli/tests/data/nunit/src" />
@@ -19,6 +19,9 @@
       <failure>
         <message><![CDATA[One or more child tests had errors]]></message>
       </failure>
+      <test-suite type="TestFixture" id="1-1010" name="Outer+Inner" fullname="calc.Outer+Inner" classname="calc.Outer+Inner" runstate="Runnable" testcasecount="1" result="Passed" start-time="2023-07-27T17:47:58.8149224Z" end-time="2023-07-27T17:47:58.8184453Z" duration="0.003523" total="1" passed="1" failed="0" warnings="0" inconclusive="0" skipped="0" asserts="0">
+        <test-case id="1-1011" name="Test3" fullname="calc.Outer+Inner.Test3" methodname="Test3" classname="calc.Outer+Inner" runstate="Runnable" seed="392591847" result="Passed" start-time="2023-07-27T17:47:58.8159407Z" end-time="2023-07-27T17:47:58.8171186Z" duration="0.001215" asserts="0" />
+      </test-suite>
       <test-suite type="TestSuite" id="1-1013" name="sub" fullname="calc.sub" runstate="Runnable" testcasecount="1" result="Passed" start-time="2021-05-25T01:24:39.5004834Z" end-time="2021-05-25T01:24:39.5073756Z" duration="0.006892" total="1" passed="1" failed="0" warnings="0" inconclusive="0" skipped="0" asserts="0">
         <test-suite type="TestFixture" id="1-1008" name="Tests2" fullname="calc.sub.Tests2" classname="calc.sub.Tests2" runstate="Runnable" testcasecount="1" result="Passed" start-time="2021-05-25T01:24:39.5013277Z" end-time="2021-05-25T01:24:39.5070867Z" duration="0.005760" total="1" passed="1" failed="0" warnings="0" inconclusive="0" skipped="0" asserts="0">
           <test-case id="1-1009" name="Foo" fullname="calc.sub.Tests2.Foo" methodname="Foo" classname="calc.sub.Tests2" runstate="Runnable" seed="193364485" result="Passed" start-time="2021-05-25T01:24:39.5030637Z" end-time="2021-05-25T01:24:39.5050639Z" duration="0.002087" asserts="0">

diff --git a/tests/data/nunit/output-windows.xml b/tests/data/nunit/output-windows.xml
@@ -19,6 +19,9 @@
       <failure>
         <message><![CDATA[One or more child tests had errors]]></message>
       </failure>
+      <test-suite type="TestFixture" id="1-1010" name="Outer+Inner" fullname="calc.Outer+Inner" classname="calc.Outer+Inner" runstate="Runnable" testcasecount="1" result="Passed" start-time="2023-07-27T17:47:58.8149224Z" end-time="2023-07-27T17:47:58.8184453Z" duration="0.003523" total="1" passed="1" failed="0" warnings="0" inconclusive="0" skipped="0" asserts="0">
+        <test-case id="1-1011" name="Test3" fullname="calc.Outer+Inner.Test3" methodname="Test3" classname="calc.Outer+Inner" runstate="Runnable" seed="392591847" result="Passed" start-time="2023-07-27T17:47:58.8159407Z" end-time="2023-07-27T17:47:58.8171186Z" duration="0.001215" asserts="0" />
+      </test-suite>
       <test-suite type="TestSuite" id="1-1013" name="sub" fullname="calc.sub" runstate="Runnable" testcasecount="1" result="Passed" start-time="2021-05-25T01:24:39.5004834Z" end-time="2021-05-25T01:24:39.5073756Z" duration="0.006892" total="1" passed="1" failed="0" warnings="0" inconclusive="0" skipped="0" asserts="0">
         <test-suite type="TestFixture" id="1-1008" name="Tests2" fullname="calc.sub.Tests2" classname="calc.sub.Tests2" runstate="Runnable" testcasecount="1" result="Passed" start-time="2021-05-25T01:24:39.5013277Z" end-time="2021-05-25T01:24:39.5070867Z" duration="0.005760" total="1" passed="1" failed="0" warnings="0" inconclusive="0" skipped="0" asserts="0">
           <test-case id="1-1009" name="Foo" fullname="calc.sub.Tests2.Foo" methodname="Foo" classname="calc.sub.Tests2" runstate="Runnable" seed="193364485" result="Passed" start-time="2021-05-25T01:24:39.5030637Z" end-time="2021-05-25T01:24:39.5050639Z" duration="0.002087" asserts="0">

diff --git a/tests/data/nunit/record_test_result.json b/tests/data/nunit/record_test_result.json
@@ -46,6 +46,21 @@
       "stderr": "",
       "data": null
     },
+    {
+      "type": "case",
+      "testPath": [
+        { "type": "Assembly",    "name": "calc.dll" },
+        { "type": "TestSuite",   "name": "calc" },
+        { "type": "TestSuite", "name": "Outer+Inner" },
+        { "type": "TestCase",    "name": "Test3" }
+      ],
+      "created_at": "2023-07-27T17:47:58.8159407Z",
+      "duration": 0.001215,
+      "status": 1,
+      "stdout": "",
+      "stderr": "",
+      "data": null
+    },
 
     {
       "type": "case",

diff --git a/tests/data/nunit/src/README.md b/tests/data/nunit/src/README.md
@@ -1,11 +1,10 @@
 # Dev environment
 * [Install .NET SDK](https://dotnet.microsoft.com/download) on your platform of choice.
-  Specifically, .NET Core 3.1
 * Install NUnit console runner from [the zip page](https://github.com/nunit/nunit-console/releases/tag/v3.12)
   or other formats of your choice
 * `dotnet build` will produce `bin/Debug/netcoreapp3.1/calc.dll` that contains all the code
 * `dotnet $NUNIT/bin/netcoreapp3.1/nunit3-console.dll` to run NUnit3 console runner.
-  More specifically, `dotnet $NUNIT/bin/netcoreapp3.1/nunit3-console.dll ./bin/Debug/netcoreapp3.1/calc.dll` will run tests
+  More specifically, `dotnet --roll-forward LatestMajor $NUNIT/bin/netcoreapp3.1/nunit3-console.dll ./bin/Debug/netcoreapp3.1/calc.dll` will run tests (with the most recent .NET SDK you have)
   and produce `TestResult.xml` that we capture as `../output.xml`.
 * `--explore=list.xml` to produec the test list.
 * Internal: See https://launchableinc.atlassian.net/l/c/RUoketE0 for more detailed notes.
diff --git a/tests/data/nunit/src/Test.cs b/tests/data/nunit/src/Test.cs
@@ -17,6 +17,23 @@ public void Test2()
             Assert.Fail();
         }
     }
+
+    public class Outer
+    {
+        public class Inner
+        {
+            [Test]
+            public void Test3()
+            {
+                // this is how you access the inner class, but ...
+                var x = new Outer.Inner();
+
+                // you get the different type name
+                Assert.That(typeof(Inner).Name, Is.EqualTo("Inner"));
+                Assert.That(typeof(Inner).FullName, Is.EqualTo("calc.Outer+Inner"));
+            }
+        }
+    }
 }
 
 namespace calc.sub

diff --git a/tests/test_runners/test_nunit.py b/tests/test_runners/test_nunit.py
@@ -184,3 +184,18 @@ def test_record_test_on_windows(self):
         expected = self.load_json_from_file(self.test_files_dir.joinpath("record_test_result.json"))
 
         self.assert_json_orderless_equal(expected, payload)
+
+    @responses.activate
+    @mock.patch.dict(os.environ, {"LAUNCHABLE_TOKEN": CliTestCase.launchable_token})
+    def test_record_test_with_nunit_reporter_bug(self):
+        result = self.cli('record', 'tests', '--session', self.session,
+                          'nunit', str(self.test_files_dir) + "/nunit-reporter-bug-with-nested-type.xml")
+        self.assertEqual(result.exit_code, 0)
+
+        payload = json.loads(gzip.decompress(responses.calls[1].request.body).decode())
+
+        # turns out we collapse all TestFixtures to TestSuitest so the golden file has TestSuite=Outer+Inner,
+        # not TestFixture=Outer+Inner
+        expected = self.load_json_from_file(self.test_files_dir.joinpath("nunit-reporter-bug-with-nested-type.json"))
+
+        self.assert_json_orderless_equal(expected, payload)