Headers containing definitions appear twice in results. #694
Conversation
Headers containing definitions appear once with all lower-case and once correct case characters in the results under Windows. That leads to wrong code coverage results when using templates.
gcovr/gcov.py
Outdated
| @@ -184,6 +185,7 @@ def guess_source_file_name( | |||
| fname = guess_source_file_name_heuristics( | |||
| gcovname, data_fname, currdir, root_dir, starting_dir, obj_dir, gcda_fname | |||
| ) | |||
| fname = str(pathlib.Path(fname).resolve()) # resolves case insensitively of windows paths | |||
There was a problem hiding this comment.
This also resolves symlinks which is a big change here.
There was a problem hiding this comment.
If os.path.realpath doesn't fix the case we should first replace every usage of os.path with pathlib.Path.
There was a problem hiding this comment.
I tried to replace os.path with pathlib.Path but stopped this because there are differences under the hood on Windows which results in Exceptions if used in SCM Clearcase.
Win errors allowed in os.path https://github.com/python/cpython/blob/a87c46eab3c306b1c5b8a072b7b30ac2c50651c0/Lib/ntpath.py#L615-L629 and here the errors allowed in pathlib https://github.com/python/cpython/blob/4710a7589aee48bec9099af308bf5938285ba336/Lib/pathlib.py#L33-L36.
There was a problem hiding this comment.
In that case simply using os.path.realpath instead of pathlib.Path().resolve() seam to be the better solution.
There was a problem hiding this comment.
But this also resolves the symlinks. I think it would be better fix the casing on our own if the OS is case insensitive.
There was a problem hiding this comment.
If we want to fix the casing by our self, I think to only reasonable soltuion would be to recursivlely resolve the path and check if the case of the child fits the resolved path.
An alternative easy soltuion would be to convert all paths to lower case if the OS is case insensitve.
There was a problem hiding this comment.
I prefer the first solution by splitting the path and fix the case for each part.
|
Maybe we can add two gcov files and use the existing files with option |
Codecov ReportBase: 95.28% // Head: 95.62% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #694 +/- ##
==========================================
+ Coverage 95.28% 95.62% +0.34%
==========================================
Files 28 28
Lines 3773 3843 +70
Branches 666 671 +5
==========================================
+ Hits 3595 3675 +80
+ Misses 93 91 -2
+ Partials 85 77 -8
Flags with carried forward coverage won't be shown. Click here to find out more.
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. |
fef9f69 to
5d50ef2
Compare
|
Thank you for working on this. |
Issue #329: Headers containing definitions appear once with all lower-case and once with correct case characters in the results under Windows. That leads to wrong code coverage results when using templates.
Implementation of @gmc-devel suggested solution: #329 (comment)
Closes: #329