8000 Fix assert and key error in --decisions flag by klausweinbauer · Pull Request #642 · gcovr/gcovr · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix assert and key error in --decisions flag #642

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 15 commits into from
Jul 30, 2022
Merged

Fix assert and key error in --decisions flag #642

merged 15 commits into from
Jul 30, 2022

Conversation

klausweinbauer
Copy link
Contributor

This PR should fix issues with the --decisions flag generating assert count_false >= 0 and KeyError: 0. I encountered the same errors described in #637 and #640, which now works in my case. Let me know if there is still an issue.

Closes #637
Closes #640

@klausweinbauer
Fix branch keys
553aa8b 
Branch numbers do not always start with 0 and are consecutive. For
example, a move reference ( for(auto &&it : vector) ) has only branch 3
and 4.
8000
@klausweinbauer
Fix negative false decision counter
3722e3b
@klausweinbauer
Don't add nonexisting lines with decisions flag
120fe90
@klausweinbauer
Comply with styling convention
b1b69a0
@klausweinbauer
Add myself to authors
6c71b05
@klausweinbauer
Supplement changelog
60756ff
@klausweinbauer
Fix wrong isse reference in changelog
e7de6b9
@klausweinbauer
Remove changes for empty line generation
533a51c 
This change does not concern the primary goal of this PR to fix the
errors. My original change would remove empty line generation from
the decisions flag, which results in test case failures because of the
expected output references.
latk
latk approved these changes Jul 27, 2022
View reviewed changes
Copy link
Member
@latk latk left a comment

Choose a reason for hiding this comment

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

Thank you for working on this! As you've experienced, the decision coverage analysis is new and still has LOTS of bugs. Please continue to fix any issues you see. It's great that you identified these two details, though I have two comments.

@codecov
Copy link
codecov bot commented Jul 27, 2022 •
edited
Loading

Codecov Report

Merging #642 (ac816ee) into master (10a8a17) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #642   +/-   ##
=======================================
  Coverage   95.11%   95.12%           
=======================================
  Files          24       24           
  Lines        3317     3322    +5     
  Branches      619      620    +1     
=======================================
+ Hits         3155     3160    +5     
  Misses         92       92           
  Partials       70       70
Flag Coverage Δ
ubuntu-18.04 93.91% <100.00%> (+<0.01%) ⬆️
windows-2019 94.76% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
gcovr/decision_analysis.py 92.30% <100.00%> (+0.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10a8a17...ac816ee. Read the comment docs.

@Spacetown Spacetown added this to the 5.2 milestone Jul 28, 2022
Spacetown
Spacetown previously approved these changes Jul 28, 2022
View reviewed changes
Spacetown
Spacetown reviewed Jul 28, 2022
View reviewed changes
@klausweinbauer
Copy link
Contributor Author
klausweinbauer commented Jul 29, 2022 •
edited
Loading

It took me quite some time, but I managed to extract a reliably working test case from my project. I think it is as small as I can get it. It triggers the assert count_false >= 0 error previous to this fix and now generates a DecisionCoverageUncheckable decision. This test could also help to find the original cause for this error. I think there is an issue with the execution count for certain lines; when calculated wrong, this could lead to negative values.
I added a screenshot from the coverage of the test case. Line 7 is executed once according to Branch but counts two in Exec. Is this a bug, or did I miss something here?

I added a test reference output for gcc 9 generated by my native system, but unfortunately, it does not pass when run with docker. I am not quite sure what I did wrong. And is it possible to generate references for other versions with the provided docker?
fig

@Spacetown
Copy link
Member

I added a test reference output for gcc 9 generated by my native system, but unfortunately, it does not pass when run with docker. I am not quite sure what I did wrong. And is it possible to generate references for other versions with the provided docker?

I've started the workflow.

How to update the reference is described in the contributors guide. You can run following command:

python3 -m nox --session 'docker_qa_compiler(all)' -- -k "decisions-neg-delta" --update_reference

@Spacetown Spacetown dismissed their stale review July 29, 2022 20:30

Investigating why the pipeline isn't shown in PR.

@Spacetown
Copy link
Member

@klausweinbauer Can you add a commit to check if the pipeline is started?

@klausweinbauer
Copy link
Contributor Author

Added a test commit

@Spacetown
Copy link
Member

@klausweinbauer My fault, for new tests you should use --generate_reference instead of --update_reference.

@Spacetown
Copy link
Member
Spacetown commented Jul 30, 2022 •
edited
Loading

Data for clang and gcc-5 is missing.

@klausweinbauer
Copy link
Contributor Author

Okay, I will add this

@Spacetown
Copy link
Member

Thank you for working on this.

@Spacetown Spacetown merged commit fcc269a into gcovr:master Jul 30, 2022
@klausweinbauer klausweinbauer deleted the fix_decisions_flag branch July 30, 2022 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Spacetown Spacetown Spacetown left review comments

@latk latk latk approved these changes

Assignees
No one assigned
Labels
Type: Bug
Projects
None yet
Milestone
5.2
Development

Successfully merging this pull request may close these issues.

Gcovr 5.1 with "--decisions" flag throws uncaught exception gcovr 5.1 with --decisions flag: Assertion Error: assert count_false >= 0
3 participants
@klausweinbauer @Spacetown @latk
0