fix(test-runner-junit-reporter): standardize JUnit format

[crdrost]

There is no single standard XML format for JUnit output, sadly. However there are some XML Schema Definition (XSD) files which are circulated defining a couple of schemas which Jenkins plugins etc. know about and can validate. This change brings the test-runner-junit-reporter output into conformance with one, @jenkinssci/xunit-plugin::junit-10.xsd, which it seemed "closest to". (XSD permits <testsuites> tags and each individual <testsuite> has an id attribute and a skipped count, but no package attribute, like the foregoing junit-reporter output.)

To make this change requires two semantic changes:

• junit-10.xsd does not permit the file attribute on <testcase>. This is no great loss of information, as it is recapitulated a level above on the <testsuite>.

• After aggregating results.reduce(addSuiteTime, 0) this change has to perform a Number.prototype.toFixed() reduction in precision to match the XSD. This change is bubbled up into the data types, which store the number as a string now.

[changeset-bot]

🦋 Changeset detected

Latest commit: 80c0bbe

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@web/test-runner-junit-reporter	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[bennypowers]

1. is this a major change?
2. will this break ci for a bunch of users? for example this run which uses junit reporter github action
3. Can these be made opt in via some reportStyle config or something like that?

[crdrost]

Change scope: I guess I considered it a fix-level change but since the reporter is v0.x.x that would put it as a patch change. We could make it a minor change if you prefer, bumping to 0.6.0 rather than 0.5.1 -- put that into the new version of the commit

Does this break CI?: If it did I'd personally consider it an error in the CI tool, CI tools should not break if nonstandard fields are not there, since the vast vast majority of junit-xml-returning packages won't produce a file= on the testcase.

But in the particular case you are looking at, this line says:

    testcase._attributes.file || (testsuite._attributes !== undefined ? testsuite._attributes.file : null),

so if the "file" does not exist on the testcase it'll look one level higher on the testsuite. I added a file= attribute to the testsuite element to provide this fallback in the latest version 80c0bbe.

Can this be made opt-in: I would push back on that but I can do it if you think it's a release blocker. Again if I just google search "example junit test xml" it's not there, if I'm looking at the junit5 legacy XmlReportWriter there's no file fields at all.

In xunit-plugin for Jenkins, they have a rather vast collection of schemas they accept, but the only other ones that have file fields at all are PHPUnit (which duplicates the info at both testsuite and testcase level, see e.g. the text.xml.txt file on this issue). I'd be okay with deleting the id= properties to make the PHPUnit schemas validate too, I just want one or another standard format to validate :)

[Westbrook]

I'd need more time to confirm what this format required to continue to support Circle CI the way that it does, but these other formatters also support Circle CI, like these, so I'd take a comparison to 1 or 2 of those as a "this will work test". I likely don't have time this week, so @crdrost if you were able to do a quick compare and make sure we're not cutting off the nose, as it were, I'm not opposed to just moving away from this data point in the name of conformity... hard to get a full coverage of CI tools this way, but if GH Action is happy (@bennypowers's tool of choice here) and CircleCi is happy (my tool of choice here), then we're all happy 😉

[crdrost]

I was able to look up some XML examples for the various frameworks mentioned from there and here's a quick compatibility table:

framework	phpunit-4.0.xsd	junit-10.xsd	file attributes?
eslint-junit-formatter	✅	❌	none
minitest-ci	✅	❌	on testcase only
jest-junit	❌	✅	none
rspec_junit_formatter	❌	❌	on testcase only
mocha-junit-formatter	❌	❌	on testsuite only
karma-junit-formatter	❌	✅	none
tap-xunit	✅	✅	none
pytest	❌	✅	none

As you can see, most frameworks don't include file on testcase, minitest-ci does but that's because they're fully PHPUnit compliant, whereas mocha-junit-formatter and rspec_junit_formatter are just kind of generally weird. (In particular, mocha-junit-formatter actually distributes XSDs which their tests do not validate against... thankfully they appear to have a validating antMode: true option.)

Could also, as #2072 mentions, think about making the thing compliant with the PHPUnit schema, this change just seemed easier. Could also just omit the testsuite file= declaration if you want.

[jasonmobley]

I'm interested in this as well @crdrost. Can we resurrect this PR? Looks like there are conflicts to be resolved.

@Westbrook did @crdrost's last comment address your request for comparison to other tools?

[JackRobards]

Bumping as I would also be interested in this change. @Westbrook (or maybe @bashmish @thepassle seem like more recent maintainers) do you all have any concerns with this PR and comparison that @crdrost did?

I'd also be happy to help update this PR or create a new to resolve the conflicts that have popped up since this was last updated if needed!