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

Fix total values when ignore regex used #176

Merged
merged 3 commits into from
Feb 17, 2024

Conversation

danctorres
Copy link
Contributor

@danctorres danctorres commented Feb 12, 2024

Expected Behavior:
The report should display the total values (stmts, miss, and cover), calculated for all files excluding those specified by the ignore_regex.

Actual Behavior:
Previously, when using ignore_regex to exclude files, the report's total values were incorrect. The values remained the same whether the ignore_regex argument was passed or not.

Steps to Reproduce:
~/pycobertura > pycobertura show --ignore-regex ".*Main|.*ISorted" --format markdown tests/cobertura.xml

Report Before Fix:

| Filename                 |   Stmts |   Miss | Cover   | Missing   |
|--------------------------|---------|--------|---------|-----------|
| search/BinarySearch.java |      12 |      1 | 91.67%  | 24        |
| search/LinearSearch.java |       7 |      2 | 71.43%  | 19-24     |
| TOTAL                    |      34 |      3 | 90.00%  |           |

Changes:

  • Added ignore_regex as an input to the functions responsible for calculating total values.
  • Introduced a test to ensure correct total value calculation when using ignore_regex.

Result:
After the fix, the total values are now calculated correctly, reflected by the following report, generated with the same command.

Report After Fix:

| Filename                 |   Stmts |   Miss | Cover   | Missing   |
|--------------------------|---------|--------|---------|-----------|
| search/BinarySearch.java |      12 |      1 | 91.67%  | 24        |
| search/LinearSearch.java |       7 |      2 | 71.43%  | 19-24     |
| TOTAL                    |      19 |      3 | 84.21%  |           |

@aconrad
Copy link
Owner

aconrad commented Feb 17, 2024

Thanks @danctorres, much appreciated! I'll take a look at it soon.

Can you update the PR description to show the before/after to better understand what you observed that looked wrong?

@danctorres
Copy link
Contributor Author

danctorres commented Feb 17, 2024

Thanks @danctorres, much appreciated! I'll take a look at it soon.

Can you update the PR description to show the before/after to better understand what you observed that looked wrong?

@aconrad updated the description, hope it helps

Copy link
Owner

@aconrad aconrad left a comment

Choose a reason for hiding this comment

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

Thanks @danctorres !!

@aconrad aconrad merged commit 283147c into aconrad:master Feb 17, 2024
5 checks passed
@aconrad
Copy link
Owner

aconrad commented Feb 17, 2024

v3.3.1

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.

2 participants