Skip to content

Include <testsuites> root tag in generated XML #5550

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

Merged

Conversation

nicoddemus
Copy link
Member

One thing that is not clear to me yet is if the attributes defined in the schema (name, title, etc) are optional:

    <xs:element name="testsuites">
        <xs:complexType>
            <xs:sequence>
                <xs:element ref="testsuite" minOccurs="0" maxOccurs="unbounded" />
            </xs:sequence>
            <xs:attribute name="name" type="xs:string" />
            <xs:attribute name="time" type="SUREFIRE_TIME"/>
            <xs:attribute name="tests" type="xs:string" />
            <xs:attribute name="failures" type="xs:string" />
            <xs:attribute name="errors" type="xs:string" />
        </xs:complexType>

Judging from the other attributes elsewhere which are defined in the schema but are not written (for example, testsuite's timestamp attribute), it seems those are optional.

Fix #5477

@nazariyv @jhunkeler @nazariydolfin

Would appreciate if you folks could help test the XML file produced by this patch and ensuring it still works with your tools (say Jenkins).

@codecov
Copy link

codecov bot commented Jul 4, 2019

Codecov Report

Merging #5550 into features will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           features    #5550      +/-   ##
============================================
+ Coverage      96.1%   96.11%   +<.01%     
============================================
  Files           117      117              
  Lines         25698    25712      +14     
  Branches       2494     2495       +1     
============================================
+ Hits          24697    24713      +16     
+ Misses          696      695       -1     
+ Partials        305      304       -1
Impacted Files Coverage Δ
testing/test_junitxml.py 98.14% <100%> (+0.03%) ⬆️
src/_pytest/junitxml.py 95.83% <100%> (+0.01%) ⬆️
src/_pytest/pastebin.py 91.22% <0%> (+3.5%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 60a358f...a43ba78. Read the comment docs.

@nazariyv
Copy link

nazariyv commented Jul 4, 2019

Will check out tomorrow! Thanks

@nazariydolfin
Copy link

I cannot run the tests on this branch:

item = <Function test_run_from_csv>

    def pytest_runtest_setup(item):
    
>       if StrictVersion(pytest.__version__) < StrictVersion("3.6"):

../../anaconda3/envs/invtools37/lib/python3.7/site-packages/pytest_remotedata/plugin.py:65: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../anaconda3/envs/invtools37/lib/python3.7/distutils/version.py:40: in __init__
    self.parse(vstring)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <[AttributeError("'StrictVersion' object has no attribute 'version'") raised in repr()] StrictVersion object at 0x117813f60>, vstring = '4.3.1.dev931+g7bef0038b'

    def parse (self, vstring):
        match = self.version_re.match(vstring)
        if not match:
>           raise ValueError("invalid version number '%s'" % vstring)
E           ValueError: invalid version number '4.3.1.dev931+g7bef0038b'

../../anaconda3/envs/invtools37/lib/python3.7/distutils/version.py:137: ValueError

@nazariydolfin
Copy link

ah, looks like this is meant to be for python 3.6? I will try that

@nazariydolfin
Copy link

Actually, I can't, because the build process is using python 3.7.

@nazariydolfin
Copy link

    tests_dir = os.path.dirname(os.path.abspath(__file__))
    tests_dir = os.path.join(tests_dir, start_dir)
    pytest.main(
        ["-x", tests_dir, "-o", "--junit_family=xunit2", "--junitxml=test_summary.xml"]
    )

doesn't create an .xml report with testsuites

@nazariydolfin
Copy link

It's because I have changed the environment and did not reinstall the lib.

@nazariydolfin
Copy link

still getting E ValueError: invalid version number '4.3.1.dev931+g7bef0038b'

@nicoddemus
Copy link
Member Author

@nazariydolfin

pytest_remotedata is checking the pytest version using StrictVersion, which doesn't allow for the development version number: 4.3.1.dev931+g7bef0038b. But that version number is odd, it should be 5.0.1.dev* or something.

Anyway you can fake the version by setting the SETUPTOOLS_SCM_PRETEND_VERSION=5.0.0 environment variable and installing pytest again. You should see pytest 5.0.0 as installed then when running pip list. 👍

@nazariydolfin
Copy link

this works for our Jenkins config. Thanks!

@nicoddemus nicoddemus force-pushed the testsuites-root-elem-5477 branch from 7bef003 to a43ba78 Compare July 5, 2019 14:29
Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, and apparently it solves the problem too 😄

Is there any we can run a validator that was failing before as part of the Pytest self-tests? That could help detect future regressions on other parts of the schema, and plugins could copy it to check their own use (e.g. IIRC Hypothesis does something iffy-to-the-schema with global properties).

Regardless, I'm happy for this to be merged.

@@ -75,7 +85,7 @@ def tag(self):
return self.__node.tagName

@property
def next_siebling(self):
def next_sibling(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this public API? If so we should note the change somewhere.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, it is used only on the test. 👍

@nazariydolfin
Copy link

When do you want to merge this?

@nicoddemus
Copy link
Member Author

When do you want to merge this?

Waiting to see if @nazariyv @jhunkeler want to take a look first.

I'm in no rush to merge this as it will only be available in pytest 5.1 anyway, and that's a few weeks off yet.

@nicoddemus
Copy link
Member Author

Is there any we can run a validator that was failing before as part of the Pytest self-tests?

Definitely, we plan to tackle this at some point: #5095

@nazariydolfin
Copy link

@nazariyv is also me :) This is my work github user. I guess I will add your branch into my requirements.txt for now.

@nicoddemus
Copy link
Member Author

@nazariyv is also me :) This is my work github user.

Oh, I noticed the first part of the name was similar. 😁

I guess I will add your branch into my requirements.txt for now.

Sure. After we merge this you can then add pytest@features to your requirements.txt.

@nicoddemus nicoddemus merged commit 0f8b462 into pytest-dev:features Jul 12, 2019
@nicoddemus nicoddemus deleted the testsuites-root-elem-5477 branch July 12, 2019 17:35
@KellyWalker
Copy link

We use pytest to test our company's products, but one of them is stuck using Python 2.7, which means that pytest 4.6.6 seems to be the latest available to us.

Our test reporting tool requires xunit 2.0 style output, as described here:
https://github.com/jenkinsci/xunit-plugin/blob/xunit-2.3.2/src/main/resources/org/jenkinsci/plugins/xunit/types/model/xsd/junit-10.xsd

According to the pytest docs, using config junit_family = xunit2 should do the trick since pytest 4.2:
https://docs.pytest.org/en/latest/reference.html

However, it seems there are bugs (at least this one but maybe others) that make the XML not match the expected style/format.

Would you consider making a patch release to pytest 4.6 to include this fix and any similar?

nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Nov 30, 2019
Include <testsuites> root tag in generated XML
@nicoddemus
Copy link
Member Author

Hi @KellyWalker,

Definitely, thanks for the ping, it was an oversight that this one was not backported.

Opened a PR with the backport: #6295

Let us know if you notice other bug-fixes that need to be backported. 👍

nicoddemus added a commit that referenced this pull request Nov 30, 2019
[4.6] Include <testsuites> root tag in generated XML (#5550)
@KellyWalker
Copy link

KellyWalker commented Nov 30, 2019

Awesome - thanks!

Will it be available in some future 4.6.7? What would be the timing of that release?

@nicoddemus
Copy link
Member Author

I plan to release mid this week 👍

@stalkerg
Copy link

stalkerg commented Mar 27, 2023

This PR break older Jenkins, do we have the option to revert to the original format?

@stalkerg
Copy link

@RonnyPfannschmidt this is a point - even for legacy it adds the top testsuites node, but I think shouldn't.

@nicoddemus
Copy link
Member Author

@stalkerg actually according to the specification, testsuites seems to be valid. Can you clarify why you think otherwise?

@stalkerg
Copy link

stalkerg commented Apr 5, 2023

@nicoddemus sorry, seems like it was an internal tool that aggregate test results and it depended on the older pytest format, we changed it and now everything is ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants