From 4144f86cf0f796ce47272db9f567fb24dac62a4c Mon Sep 17 00:00:00 2001
From: Marcel Gaupp <m.gaupp@tum.de>
Date: Tue, 15 Oct 2024 12:11:26 +0200
Subject: [PATCH 1/2] Fix nested <testsuite> parsing

---
 .../service/TestResultXmlParser.java          | 28 +++++----------
 .../service/TestResultXmlParserTest.java      | 34 +++++++++++++++++++
 2 files changed, 42 insertions(+), 20 deletions(-)

diff --git a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParser.java b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParser.java
index 5e2672e4b426..50c3c5925f97 100644
--- a/src/main/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParser.java
+++ b/src/main/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParser.java
@@ -33,22 +33,8 @@ public static void processTestResultFile(String testResultFileString, List<Build
         testResultFileString = testResultFileString.replaceAll(INVALID_XML_CHARS, "");
         TestSuite testSuite = mapper.readValue(testResultFileString, TestSuite.class);
 
-        // If the xml file is only one test suite, parse it directly
-        if (!testSuite.testCases().isEmpty()) {
-            processTestSuite(testSuite, failedTests, successfulTests);
-        }
-        else {
-            // Else, check if the file contains an outer <testsuites> element
-            // And parse the inner test suites
-            TestSuites suites = mapper.readValue(testResultFileString, TestSuites.class);
-            if (suites.testsuites() == null) {
-                return;
-            }
-
-            for (TestSuite suite : suites.testsuites()) {
-                processTestSuite(suite, failedTests, successfulTests);
-            }
-        }
+        // A toplevel <testsuites> element is parsed like a <testsuite>
+        processTestSuite(testSuite, failedTests, successfulTests);
     }
 
     private static void processTestSuite(TestSuite testSuite, List<BuildResult.LocalCITestJobDTO> failedTests, List<BuildResult.LocalCITestJobDTO> successfulTests) {
@@ -64,17 +50,19 @@ private static void processTestSuite(TestSuite testSuite, List<BuildResult.Local
                 successfulTests.add(new BuildResult.LocalCITestJobDTO(testCase.name(), List.of()));
             }
         }
-    }
 
-    @JsonIgnoreProperties(ignoreUnknown = true)
-    record TestSuites(@JacksonXmlElementWrapper(useWrapping = false) @JacksonXmlProperty(localName = "testsuite") List<TestSuite> testsuites) {
+        for (TestSuite suite : testSuite.testSuites()) {
+            processTestSuite(suite, failedTests, successfulTests);
+        }
     }
 
     @JsonIgnoreProperties(ignoreUnknown = true)
-    record TestSuite(@JacksonXmlElementWrapper(useWrapping = false) @JacksonXmlProperty(localName = "testcase") List<TestCase> testCases) {
+    record TestSuite(@JacksonXmlElementWrapper(useWrapping = false) @JacksonXmlProperty(localName = "testcase") List<TestCase> testCases,
+            @JacksonXmlElementWrapper(useWrapping = false) @JacksonXmlProperty(localName = "testsuite") List<TestSuite> testSuites) {
 
         TestSuite {
             testCases = Objects.requireNonNullElse(testCases, Collections.emptyList());
+            testSuites = Objects.requireNonNullElse(testSuites, Collections.emptyList());
         }
     }
 
diff --git a/src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java b/src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java
index 3af5e6b64a69..5dc82df6cd57 100644
--- a/src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java
+++ b/src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java
@@ -208,4 +208,38 @@ void testEmptyTestMessage() throws IOException {
         assertThat(test.getName()).isEqualTo("mwe-name");
         assertThat(test.getTestMessages()).hasSize(1).contains("");
     }
+
+    @Test
+    void testNestedTestsuite() throws IOException {
+        String input = """
+                <?xml version="1.0" encoding="UTF-8" standalone="no"?>
+                <testsuites errors="0" failures="0" tests="12" time="0.013">
+                    <testsuite name="Tests" tests="12">
+                        <testsuite name="Properties" tests="9">
+                            <testsuite name="Checked by SmallCheck" tests="6">
+                                <testcase classname="Tests.Properties.Checked by SmallCheck" name="Testing filtering in A" time="0.004"/>
+                                <testcase classname="Tests.Properties.Checked by SmallCheck" name="Testing mapping in A" time="0.000"/>
+                                <testcase classname="Tests.Properties.Checked by SmallCheck" name="Testing filtering in B" time="0.003"/>
+                                <testcase classname="Tests.Properties.Checked by SmallCheck" name="Testing mapping in B" time="0.000"/>
+                                <testcase classname="Tests.Properties.Checked by SmallCheck" name="Testing filtering in C" time="0.001"/>
+                                <testcase classname="Tests.Properties.Checked by SmallCheck" name="Testing mapping in C" time="0.000"/>
+                            </testsuite>
+                            <testsuite name="Checked by QuickCheck" tests="3">
+                                <testcase classname="Tests.Properties.Checked by QuickCheck" name="Testing A against sample solution" time="0.001"/>
+                                <testcase classname="Tests.Properties.Checked by QuickCheck" name="Testing B against sample solution" time="0.001"/>
+                                <testcase classname="Tests.Properties.Checked by QuickCheck" name="Testing C against sample solution" time="0.001"/>
+                            </testsuite>
+                        </testsuite>
+                        <testsuite name="Unit Tests" tests="3">
+                            <testcase classname="Tests.Unit Tests" name="Testing selectAndReflectA (0,0) []" time="0.000"/>
+                            <testcase classname="Tests.Unit Tests" name="Testing selectAndReflectB (0,1) [(0,0)]" time="0.000"/>
+                            <testcase classname="Tests.Unit Tests" name="Testing selectAndReflectC (0,1) [(-1,-1)]" time="0.000"/>
+                        </testsuite>
+                    </testsuite>
+                </testsuites>
+                """;
+
+        TestResultXmlParser.processTestResultFile(input, failedTests, successfulTests);
+        assertThat(successfulTests).hasSize(12);
+    }
 }

From a444db5fe264ee944be50bcdd8322c35a6205413 Mon Sep 17 00:00:00 2001
From: Marcel Gaupp <m.gaupp@tum.de>
Date: Tue, 15 Oct 2024 12:50:15 +0200
Subject: [PATCH 2/2] Test more comprehensively

---
 .../artemis/buildagent/service/TestResultXmlParserTest.java | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java b/src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java
index 5dc82df6cd57..f228d0223f9d 100644
--- a/src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java
+++ b/src/test/java/de/tum/cit/aet/artemis/buildagent/service/TestResultXmlParserTest.java
@@ -240,6 +240,10 @@ void testNestedTestsuite() throws IOException {
                 """;
 
         TestResultXmlParser.processTestResultFile(input, failedTests, successfulTests);
-        assertThat(successfulTests).hasSize(12);
+        assertThat(successfulTests).hasSize(12).extracting(BuildResult.LocalCITestJobDTO::getName).containsExactlyInAnyOrder("Testing filtering in A", "Testing mapping in A",
+                "Testing filtering in B", "Testing mapping in B", "Testing filtering in C", "Testing mapping in C", "Testing A against sample solution",
+                "Testing B against sample solution", "Testing C against sample solution", "Testing selectAndReflectA (0,0) []", "Testing selectAndReflectB (0,1) [(0,0)]",
+                "Testing selectAndReflectC (0,1) [(-1,-1)]");
+        assertThat(failedTests).isEmpty();
     }
 }