name property on <testsuites> element in JUnit XML logfile

[joseph-sentry]

This commit adds and sets the name property on the testsuites element of the generated JUnit report to 'PHPUnit tests'. This is in line with behaviour in Jest and Vitest where the tools set that property to their respective names.

This is helpful for processing JUnit XMLs depending on the framework that
generated the file.

[sebastianbergmann]

Can you provide links to Jest and Vitest examples? Thanks.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.61%. Comparing base (93961d3) to head (5f1ed86).
Report is 13 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #5931   +/-   ##
=========================================
  Coverage     94.61%   94.61%           
  Complexity     6673     6673           
=========================================
  Files           709      709           
  Lines         20138    20139    +1     
=========================================
+ Hits          19054    19055    +1     
  Misses         1084     1084           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[joseph-sentry]

Jest doesn't seem to have an example file in their repo but their README contains this example:

<testsuites name="jest tests">
  <testsuite name="addition" tests="1" errors="0" failures="0" skipped="0" timestamp="2017-07-13T09:45:42" time="0.154">
    <testcase classname="addition positive numbers" name="should add up" time="0.005">
    </testcase>
  </testsuite>
</testsuites>

here's what an example of vitest would look like, from this file in their repo:

<?xml version="1.0" encoding="UTF-8" ?>
<testsuites name="vitest tests" tests="1" failures="0" errors="0" time="...">
    <testsuite name="ok.test.ts" timestamp="..." hostname="..." tests="1" failures="0" errors="0" skipped="0" time="...">
        <testcase classname="ok.test.ts" file="ok.test.ts" name="ok" time="...">
        </testcase>
    </testsuite>
</testsuites>

in both cases, the property is customizable, which is where the behaviour of this PR differs from their implementations.

[sebastianbergmann]

Cherry-picked into 10.5 and merged to 11.3 and main from there.

[demiankatz]

@joseph-sentry, after upgrading to the latest PHPUnit release, the Jenkins xUnit plugin fails validation as a result of this change and now refuses to process my PHPUnit output:

INFO: [PHPUnit-4.x (default)] - 1 test report file(s) were found with the pattern 'build/vufind/reports/phpunit.xml' relative to '/opt/jenkins/jobs/VuFind/workspace' for the testing framework 'PHPUnit-4.x (default)'.
WARNING: The file '/opt/jenkins/jobs/VuFind/workspace/build/vufind/reports/phpunit.xml' is an invalid file.
WARNING: At line 2 of file:/opt/jenkins/jobs/VuFind/workspace/build/vufind/reports/phpunit.xml:cvc-complex-type.3.2.2: Attribute 'name' is not allowed to appear in element 'testsuites'.
WARNING: The result file '/opt/jenkins/jobs/VuFind/workspace/build/vufind/reports/phpunit.xml' for the metric 'PHPUnit' is not valid. The result file has been skipped.

I can do some post-processing to work around this, of course, and maybe this is a problem that needs to be taken to the xUnit plugin team rather than here -- but I wanted to mention it because I imagine I may not be the only person suddenly experiencing broken builds after this minor update, and it might be helpful to spread awareness. If this could be made configurable, that would be fantastic!

[sebastianbergmann]

I think I will just revert this. I do wonder what part of "eXtensible" these plugins do not understand ...

[joseph-sentry]

I think it makes sense to revert this right now. I will take a look at the xunit plugin and see if I can do anything there to make this possible without breaking users' setups. Sorry for the inconvenience!

[demiankatz]

Thanks, @joseph-sentry and @sebastianbergmann. I agree that it's strange that xUnit is so strict about validation! I'm certainly not opposed to this change in theory, if we can just find a way to make it coexist peacefully with existing tools. :-) I was able to change a setting to prevent it from totally breaking my builds, but it still refuses to publish my results since it won't try to read a file that fails its validation routine... but I haven't dug too deeply in on that side, so it's certainly possible I'm missing something! (And the fact that it still refers to PHPUnit 4 suggests that it probably hasn't gotten a lot of attention in a long time).