Skip to content

Introduce record_testsuite_property fixture#5205

Merged
nicoddemus merged 1 commit into
pytest-dev:featuresfrom
nicoddemus:record-testsuite-property
May 11, 2019
Merged

Introduce record_testsuite_property fixture#5205
nicoddemus merged 1 commit into
pytest-dev:featuresfrom
nicoddemus:record-testsuite-property

Conversation

@nicoddemus

@nicoddemus nicoddemus commented May 3, 2019

Copy link
Copy Markdown
Member

This exposes the functionality introduced in fa6acdc as a session-scoped fixture.

Plugins that want to remain compatible with the xunit2
standard should use this fixture instead of record_property.

Fix #5202

@Zac-HD Hypothesis might want to use this fixture to produce xunit2-compatible reports according to #5202.

cc @danilomendesdias

@codecov

codecov Bot commented May 3, 2019

Copy link
Copy Markdown

Codecov Report

Merging #5205 into features will decrease coverage by 0.15%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           features    #5205      +/-   ##
============================================
- Coverage     91.49%   91.34%   -0.16%     
============================================
  Files           115      115              
  Lines         26138    26174      +36     
  Branches       2578     2580       +2     
============================================
- Hits          23915    23908       -7     
- Misses         1897     1945      +48     
+ Partials        326      321       -5
Impacted Files Coverage Δ
src/_pytest/junitxml.py 95.84% <100%> (+0.2%) ⬆️
testing/test_junitxml.py 97.94% <100%> (+0.06%) ⬆️
testing/python/fixtures.py 83.67% <0%> (-14.85%) ⬇️
testing/python/collect.py 89.55% <0%> (-9.39%) ⬇️
testing/python/raises.py 88.51% <0%> (-6.09%) ⬇️
testing/python/integration.py 89.36% <0%> (-2.13%) ⬇️
src/_pytest/fixtures.py 97.79% <0%> (-0.14%) ⬇️
src/_pytest/terminal.py 93.42% <0%> (+1.67%) ⬆️
testing/python/setup_only.py 100% <0%> (+7.14%) ⬆️
testing/python/metafunc.py 95.23% <0%> (+8.05%) ⬆️
... and 1 more

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 3a4a815...73bbff2. Read the comment docs.

Comment thread doc/en/usage.rst
@nicoddemus nicoddemus added this to the 4.5 milestone May 7, 2019
@nicoddemus

Copy link
Copy Markdown
Member Author

Any reviewers? I would like to get this out in 4.5 if possible.

@Zac-HD

Zac-HD commented May 9, 2019

Copy link
Copy Markdown
Member

Happy to do a review, but I don't fully understand what this is meant to do!

  • I'd need considerably more documentation to be able to use it. For example, what is the range of valid input for name and value? Must both be strings, can value be a dict, etc.
  • It would be great if the fixture function could actually enforce this, e.g.
xml = getattr(request.config, "_xml", None)
if xml is None:
    def record_func(name, value):
        """noop function in case --junitxml was not passed in the command-line"""

else:
    def record_func(name, value):
        if not isinstance(name, str):
            raise ...
        xml.add_global_property(name, value)

return record_func

This might just be Hypothesis-level nitpicking, but it's what I'm used to - and I can't see any obvious problems if that helps 😄

@nicoddemus

Copy link
Copy Markdown
Member Author

This might just be Hypothesis-level nitpicking, but it's what I'm used to - and I can't see any obvious problems if that helps

Not at all! All good suggestions.

I will get to them later. 👍

This exposes the functionality introduced in fa6acdc as a session-scoped fixture.

Plugins that want to remain compatible with the `xunit2`
standard should use this fixture instead of `record_property`.

Fix pytest-dev#5202
@nicoddemus nicoddemus force-pushed the record-testsuite-property branch from b27275e to 73bbff2 Compare May 10, 2019 22:45
@nicoddemus

Copy link
Copy Markdown
Member Author

@Zac-HD done! Please let me know if you have any other suggestions/comments. 👍

@Zac-HD Zac-HD left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

With the caveat that I haven't used the XML reporting feature, this looks good to me 👍

@nicoddemus nicoddemus merged commit 184ef92 into pytest-dev:features May 11, 2019
@nicoddemus nicoddemus deleted the record-testsuite-property branch May 11, 2019 16:27
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.

3 participants