LocalCI: Nested Testsuites are handled incorrectly

b-fein commented 2 weeks ago

Describe the bug

JUnit-XMLs support nested test suited. This is for example extensively used by the Haskell template.

Before, if there was a nested test suite like:

<testsuite name="Properties">
    <testsuite name="Checked by QuickCheck">
        <testcase name="Test A against sample solution"/>
    </testsuite>
</testsuite>

the resulting test name was Properties.Checked by QuickCheck.Testing A against sample solution. Currently it is only Test A against sample solution.

The old behaviour should be restored.

To Reproduce

Create a new Haskell exercise.
The exercise description contains test cases like Properties.Checked by QuickCheck.Testing A against sample solution.

However, Artemis only receives test cases with the partial name after the last dot like Testing A against sample solution.

Expected behavior

The full test name including the names of the test suites is retained. The original name as in the exercise template is returned as test case name to Artemis. This restores the old behaviour.

Keeping only the last section of the name and adapting the template is not the preferred solution:

Two different exercise descriptions for setups using LocalCI and Jenkins would have to be defined.
The old behaviour that concatenated the test suite + final test names with a . in between resulted in unique test cases. This can no longer be ensured when using only the last part. Tests in different test suites are allowed to have the same name.
Existing exercises created in Bamboo, before 7.6.1, or in Jenkins setups may depend on the original behaviour.

Screenshots

No response

Which version of Artemis are you seeing the problem on?

7.6.5

What browsers are you seeing the problem on?

Firefox

Additional context

The flattening of Testsuite objects has been implemented already in the Jenkins plugin: https://github.com/ls1intum/jenkins-server-notification-plugin/blob/798ffccb8d55186e468ffedb552cf12f43e1855e/src/main/java/de/tum/in/www1/jenkins/notifications/model/Testsuite.java#L98

It can potentially be ported over to Artemis to work the same way when parsing results of LocalCI jobs.

Relevant previous PR: #9490

Relevant log output

No response

b-fein commented 2 weeks ago

/cc @magaupp

magaupp commented 2 weeks ago

The Jenkins implementation seems to leave out the name of the top-level testsuite. Is this intentional?

b-fein commented 2 weeks ago

Yes, I think so. This is consistent with how the tests for a non-nested test suite are parsed. In that case the name of the top-level (and only level) test suite is also not part of the test case name.

For reference, an example test suite produced by the standard Haskell template:

<?xml version='1.0' ?>
<testsuites errors="0" failures="0" tests="12" time="0.007">
    <testsuite name="Tests" tests="12">
        <testsuite name="Properties" tests="9">
            <testsuite name="Checked by SmallCheck" tests="6">
                <testcase name="Testing filtering in A" time="0.003" classname="Tests.Properties.Checked by SmallCheck" />
                <testcase name="Testing mapping in A" time="0.000" classname="Tests.Properties.Checked by SmallCheck" />
                <testcase name="Testing filtering in B" time="0.001" classname="Tests.Properties.Checked by SmallCheck" />
                <testcase name="Testing mapping in B" time="0.000" classname="Tests.Properties.Checked by SmallCheck" />
                <testcase name="Testing filtering in C" time="0.000" classname="Tests.Properties.Checked by SmallCheck" />
                <testcase name="Testing mapping in C" time="0.000" classname="Tests.Properties.Checked by SmallCheck" />
            </testsuite>
            <testsuite name="Checked by QuickCheck" tests="3">
                <testcase name="Testing A against sample solution" time="0.000" classname="Tests.Properties.Checked by QuickCheck" />
                <testcase name="Testing B against sample solution" time="0.000" classname="Tests.Properties.Checked by QuickCheck" />
                <testcase name="Testing C against sample solution" time="0.000" classname="Tests.Properties.Checked by QuickCheck" />
            </testsuite>
        </testsuite>
        <testsuite name="Unit Tests" tests="3">
            <testcase name="Testing selectAndReflectA (0,0) []" time="0.000" classname="Tests.Unit Tests" />
            <testcase name="Testing selectAndReflectB (0,1) [(0,0)]" time="0.000" classname="Tests.Unit Tests" />
            <testcase name="Testing selectAndReflectC (0,1) [(-1,-1)]" time="0.000" classname="Tests.Unit Tests" />
        </testsuite>
    </testsuite>
</testsuites>

and after removing the top-level <testsuites>:

<?xml version='1.0' ?>
<testsuite name="Tests" tests="12">
    <testsuite name="Properties" tests="9">
        <testsuite name="Checked by SmallCheck" tests="6">
            <testcase name="Testing filtering in A" time="0.003" classname="Tests.Properties.Checked by SmallCheck" />
            <testcase name="Testing mapping in A" time="0.000" classname="Tests.Properties.Checked by SmallCheck" />
            <testcase name="Testing filtering in B" time="0.001" classname="Tests.Properties.Checked by SmallCheck" />
            <testcase name="Testing mapping in B" time="0.000" classname="Tests.Properties.Checked by SmallCheck" />
            <testcase name="Testing filtering in C" time="0.000" classname="Tests.Properties.Checked by SmallCheck" />
            <testcase name="Testing mapping in C" time="0.000" classname="Tests.Properties.Checked by SmallCheck" />
        </testsuite>
        <testsuite name="Checked by QuickCheck" tests="3">
            <testcase name="Testing A against sample solution" time="0.000" classname="Tests.Properties.Checked by QuickCheck" />
            <testcase name="Testing B against sample solution" time="0.000" classname="Tests.Properties.Checked by QuickCheck" />
            <testcase name="Testing C against sample solution" time="0.000" classname="Tests.Properties.Checked by QuickCheck" />
        </testsuite>
    </testsuite>
    <testsuite name="Unit Tests" tests="3">
        <testcase name="Testing selectAndReflectA (0,0) []" time="0.000" classname="Tests.Unit Tests" />
        <testcase name="Testing selectAndReflectB (0,1) [(0,0)]" time="0.000" classname="Tests.Unit Tests" />
        <testcase name="Testing selectAndReflectC (0,1) [(-1,-1)]" time="0.000" classname="Tests.Unit Tests" />
    </testsuite>
</testsuite>

magaupp commented 2 weeks ago

In this is the case, how should multiple top-level <testsuite> elements in a <testsuites> be handled?

b-fein commented 2 weeks ago

At the moment I think neither the Jenkins plugin nor Artemis LocalCI handle that case, since all CI scripts remove the <testsuites> tag if present using sed -i 's/<testsuites[^>]*>//g ; s/<\\/testsuites>//g'.

I think for now it would be fine to assume only a single top-level test suite exists, like is the case currently.

In the future, we could instead extend the parser to handle <testsuites> elements properly and only if there are multiple <testsuite>s within also add the name of the top-most level <testsuite> as a prefix? If there is a single <testsuite> within a <testsuites> it would not be required to ensure test case name uniqueness.

ls1intum / Artemis