Bugfix: junitxml violates Jenkins/xUnit JUnit schema #4493
Conversation
* Remove non-standard testcase elements: 'file' and 'line' * Replace testcase element 'skips' with 'skipped' * Time resolution uses the standard format: 0.000 * Tests use corrected XML output with proper attributes
jhunkeler
force-pushed
the
junit-compat
branch
from
79f1163 to
c18f40d
Compare
Codecov Report
@@ Coverage Diff @@
## master #4493 +/- ##
==========================================
- Coverage 95.9% 95.57% -0.34%
==========================================
Files 111 111
Lines 25085 25076 -9
Branches 2447 2443 -4
==========================================
- Hits 24059 23966 -93
- Misses 724 797 +73
- Partials 302 313 +11
Continue to review full report at Codecov.
|
| @@ -110,20 +110,14 @@ def record_testreport(self, testreport): | |||
| classnames = names[:-1] | |||
| if self.xml.prefix: | |||
| classnames.insert(0, self.xml.prefix) | |||
| attrs = { | |||
| "classname": ".".join(classnames), | |||
| "name": bin_xml_escape(names[-1]), | |||
There was a problem hiding this comment.
Unfortunately this one iis a breaking change, well have to sort out the details
There was a problem hiding this comment.
Please elaborate. What does it break?
There was a problem hiding this comment.
@jhunkeler off hand we cant rule out that people rely on the invalid extra attributes we output
There was a problem hiding this comment.
or to elaborate - junitxml output has been brokenly incorrect for so long that we now are likely to break other peoples code with the fix and we have to account for that in some way (be it a major release for this fix or a opt-in for correct behaviour that will later be a opt out and finally be phased out
There was a problem hiding this comment.
How about creating an pytest.ini option to configure this then? junitxml_strict=true?
There was a problem hiding this comment.
And this would need to be based on features btw.
| } | ||
| if testreport.location[1] is not None: | ||
| attrs["line"] = testreport.location[1] | ||
| attrs = {"classname": ".".join(classnames), "name": bin_xml_escape(names[-1])} | ||
| if hasattr(testreport, "url"): | ||
| attrs["url"] = testreport.url |
There was a problem hiding this comment.
Note to self: url cannot be present in testcase either.
|
i'd like to note that its painful to leave a good change like this in the limbo over release management and compatibility shenanigans my correctness hat would like to see this in asap, but with my maintenance/compatibility hat on i have to assess impact on potential users of the wrong things as well |
An option would fit this bill I guess? |
|
@nicoddemus correct - your idea about an option would be nice - but based on the diff i fear that it might generate a unjust/unreasonable maintenance burden with trickiness for both @jhunkeler as the person creating the pr as well as for the pytest core team that will keep on maintaining the junitxml plugin i'd like to get @jhunkeler 's input as his situation (aka junitxml breaks with modern plugin versions is certainly absolutely unacceptable and should be sorted out asap |
|
We're dead in the water so to speak. In the worst case I could temporarily impose the changes I've made by forcing our build/test infrastructure to install I've started to implement a variation of what @nicoddemus has suggested. I feel like this won't be the last time this will come up. Maybe not now, however, in a few years they could change their schema again and we'll find ourselves in the same situation. Short of verifying XSDs directly using Maintaining a test suite for all of these subtle modifications, on the other hand, will be horrendous. |
|
@jhunkeler a while back i had some initial contact about bringing efforts together with one of the maintainers of the junitxml reporter for unittests i believe that the junitxml format integration and generators should eventually move to a own package that can sort out the details of junitxml format versions |
|
superseded by #4511 |
At my institution we use Jenkins and
xUnitto record test results created withpytest. DowngradingxUnitandJUnitto a version that uses an older schema has become impossible, due to security vulnerabilities, and plugin compatibility with the latest Jenkins releases. This schema problem has been a known issue for a while judging from the chatter in a few issues here.My point is, we can't ingest test results from our pipelines at all. There's no going back.
This PR should take care of the problem.
Related: #1126 #3808
Results (mine)
XML output example:
Validation:
Jenkins xUnit plugin