Change junitxml.py to produce results that comply with Junitxml schema #2236
Conversation
|
@nicoddemus @RonnyPfannschmidt Could you please CR and tell me if any changes are needed? |
|
Not sure what happened to Travis/Appveyor here. Closing/reopening to hopefully trigger them. |
There was a problem hiding this comment.
while reviewing the interaction with xdist i noticed that we cant tell, which slave a report is from,
as such it makes sense to at least do some tracking based on nodeid and item_index (item_index is optional from pytest-xdist)
i will report an xdist bug to add nodeid tracking as well
unfortunately currently the worker process id is not available on a report
_pytest/junitxml.py
Outdated
| @@ -337,6 +339,10 @@ def pytest_runtest_logreport(self, report): | |||
| reporter = self._opentestcase(report) | |||
| reporter.append_pass(report) | |||
| elif report.failed: | |||
| if self.last_report and self.last_report.failed: | |||
There was a problem hiding this comment.
this one will report a double fail if 2 xdist failures of a call happen at the same time
when reports from pytest-xdist come in, the report order is intermixed and cant be used as if it was linear
There was a problem hiding this comment.
Just to complement what @RonnyPfannschmidt said:
During normal pytest operation, report of setup/call/teardown all occur predictably because the tests are executed sequentially:
test1 setup
test1 call
test1 teardown
test2 setup
test2 call
test2 teardown
But using the pytest-xdist plugin this does not hold, because there are multiple workers running tests in parallel so the calls might end up intertwined in the master node, for example:
test1 setup
test1 call
test2 setup
test1 teardown
test2 call
test2 teardown
KKoukiou
force-pushed
the
junitxml-change-schema
branch
from
8287222
to
7b0837e
Compare
|
I reposted the PR with some slight change which I think will solve the issue with the xdist. I think it's not needed to check the worker_id, check on nodeid is enough for this purpose. @RonnyPfannschmidt @nicoddemus let me know what you think! |
|
@RonnyPfannschmidt @nicoddemus Did you manage to check the last commited PR for this? Thanks |
|
@KKoukiou i'm not seeing the use of the optional upcoming worker id, was your plan to implement that after a release? |
|
@RonnyPfannschmidt hi, I know we were thinking of using the worker_id, but the last patch revision solves the problem of non serialized testcase reports when using xdist, with keeping a list of open reports, and searching through them with nodeid, which is unique IIUC. |
|
|
KKoukiou
force-pushed
the
junitxml-change-schema
branch
4 times, most recently
from
1e975b5
to
439ce9a
Compare
|
I reposted the PR using node_id, worker_id and item_index for the checks. PS: The CI tests failed only on py37 nightly. |
|
looks good, the 3.7 failure is unrelated, i'm going to merge this one, a a followup with optimizations is optional, but would be appreciated |
|
oh, wait, i forgot about the changelog again, please add a note on this one in the cangelog, then @nicoddemus or me can merge |
KKoukiou
force-pushed
the
junitxml-change-schema
branch
from
439ce9a
to
43de660
Compare
Change XML file structure in the manner that failures in call and errors in teardown in one test will appear under separate testcase elements in the XML report.
KKoukiou
force-pushed
the
junitxml-change-schema
branch
from
43de660
to
26e50f1
Compare
|
@RonnyPfannschmidt what kind of optimizations? |
|
the travis fail is a python3.7 ast issue in the assertion rewriter, thus not relevant for this pr |
This patch is a solution to #2228