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

Add parser for Python coverage #103

Closed
wants to merge 1 commit into from
Closed

Add parser for Python coverage #103

wants to merge 1 commit into from

Conversation

pyieh
Copy link

@pyieh pyieh commented May 7, 2024

Adding a parser for Python coverage files.

Testing done

Testing was done in both with unit tests and a test coverage file (based on real coverage files). As well as on a local Jenkins, where a local version of the Jenkins coverage-plugin was installed to use this coverage model. Ran a simple Python pipeline and validated the coverage output was available in the UI.

Submitter checklist

@uhafner uhafner added the feature New features label May 8, 2024
<?xml version="1.0" ?>
<coverage version="6.5.0" timestamp="1710545957568" lines-valid="83" lines-covered="33" line-rate="0.3976" branches-covered="0" branches-valid="0" branch-rate="0" complexity="0">
<!-- Generated by coverage.py: https://coverage.readthedocs.io -->
<!-- Based on https://raw.githubusercontent.com/cobertura/web/master/htdocs/xml/coverage-04.dtd -->
Copy link
Member

Choose a reason for hiding this comment

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

Why can't we use the Cobertura parser to read such reports?

Copy link
Author

@pyieh pyieh May 13, 2024

Choose a reason for hiding this comment

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

We probably could actually. We've noticed from our internal testing of the default parser changes (documented here), that often times the Python parser will pick up Cobertura reports and vice versa.
Do you think the PythonParser should simply extend the CoberturaParser so as to differentiate when it's a Python project (vs Cobertura seems to be commonly used for Java projects)?

Copy link
Member

Choose a reason for hiding this comment

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

No. What I mean: we do not need another parser as the CoberturaParser reads all Cobertura reports. Is does not matter if the report is from Java, Python, or anything else.

Copy link
Author

Choose a reason for hiding this comment

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

I guess the UI in the Jenkins coverage-plugin, which uses this library, doesn't indicate which parser was used to parse and create the coverage model. For users that are generating coverage for their Python project, should we just have them use the Cobertura parser?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this is exactly what I mean. Why use a different parser if it does the same thing... Other non-Java teams do use this parser as well with great success.

If it helps we can add another enum value in the Jenkins plugin that provides a "Python Coverage" name, but still uses the existing parser from the model.

@uhafner uhafner marked this pull request as draft May 13, 2024 20:08
@uhafner
Copy link
Member

uhafner commented Jun 4, 2024

As mentioned in the review above, this parser does the same thing as the existing Cobertura parser.

@uhafner uhafner closed this Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants