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 source filename handling when using --use-gcov-files #146
Conversation
3c44fe9
to
d397e05
Compare
6abd63d
to
f571811
Compare
8b563d9
to
7adc3bb
Compare
I've looked at this PR a couple of times and still don't understand it. There's a quite complicated test case for a change of three lines of code. While I'm sure this represents a good improvement, I'm not seeing the big picture here. Could you give me an explanation on what the problem is, and how the test case represents this problem? It would be extremely helpful if you can walk me through the various file paths and what gcovr will do with them under your change. We can then later edit this explanation into a README for the test case. Though not strictly required, I prefer to merge a change only if I understand what I'm doing :D |
The new test First a little sample of what happens without the patch.
Output for
As we can see, we're missing quite a bit of coverage information. If we have a look at the verbose output for
We can see that Instead, If we do that, here's the verbose output for that same file:
This time, the source filename is resolved correctly. |
3f64a77
to
2436567
Compare
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 very much for the clear and thorough explanation. I understand now. This is a great improvement and seems like a very reasonable approach for resolving the path.
scripts/gcovr
Outdated
# let's try using the path to the gcov file as base directory | ||
# Test before assigning not to change behavior compared to previous versions | ||
if os.path.exists(os.path.join(os.path.dirname(data_fname), segments[-1].strip())): | ||
fname = aliases.unalias_path(os.path.join(os.path.dirname(data_fname), segments[-1].strip())) |
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.
Can we assign the path to some variable to avoid code duplication? E.g.
possible_gcov_path = os.path.join(...)
if os.path.exists(possible_gcov_path):
fname = aliases.unalias_path(possible_gcov_path)
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.
Done
gcov documentation states that gcov should be run with the current directory the same as that when you invoked the compiler. It makes sense to handle source filename resolution using this information.
2436567
to
b9debeb
Compare
gcov documentation states that gcov should be run with the current directory the same as that when you invoked the compiler.
It makes sense to handle source filename resolution using this information.
Fixes #145