-
Notifications
You must be signed in to change notification settings - Fork 7
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
Adds unit-test xml report generation task #212
base: develop
Are you sure you want to change the base?
Conversation
97f64cc
to
b066d49
Compare
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!
@task(pre=[unittest]) | ||
def unittest_xml_coverage(context): | ||
"""Produce an XML coverage report that can be ingested into other tools.""" | ||
produce_xml_cmd = "coverage xml --include '{{ cookiecutter.app_name }}/*' --omit '*/migrations/*,*/tests/*'" |
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 wonder if it would be preferable not to omit migrations
(since we should be sure to test data migrations if possible) and tests
(since it would flag cases of dead test code that never gets called, etc.)
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.
Took in changes in latest push
cur_dir = os.path.dirname(__file__) | ||
cov_xml_path = os.path.join(cur_dir, "coverage.xml") | ||
|
||
# I was originally going to use ET lib but linter throws a CVE warning |
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.
Should be fine to use ElementTree and just add an appropriate noqa
comment, IMHO.
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.
Okay latest commit i went back to using ET
b066d49
to
497732b
Compare
What's Changed
coverage.xml
). This can be used by tooling (such as VSCode code coverage extensions) for ingestion.Of note that the XML has to be modified to work correctly with tooling outside the container. I first tried to use the std-lib xml to do this but received aruff
CVE warning. I didn't want to pull in a new dependency here, so I did the hacky thing to just manipulate the file with string replacement. Open to feedback whether that was the right call or not.Earlier commit i worked around a CVE in XML but after feedback decided it was acceptable for dev environments and went back to using the stdlib xml helpers.
Testing
Tested this logic in the floor-plan app while I was reviewing a PR and it worked fine.