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

close #219: incomplete gcov parsing #220

Closed
wants to merge 5 commits into from
Closed

close #219: incomplete gcov parsing #220

wants to merge 5 commits into from

Conversation

Enchufa2
Copy link

@Enchufa2 Enchufa2 commented Sep 3, 2016

I'm exhausted, but this patch solves the issue. Proof: see my coverage before and after the patch. :)))

I'm going to bed...

@codecov-io
Copy link

codecov-io commented Sep 3, 2016

Current coverage is 88.50% (diff: 100%)

Merging #220 into master will increase coverage by 0.01%

@@             master       #220   diff @@
==========================================
  Files            23         23          
  Lines          1381       1383     +2   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1222       1224     +2   
  Misses          159        159          
  Partials          0          0          

Powered by Codecov. Last update ba4d6de...01fa9de

@Enchufa2
Copy link
Author

Enchufa2 commented Sep 3, 2016

Ready for your review.

@jimhester
Copy link
Member

@Enchufa2 Thank you for working on this, is could we get a minimal example of the problem so we can have a test for this?

@Enchufa2
Copy link
Author

Before this patch you were invoking gcov once per file:

gcov src/file1.gcno
gcov src/file2.gcno
...

and so on. But it seems that this way some interdependencies are missing in the final report. In fact, the manual says the following:

If you invoke gcov with multiple input files, the contributions from each input file are summed. Typically you would invoke it with the same list of files as the final link of your executable.

That is,

gcov src/file1.gcno src/file2.gcno ...

and that's what this patch does. I agree that it would be better to have a test for this. I think that a method defined in one file and called in another one should do the trick: it should fail without this patch and success with it. But I can't work on this now. We can postpone this pull request, or merge it and open a new issue for the test. What do you think?

@Enchufa2
Copy link
Author

I saw that commit 586e65e already applies one gcov per directory, so this issue is fixed, right? There's no point in keeping this PR (as well as #226 and #219) open.

@jimhester
Copy link
Member

I think so, I believe I left them open because I wanted to verify the issues were fixed, if you can verify that was the case we can close them.

@Enchufa2
Copy link
Author

I switched to your covr repo in a new branch and the coverage remains at 100%, so it's definitely fixed.

@jimhester
Copy link
Member

Great thanks

@jimhester jimhester closed this Dec 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants