Apply --ignore-regex to the TOTAL row of diff reports - #202
Open
Eljees wants to merge 1 commit into
Open
Conversation
--ignore-regex removes files from the body of a diff report, but the TOTAL row is still computed over every file, so the totals do not match the rows above them. With --ignore-regex '.*' the table is empty and TOTAL still reads -1 -4 +31.06%. DeltaReporter.get_summary_lines() calls diff_total_statements(), diff_total_misses() and diff_line_rate() without passing self.ignore_regex, and those three plus _diff_attr had no such parameter to pass it to. The underlying Cobertura.line_rate(), total_statements(), total_misses() and total_hits() already accept it. This threads ignore_regex through, which fixes the TOTAL row for every delta format (text, csv, markdown, json, yaml, html), since they all share get_summary_lines(). The show command was given the same treatment in aconrad#176. Adds one CLI test and two reporter tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
--ignore-regexfilters files out of the body of adiffreport, but theTOTALrow is still computed over every file, so the totals disagree withthe rows printed above them. Ignoring everything makes it obvious:
The table is empty, and
TOTALstill reports the numbers for the files thatwere supposed to be excluded.
With this change:
Why this looks like a bug rather than intended behaviour
showalready behaves the other way: Fix total values when ignore regex used #176 threadedignore_regexinto thetotal calculations for
showand was released in 3.3.1, andtest_text_report__with_ignore_regexpins that behaviour.diffwas leftout of that change.
Cobertura.line_rate(),total_statements(),total_misses()andtotal_hits()all takeignore_regex. Only theCoberturaDiffcounterparts do not.The change
DeltaReporter.get_summary_lines()callsdiff_total_statements(),diff_total_misses()anddiff_line_rate()without passingself.ignore_regex, and those three (plus the_diff_attrhelper behindthem) had no parameter to receive it. This threads
ignore_regexthrough.Because every delta format goes through the same
get_summary_lines(), thisfixes the TOTAL row for text, csv, markdown, json, yaml and html at once.
Tests
One CLI test and two reporter tests. Without the production change they fail
on the numbers, not on the shape of the output:
I also checked that both halves of the change are load-bearing by breaking
each one on its own and confirming the same three tests go red: reverting the
three call sites in
reporters.pygives 187/3, and reverting only the fileselection in
_diff_attrgives 187/3 as well.flake8 pycobertura/andblack --check --diff pycobertura/(the two toxgates) both pass.