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

New option --no-exclude-noncode-lines to not exclude noncode lines #704

Merged
merged 3 commits into from
Jan 3, 2023

Conversation

Spacetown
Copy link
Member

This PR adds the flag --no-exclude-noncode-lines to turn off the exclude of noncode lines (opening and closing braces which are tagged as uncovered by gcov).

Only the test for clang has such lines, I haven't found a construct which works for GCC.

Related issue: #212

@codecov
Copy link

codecov bot commented Jan 1, 2023

Codecov Report

Base: 94.39% // Head: 95.48% // Increases project coverage by +1.09% 🎉

Coverage data is based on head (f3b0fba) compared to base (451b176).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #704      +/-   ##
==========================================
+ Coverage   94.39%   95.48%   +1.09%     
==========================================
  Files          27       27              
  Lines        3498     3502       +4     
  Branches      605      606       +1     
==========================================
+ Hits         3302     3344      +42     
+ Misses        125       90      -35     
+ Partials       71       68       -3     
Flag Coverage Δ
ubuntu-20.04 94.40% <100.00%> (+<0.01%) ⬆️
windows-2019 95.17% <100.00%> (?)

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

Impacted Files Coverage Δ
gcovr/configuration.py 99.68% <ø> (ø)
gcovr/exclusions/__init__.py 100.00% <100.00%> (ø)
gcovr/workers.py 98.82% <0.00%> (+0.02%) ⬆️
gcovr/writer/html.py 95.60% <0.00%> (+0.58%) ⬆️
gcovr/tests/test_gcovr.py 97.64% <0.00%> (+6.47%) ⬆️
gcovr/utils.py 91.21% <0.00%> (+7.80%) ⬆️
gcovr/tests/test_html_generator.py 100.00% <0.00%> (+50.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.

@Spacetown Spacetown merged commit 9b16926 into gcovr:master Jan 3, 2023
@Spacetown Spacetown deleted the flag_for_noncode_heuristic branch January 3, 2023 20:50
Comment on lines +1257 to +1263
GcovrConfigOption(
"exclude_noncode_lines",
["--no-exclude-noncode-lines"],
group="gcov_options",
help="Exclude coverage from lines which seem to be non-code. Default: {default!s}.",
action="store_false",
),
Copy link
Member

Choose a reason for hiding this comment

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

This comes a bit late, but note that options that are phrased as negations should be avoided.

  1. Negations can be more complicated to understand for users, especially here were we have a triple negation “no”, “exclude”, “noncode”.
  2. This interacts badly with config files.

For example, a config file entry no-exclude-noncode-lines = no is incomprehensible to me. The more I think about it, the less sure I am what is happening. As a user, I would also be confused by the help message:

--no-exclude-noncode-lines
    Exclude coverage from lines which seem to be non-code.
    Default: True.

Does this mean that the exclusion is enabled (true), or that the option (no-exclude) is enabled by default?

Here, this could be avoided by phrasing the option positively, and auto-generating a negation:

    GcovrConfigOption(
        "exclude_noncode_lines",
        ["--exclude-noncode-lines"],
        group="gcov_options",
        help="Exclude coverage from lines that don't seem to contain any code. Default: {default!s}.",
        action="store_true",
        const_negate=False, # <-- autogenerate --no-FLAG option
    ),

Alternatively, if a --exclude-noncode-lines option should never be allowed, it would be sufficient here to explicitly name the config key via config="exclude-noncode-lines".

Copy link
Member Author

Choose a reason for hiding this comment

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

I had also my problems with the name but the default is to remove lines which seems to have no code.
The easiest way would be to change our default to not remove them which can result in uncovered lines for the users. But then the user will get the line coverage which is also shown by gcov and this seems to be better.

Copy link
Member Author

@Spacetown Spacetown Jan 4, 2023

Choose a reason for hiding this comment

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

@latk I've tried your suggestion but it's only possible if the noncode lines are not excluded by default anymore. Our Command line parser doesn't allow a default for action="sore_true" (AssertionError: action=store_true and default conflict):

    GcovrConfigOption(
        "exclude_noncode_lines",
        ["--exclude-noncode-lines"],
        config="exclude-noncode-lines",
        group="gcov_options",
        help="Exclude coverage from lines which seem to be non-code. Default: {default!s}.",
        action="store_true",
        const_negate=False,
        default=True, # This has to be removed!
    ),

I can go with this because than the default coverage numbers are closed to the numbers of gcov. Shall I do this breaking change?

kodiakhq bot pushed a commit to acts-project/acts that referenced this pull request Nov 5, 2023
Addresses and closes Issue #1211 
----------------------------------
The supposed problem was "Parallel processing of gcov data. ([#%s592](gcovr/gcovr#592))" in gcov 5.1. This was already fixed in 5.2.

We need to add `--exclude-noncode-lines` due to a breaking change in gcovr 6.0, otherwise our coverage would change a lot (-0.33%):
New [--exclude-noncode-lines](https://gcovr.com/en/stable/manpage.html#cmdoption-gcovr-exclude-noncode-lines) to exclude noncode lines. Noncode lines are not excluded by default anymore. ([#%s704](gcovr/gcovr#704), [#%s705](gcovr/gcovr#705))

Closes #1211

Update the gcovr-call
----------------------
- remove use of local bundled version (files were removed some years ago)
- remove outdated exclude of folder `/Legacy/`
- remind user to update gcovr to at least `v5.0`
- assert if `v5.1` is used
- black
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants