Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR 1091 generates changes in junitxml output which cause incompatibility with Jenkins Junit plugin #2228

Closed
KKoukiou opened this issue Feb 2, 2017 · 7 comments
Labels
plugin: junitxml related to the junitxml builtin plugin type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog type: enhancement new feature or API change, should be merged into features branch type: feature-branch new feature or API change, should be merged into features branch type: regression indicates a problem that was introduced in a release which was working previously

Comments

@KKoukiou
Copy link
Contributor

KKoukiou commented Feb 2, 2017

Let's consider testcase that fails in the call thus resulting in 1 failure
and then when it's teardown fixture is executed it fails there as well
thus resulting in 1 error. So we have 1 failure and 1 error for the
same test. This results in the junitxml XML report, as two different
attributes, one error and one failure for the same testcase.

However, until pytest 2.8.4, in such tests the XML file had two
testcase elements, one containing the failure in the call of the test,
and a second testcase element containing the error in the teardown.
From pytest 2.8.5 and after the XML file has only one testcase element
for such test, and this contains both fail and error sections.

The problem with the new XML files is that they cannot
be correctly parsed by Jenkins Junit plugin. The reason behind that is
that they violate the JUnit XML format, which is built so that in
every testcase element you can have as a child only one of {skipped,
error, failure, system-out, system-err} elements.

I would like your opinion on that, and if you agree on reverting the old style
XML reports I would be happy to send a PR.
@RonnyPfannschmidt

EXAMPLE:
The following code will have difference in the XML report between
pytest versions.

@pytest.fixture()
def setup_class(request):
    def finalize():
        raise Exception("test teardown Exception!")
    request.addfinalizer(finalize)

def test_1(setup_class):
    raise Exception("test main Exception!")

pytest version >= 2.4.5

<testsuite errors="1" failures="1" name="pytest" skips="0" tests="1"
time="0.009">
    <testcase classname="test" file="test.py" line="9" name="test_1"
time="0.000397205352783">
        <failure message="Exception: test main Exception!">
         .. some info...
        </failure>
        <error message="test setup failure">
         ..some info about teardown exception...
        </error>
    </testcase>
</testsuite>

pytest version < 2.4.5

<testsuite errors="1" failures="1" name="pytest" skips="0" tests="1"
time="0.009">
    <testcase classname="test" file="test.py" line="9" name="test_1"
time="0.000282049179077">
        <failure message="Exception: test main Exception!">
         .. some info...
        </failure>
    </testcase>
    <testcase classname="test" file="test.py" line="9" name="test_1"
time="0.00037407875061">
        <error message="test setup failure">
         .. some info...
        </error>
    </testcase>
 </testsuite>
@KKoukiou KKoukiou changed the title https://github.com/pytest-dev/pytest/pull/1091/ generates changes in junitxml output which cause incompatibility with Jenkins Junit plugin PR 1091 generates changes in junitxml output which cause incompatibility with Jenkins Junit plugin Feb 2, 2017
@RonnyPfannschmidt
Copy link
Member

for reference #1091

@nicoddemus
Copy link
Member

(Hi @KKoukiou, edited your post to improve on the formatting)

The problem with the new XML files is that they cannot
be correctly parsed by Jenkins Junit plugin.

Why do you say that? We use Jenkins at work and it parses the JUnit file generated by pytest just fine.

I think we followed this schema when implementing the PR, and it seems to support multiple failures/errors etc per test case (L71):

<xs:element name="testcase" minOccurs="0" maxOccurs="unbounded">
  <xs:complexType>
    <xs:choice minOccurs="0">
      <xs:element name="error">
      ...
      </xs:element>
      <xs:element name="failure">                        

@KKoukiou
Copy link
Contributor Author

KKoukiou commented Feb 2, 2017

@nicoddemus The link you sent me actually confirms that failure and error are mutually exclusive. You can have multiple errors/failures but not error and failure together. Excuse me if I didn't make myself clear earlier.
From the docs: >> XML Schema choice element allows only one of the elements contained in the declaration to be present within the containing element.

Regarding to the Jenkins, you can notice that when the junitxml XML file has for a single testcase error and failure sections, only the error sections in shown in the Jenkins TestReport.

@nicoddemus
Copy link
Member

From the docs: >> XML Schema choice element allows only one of the elements contained in the declaration to be present within the containing element.

Oh my schema-fu is rusty then, thanks for the pointer.

Regarding to the Jenkins, you can notice that when the junitxml XML file has for a single testcase error and failure sections, only the error sections in shown in the Jenkins TestReport.

That might explain then why I never noticed it myself.

I would be happy to send a PR.

A PR to the features branch would be greatly appreciated then, thanks!

@nicoddemus nicoddemus added plugin: junitxml related to the junitxml builtin plugin and removed plugin: junitxml related to the junitxml builtin plugin labels Feb 2, 2017
@RonnyPfannschmidt RonnyPfannschmidt added type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog type: feature-branch new feature or API change, should be merged into features branch type: regression indicates a problem that was introduced in a release which was working previously labels Feb 2, 2017
@KKoukiou
Copy link
Contributor Author

KKoukiou commented Feb 3, 2017

@RonnyPfannschmidt @nicoddemus

I was thinking about how to implement the change and I need some feedback on that:
Before v2.8.4 for testcases that failed on call and then on teardown there where two elements
for the same testcase in the XML, which looked like this:

    <testcase classname="test" file="test.py" line="9" name="test_1"
time="0.000282049179077">
        <failure message="Exception: test main Exception!">
         .. some info...
        </failure>
    </testcase>
    <testcase classname="test" file="test.py" line="9" name="test_1"
time="0.00037407875061">
        <error message="test setup failure">
         .. some info...
        </error>
    </testcase>

Though, I don't find appropriate to have two testcase elements with the same name.
I would rather see the testcase element with failure in the teardown as following:

    <testcase classname="test" file="test.py" line="9" name="test_1"
time="0.000282049179077">
        <failure message="Exception: test main Exception!">
         .. some info...
        </failure>
    </testcase>
    <testcase classname="test" file="test.py" line="9" name="test_1.teardown"
time="0.00037407875061">
        <error message="test setup failure">
         .. some info...
        </error>
    </testcase>

So what I suggest, is that for the testcases that fail in setup, or teardown we should have testcase elements with the regarding suffix in the test name (ex: test_1.teardown), and not just have duplicate testcase elements.
Please let me know what you think!

@RonnyPfannschmidt
Copy link
Member

both cases look good

while i think the case with the new name is more informative, i it will throw off peoples tooling
after all they will get a new test name thats either a failure or missing,

as such im under the impression we will have to go for the same name variant (which us quite unfortunate)

@nicoddemus
Copy link
Member

I agree with @RonnyPfannschmidt, it might throw people's tooling off, better use the same name.

This was referenced Mar 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: junitxml related to the junitxml builtin plugin type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog type: enhancement new feature or API change, should be merged into features branch type: feature-branch new feature or API change, should be merged into features branch type: regression indicates a problem that was introduced in a release which was working previously
Projects
None yet
Development

No branches or pull requests

3 participants