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
Fix verbose output when using existing gcov files #144
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this excellent pull request! I love it that you added the test case to validate these changes 💯
I just have a few minor nitpicks that I'd like to see addressed before I merge…
foo( | ||
0 | ||
) | ||
; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fancy code layout is not relevant to the gcovr behaviour being tested: the -v
option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c.f. updated README file => same cpp file as simple1 test
gcovr/tests/use-existing/README
Outdated
|
||
The second conditional in main() includes lines with | ||
(, ) and ; characters. These need to be treated as | ||
covered lines. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paragraph is misleading, because it is unrelated to the actual test. And the conditional doesn't contain any commas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
README file updated to reflect actual content of this folder
scripts/gcovr
Outdated
@@ -526,7 +526,8 @@ def process_gcov_data(data_fname, covdata, source_fname, options): | |||
print(' currdir '+currdir) | |||
print(' gcov_fname '+data_fname) | |||
print(' '+str(segments)) | |||
print(' source_fname '+source_fname) | |||
if not source_fname is None: | |||
print(' source_fname '+source_fname) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PEP-8 suggests to write the conditional like if source_fname is not None
. Let's use that for consistency.
Also, do you think print(' source_fname '+(source_fname if source_fname is not None else '(None)'))
might be better? This makes it easier to see that the output is aligned, and makes it clearer that this field is explicitly empty. Or if we simply concatenate str(source_fname)
rather than source_fname
, we don't even need any conditionals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went with str(source_fname)
as suggested.
d1382d0
to
bcf9c6e
Compare
When gcovr is invoked with `-v` and `-g` switches, it fails with error: ``` .... print(' source_fname '+source_fname) TypeError: cannot concatenate 'str' and 'NoneType' objects ``` This patch disables printing of `source_fname` when `source_fname is None`
bcf9c6e
to
95a3fcd
Compare
When gcovr is invoked with
-v
and-g
switches, it fails with error:This patch disables printing of
source_fname
whensource_fname is None
Fixes #143