8000 Add handling of negative counter by Spacetown · Pull Request #701 · gcovr/gcovr · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add handling of negative counter #701

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 8 commits into from
Feb 5, 2023
Merged

Add handling of negative counter #701

merged 8 commits into from
Feb 5, 2023

Conversation

Spacetown
Copy link
Member
@Spacetown Spacetown commented Dec 8, 2022 •
edited
Loading

Add option --gcov-ignore-parse-errors=negative_hits.warn or
--gcov-ignore-parse-errors=negative_hits.warn_once_per_file to ignore negative gcov counter values.
See #583 (comment) for details.

Closes #583

@Spacetown Spacetown added this to the Upcoming release milestone Dec 8, 2022
@Spacetown Spacetown requested a review from latk December 8, 2022 22:09
@codecov
Copy link
codecov bot commented Dec 8, 2022 •
edited
Loading

Codecov Report

Base: 95.55% // Head: 95.50% // Decreases project coverage by -0.06% ⚠️

Coverage data is based on head (45bf5fd) compared to base (3a99c73).
Patch coverage: 90.62% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #701      +/-   ##
==========================================
- Coverage   95.55%   95.50%   -0.06%     
==========================================
  Files          28       28              
  Lines        3736     3759      +23     
  Branches      640      645       +5     
==========================================
+ Hits         3570     3590      +20     
- Misses         93       94       +1     
- Partials       73       75       +2
Flag Coverage Δ
ubuntu-20.04 94.46% <90.62%> (-0.05%) ⬇️
windows-2019 95.17% <90.62%> (-0.06%) ⬇️

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

Impacted Files Coverage Δ
gcovr/configuration.py 99.68% <ø> (ø)
gcovr/gcov_parser.py 97.63% <86.36%> (-1.12%) ⬇️
gcovr/tests/test_gcov_parser.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@latk
Copy link
Member
latk commented Dec 9, 2022

I'm simply not sure what the appropriate behaviour is here. Could you explain on a design level what the intended behaviour is, and why?

  • what is the intended default behaviour?
  • what is the intended behaviour if this flag is explicitly enabled/disabled?
  • how does this interact with --gcov-ignore-parse-errors?
  • what levels of control over the errors/warnings/messages is given to users?
    • can users get an error/warning for each occurrence of a problem?
    • can users get summarized errors/warnings so that they don't get flooded with messages?
    • can users completely silence any mention of this problem?

From what I understand:

  • default behaviour: exception is raised to alert user to problem, this exception is subject to ignore-parse-error handling
  • behaviour if flag is enabled: warning is logged, this warning does not interact with ignore-parse-error
  • user will always get one message per occurrence of the error, no way to summarize or complet 8000 ely silence it

By summarization, I mean output such as the following that reports a warning once, and then reports how many instances of the warning type there were (without printing all of them):

WARN foo.cpp:42: ignoring corrupted branch coverage data on gcov line "branch 4 taken -12345"
INFO see https://github.com/gcovr/gcovr/issue/... for details
WARN 500 duplicate warnings skipped

I am not saying that we should implement such summarization (it does involve tricky data flows), but want to make sure that “always show all warnings/errors” is the intended behaviour in all modes.

latk
latk reviewed Dec 9, 2022
View reviewed changes
@Spacetown @latk
Update gcovr/tests/test_gcov_parser.py
8bcda84 
Co-authored-by: Lukas Atkinson <opensource@LukasAtkinson.de>
@Spacetown
Copy link
Member Author

The intended default behavior for me is to reject the data and abort.

You're right that the user can get lots of warnings and a summarize would be helpful. Counting the issues is not so simple, since we need a parameter (summarize for the file) or a global value (summarize all occurrences).
The place where the error is logged is only aware of the parsed items and not the original line.
If the option is a enumeration we can use abort, log-all and log-first. In the last case we can add a message "... and possible more" instead of counting the errors. I think the user should be aware that there is a problem but if it occur 500 times doesn't give much more information.

Seeing your log example I think it would be better to check the value when splitting the line.

Another option would be to reuse the old flag. If we add an optional argument which defaults to all we can define which parser errors shall be ignored, e.g. negative_counter[log_once].
This configuration can be described in deep in the documentation.

@Spacetown
Copy link
Member Author

I've merged the option and we now only have the --gcov-ignore-parse-errors which got an optional argument. The option can be given several times and if no value is specified the value all is used.

The summary on file level is added but not on project level. I think it's important to warn for each file with unrecognized data and not only once per project.

@Spacetown Spacetown force-pushed the add_handling_of_negative_counter branch from 5fe61c4 to 70f9946 Compare December 10, 2022 23:36
@Spacetown Spacetown force-pushed the add_handling_of_negative_counter branch from 70f9946 to ff3496b Compare December 10, 2022 23:43
@Spacetown Spacetown mentioned this pull request Feb 3, 2023
Closed
latk
latk reviewed Feb 4, 2023
View reviewed changes
@Spacetown Spacetown merged commit 5b03c81 into gcovr:master Feb 5, 2023
@Spacetown Spacetown deleted the add_handling_of_negative_counter branch February 5, 2023 20:59
@martinpitt martinpitt mentioned this pull request Feb 27, 2023
Closed
@golowanow golowanow mentioned this pull request Oct 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@latk latk latk left review comments

Assignees
No one assigned
Labels
Gcov Type: Enhancement
Projects
None yet
Milestone
6.0
Development

Successfully merging this pull request may close these issues.

Unrecognized GCOV output
2 participants
@Spacetown @latk
0