Programming exercises: Fix test case parsing of nested test suite ele…

…ments (#9490)
ls1intum · Oct 20, 2024 · 96e53d4 · 96e53d4
1 parent 6e64f08
commit 96e53d4
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 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
@@ -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
@@ -208,4 +208,42 @@ 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).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();
+    }
 }