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

Allow recording custom xml attributes #3130

Closed
raphaelcastaneda opened this issue Jan 18, 2018 · 7 comments
Closed

Allow recording custom xml attributes #3130

raphaelcastaneda opened this issue Jan 18, 2018 · 7 comments
Labels
plugin: junitxml related to the junitxml builtin plugin type: enhancement new feature or API change, should be merged into features branch

Comments

@raphaelcastaneda
Copy link

raphaelcastaneda commented Jan 18, 2018

The experimental fixture record_xml_property inserts an additional <properties> element into the <testcase> element.

This violates typical junit xml validation, causing many parsers (e.g. Jenkins xunit plugin) to fail when processing pytest xml output that contains these extra properties.

While additional properties are very likely to break junit parsers, attributes may not.
For example, the junit spec does allow for an additional optional string attribute called assertions:

    <xs:element name="testcase">
        <xs:complexType>
            <xs:sequence>
                <xs:element ref="skipped" minOccurs="0" maxOccurs="1"/>
                <xs:element ref="error" minOccurs="0" maxOccurs="unbounded"/>
                <xs:element ref="failure" minOccurs="0" maxOccurs="unbounded"/>
                <xs:element ref="system-out" minOccurs="0" maxOccurs="unbounded"/>
                <xs:element ref="system-err" minOccurs="0" maxOccurs="unbounded"/>
            </xs:sequence>
            <xs:attribute name="name" type="xs:string" use="required"/>
            <xs:attribute name="assertions" type="xs:string" use="optional"/>
            <xs:attribute name="time" type="xs:string" use="optional"/>
            <xs:attribute name="classname" type="xs:string" use="optional"/>
            <xs:attribute name="status" type="xs:string" use="optional"/>
        </xs:complexType>
    </xs:element>

As a test author, I would like to be able to insert my own attributes on a testcase element.

Proposal:

  • Add an additional fixture called record_xml_attribute that allows a test author to add an attribute to the existing <testcase> element in the xml report.
  • This new fixture should take in two parameters: name and value
  • If a user specifies a name that overlaps with existing test case attributes, the user-specified values should overwrite the existing values.
  • User-specified attribute names should be xml-escaped
@pytestbot pytestbot added plugin: junitxml related to the junitxml builtin plugin type: enhancement new feature or API change, should be merged into features branch labels Jan 18, 2018
@nicoddemus
Copy link
Member

I believe #2770 address this.

@raphaelcastaneda
Copy link
Author

raphaelcastaneda commented Jan 18, 2018

@nicoddemus I might be missing something here, but I'm not sure that #2770 addresses this.
Part of the issue is that if a <testcase> element contains a <properties> element, then most parsers will refuse to treat the junit xml as valid.

#2770 makes the addition of custom properties easier (and seems to be agnostic to reporting format), but will likely still break output parsing for ci tools.

I'm working on a PR for this which might help clear up my intentions here.
However this example may demonstrate it as well:

Currently, an xunit file from a test that uses record_xml_property may look like this:

<?xml version="1.0"?>
<testsuite errors="0" failures="0" name="pytest" skips="0" tests="1" time="0.014">
  <testcase classname="test_fixtures.TestCustomMetadataInjection" file="test_fixtures.py" line="99" name="example" time="0.00146698951721">
    <properties>
      <property name="assertions" value="REQ-1234, REQ-5678, REQ-9101"/>
    </properties>
    <system-out>
hello world
</system-out>
  </testcase>
</testsuite>

What I'd like to do is this:

<?xml version="1.0"?>
<testsuite errors="0" failures="0" name="pytest" skips="0" tests="1" time="0.014">
  <testcase classname="test_fixtures.TestCustomMetadataInjection" file="test_fixtures.py" line="99" name="example" time="0.00146698951721" assertions="REQ-1234, REQ-5678, REQ-9101">
    <system-out>
hello world
</system-out>
  </testcase>
</testsuite>

raphaelcastaneda pushed a commit to raphaelcastaneda/pytest that referenced this issue Jan 19, 2018
raphaelcastaneda pushed a commit to raphaelcastaneda/pytest that referenced this issue Jan 19, 2018
raphaelcastaneda pushed a commit to raphaelcastaneda/pytest that referenced this issue Jan 19, 2018
update incorrect expected attribute value in test_record_attribute
raphaelcastaneda pushed a commit to raphaelcastaneda/pytest that referenced this issue Jan 19, 2018
update incorrect expected attribute value in test_record_attribute

attr names must be strings
raphaelcastaneda pushed a commit to raphaelcastaneda/pytest that referenced this issue Jan 19, 2018
update incorrect expected attribute value in test_record_attribute

attr names must be strings
@nicoddemus
Copy link
Member

Hi @raphaelcastaneda indeed they are not the same thing, thanks for the detailed explanation. I added some comments to your PR, please take a look when you get the time! 👍

@raphaelcastaneda
Copy link
Author

Thanks for looking this over @nicoddemus. I've updated the usage documentation. Please let me know if you'd like any additional changes.

@nicoddemus
Copy link
Member

Thanks @raphaelcastaneda. The PR LGTM and I've approved it, let's wait a few days to give the other maintainers a chance to review as well.

raphaelcastaneda pushed a commit to raphaelcastaneda/pytest that referenced this issue Jan 22, 2018
update incorrect expected attribute value in test_record_attribute

attr names must be strings

Update CHANGELOG formatting

update usage documentation

Fix versionadded for record_xml_attribute

Indent the xml schema properly inside the warning box in the docs
raphaelcastaneda pushed a commit to raphaelcastaneda/pytest that referenced this issue Jan 22, 2018
update incorrect expected attribute value in test_record_attribute

attr names must be strings

Update CHANGELOG formatting

update usage documentation

Fix versionadded for record_xml_attribute

Indent the xml schema properly inside the warning box in the docs
nicoddemus added a commit that referenced this issue Jan 29, 2018
…-attribute

implement #3130 - add record_xml_attribute fixture
@KKomarov
Copy link

@raphaelcastaneda thank you for your contribution! Do you plan to make it compatible with pytest-xdist?
https://github.com/pytest-dev/pytest-xdist/issues/538

@raphaelcastaneda
Copy link
Author

@raphaelcastaneda thank you for your contribution! Do you plan to make it compatible with pytest-xdist?
pytest-dev/pytest-xdist#538

I do not plan on implementing this. Though looking through the history of the project, I imagine a similar conversion is needed as was done for record_property (formerly record_xml_property). : 8b49ddf

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: enhancement new feature or API change, should be merged into features branch
Projects
None yet
Development

No branches or pull requests

4 participants