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

In HTML output, use LightGray and '-' output when there are no branches #196

Merged
merged 1 commit into from Jan 17, 2018

Conversation

@mayeut
Copy link
Contributor

@mayeut mayeut commented Jan 15, 2018

Fixes #105

Copy link
Member

@latk latk left a comment

Thank you for this PR. It is good that you noticed the connection of this feature to the --fail-under-* options!

Your change to the calculate_coverage() function is a very elegant solution. If we already have this function, should it be used throughout the code? There are a few other places calculating a percentage, identified by searching for 100.0 *. If you want you can contribute that refactoring as another commit within this PR.

Loading


txt:
./testcase
../../../scripts/gcovr -d --fail-under-branch 100.0 -o coverage.txt
Copy link
Member

@latk latk Jan 16, 2018

Choose a reason for hiding this comment

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

Would it make sense to use the --branch report here, to test that it too can deal sensibly with 0/0 branch coverage?

Loading

Copy link
Contributor Author

@mayeut mayeut Jan 16, 2018

Choose a reason for hiding this comment

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

done. It makes perfect sense.

Loading

scripts/gcovr Outdated
coverage = calculate_coverage(branchCovered, branchTotal)
data['BRANCHES_COVERAGE'] = str(coverage)
coverage = calculate_coverage(branchCovered, branchTotal, zero_value=None)
data['BRANCHES_COVERAGE'] = '-' if coverage is None else str(coverage)
Copy link
Member

@latk latk Jan 16, 2018

Choose a reason for hiding this comment

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

can be simplified with ... = str(calculate_coverage(..., zero_value='-'))

Edit: sorry, please disregard that, the raw coverage is needed for the below coverage_to_color() call.

Loading

@mayeut
Copy link
Contributor Author

@mayeut mayeut commented Jan 16, 2018

I replaced code with calculate_coverage() everywhere I found 100.0 *
This helped identify one last place where branch coverage will display 0.0 % (still not corrected).
The summary (no test for that if I'm not mistaken):
./scripts/gcovr -s

------------------------------------------------------------------------------
                           GCC Code Coverage Report
Directory: .
------------------------------------------------------------------------------
File                                       Lines    Exec  Cover   Missing
------------------------------------------------------------------------------
------------------------------------------------------------------------------
TOTAL                                          0       0    --%
------------------------------------------------------------------------------
lines: 0.0% (0 out of 0)
branches: 0.0% (0 out of 0)

We might want to check 0 / 0 line coverage as well ?

Loading

latk
latk approved these changes Jan 16, 2018
@latk
Copy link
Member

@latk latk commented Jan 16, 2018

0/0 line coverage is going to be quite rare (can only happen for an empty report?) so I don't think it's worth the effort to investigate and fix this. Of course you can still look into this if you want.

In the summary report, replacing 0.0% with --% might look odd? I have no clear preference and leave this to your judgement.

Thank you for your effort so far. I'll try to merge this tomorrow, or whenever the Travis builds have caught up. 🐌

Loading

@mayeut
Copy link
Contributor Author

@mayeut mayeut commented Jan 17, 2018

I won't look into 0/0 line coverage. As you said it should not happen except for an empty report (TBC).
I have no preference as well for the summary report.

Loading

@latk latk merged commit 232fb11 into gcovr:master Jan 17, 2018
1 check passed
Loading
@mayeut mayeut deleted the no-branch branch Jan 17, 2018
@latk latk removed the needs review label Jan 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants