-
Notifications
You must be signed in to change notification settings - Fork 52
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
implement flavors #108
implement flavors #108
Conversation
junitparser/flavors/pytest_xml.py
Outdated
SystemOut, SystemErr, TestCase as OrigTestCase, System | ||
|
||
|
||
class PytestXml(JUnitXml): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe name it to JUnitXml
as well. Could result in less cognitive load. Flavors are separated by file names already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this should be
class PytestXml(JUnitXml): | |
class JUnitXml(junitparser.JUnitXml): |
Then user code looks identical, no matter which import they use:
from junitparser.junitparser import JUnitXml
from junitparser.flavors.pytest_xml import JUnitXml
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #108 +/- ##
==========================================
+ Coverage 97.96% 98.24% +0.27%
==========================================
Files 6 8 +2
Lines 1231 1424 +193
==========================================
+ Hits 1206 1399 +193
Misses 25 25
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of using packages to select the flavour, re-exporting unchanged classes and deriving changed classes.
I think the xml
is redundant in the pytest_xml
flavour name.
junitparser/flavors/pytest_xml.py
Outdated
SystemOut, SystemErr, TestCase as OrigTestCase, System | ||
|
||
|
||
class PytestXml(JUnitXml): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this should be
class PytestXml(JUnitXml): | |
class JUnitXml(junitparser.JUnitXml): |
Then user code looks identical, no matter which import they use:
from junitparser.junitparser import JUnitXml
from junitparser.flavors.pytest_xml import JUnitXml
tests/test_pytest_xml.py
Outdated
# -*- coding: utf-8 -*- | ||
|
||
import unittest | ||
from junitparser.flavors.pytest_xml import PytestXml, TestSuite, TestCase, FlakyError, FlakyFailure, RerunError, RerunFailure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer
from junitparser.pytest_xml import ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Less nesting is good. Keeping the root clean may be also worth it. Let me think this over.
e53c43b
to
0bf31a1
Compare
junitparser/junitparser.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No code change. Just that orders need to be adjusted for the type hints to work.
tests/test_general.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Format change curtesy of black.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
This is to address #101, as well as other frequent xml schema support requirements.
I've thought about
JunitXml(flavor='pytest')
, but typings will be tricky when it comes to dealing with elements likeTestSuite
andTestCase
, which will also be different with those in the "standard" implementation, For example, autocompletion won't know thisTestCase
now has arerun_errors()
method.With implementation in this PR, a flavor and its elements are imported and used separately, and autocompletion works on each case.
Also address #97 by adding rudimentary types