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

Added Codecov.io coverage reports #1002

Closed
wants to merge 1 commit into from
Closed

Added Codecov.io coverage reports #1002

wants to merge 1 commit into from

Conversation

stevepeak
Copy link

Hey Friends! I'm from Codecov.io and I wanted to personally add our coverage reporting tools to junit.

You can see the reports now here: codecov.io/github/stevepeak/junit

codecov.io

I'm excited to hear your feedback! Thank you for your time.

@stevepeak stevepeak changed the title Hey Friends! I'm from [Codecov.io](https://codecov.io/) and I wanted to personally add our coverage reporting tools to junit. Added Codecov.io coverage reports Oct 2, 2014
@@ -1,7 +1,9 @@
language: java
before_install: sudo pip install codecov
Copy link
Member

Choose a reason for hiding this comment

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

It seems more than a bit frightening to have "sudo" in a checked in file

Copy link
Author

Choose a reason for hiding this comment

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

I understand your concern. I've tried to remove the sudo, but it gets rejected during the build process. If you have any ideas how to avoid that it would be a great help :)

Example build error on Travis

Copy link
Member

Choose a reason for hiding this comment

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

@stevepeak could you contact someone on Travis to see if there is a better way of getting codecov.io to work (and if there isn't, if they know of another code coverage tool that works with Travis)?

Copy link
Author

Choose a reason for hiding this comment

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

@kcooney I'll reach out. I'll let you know when they get back to me.

Copy link
Member

Choose a reason for hiding this comment

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

@stevepeak Any news on the sudo question?

@kcooney
Copy link
Member

kcooney commented Oct 2, 2014

Interesting.

I only did a quick glance at the coverage. The coverage for this file looks wrong:

https://codecov.io/github/stevepeak/junit/src/main/java/org/junit/internal/Throwables.java?ref=master

There's a private method that being listed as covered, but the one line of code where it is called is not listed as covered (because the private method throws an exception?(

@stevepeak
Copy link
Author

Which line are you mentioning has issues? We simply take the coverage reports and present the data collected...I'll have to take a look at the report itself. What were you expecting that line to be?

@kcooney
Copy link
Member

kcooney commented Oct 2, 2014

@stevepeak for Throwables.java, if line 40 is covered, then line 34 must be covered.

What tool is being used to collect coverage?

@stevepeak
Copy link
Author

JaCoCo coverage via XML output. I have not got a change to review the output yet. I'll send you a copy when I have it available so we can review it. But I'm simply extracting the coverage data provided, unaltered. I'm saddened that this is not accurate... I may need to express this concern to JaCoCo.

@stevepeak
Copy link
Author

@kcooney I got around to producing the logs containing the reports.

I noticed a new section of of the output that I was not accounting for (out java uploader new) and I really appreciate you pointing this out to my attention! I'll work today to get the reports to display perfect accuracy, and get back to you shortly.

I'll be building a new uploader in shell so that you do not need to sudo it.

Thanks! I look forward to you using Codecov, and I hope you can excuse this confusion as we continue to improve this product :)


Here is an extract I found in the logs. Notice how it shows covered=0 for line 34

<method name="rethrowAsException" desc="(Ljava/lang/Throwable;)Ljava/lang/Exception;" line="34">
    <counter type="INSTRUCTION" missed="4" covered="0" />
    <counter type="LINE" missed="2" covered="0" />
    <counter type="COMPLEXITY" missed="1" covered="0" />
    <counter type="METHOD" missed="1" covered="0" />
</method>

What do you make of this?

@kcooney
Copy link
Member

kcooney commented Oct 6, 2014

@stevepeak perhaps the compiler is doing some kind of inlining of the method? I'm curious if EMMA would consider that line covered.

@marcphilipp
Copy link
Member

@stevepeak Any updates on this?

@stevepeak
Copy link
Author

@marcphilipp I'll include parsing coverage reports for EMMA in the coming month. I'll post on this thread when that is accomplished. Sorry for the delay and thank you for your patience :)

@marcphilipp
Copy link
Member

@stevepeak Why EMMA? Why not JaCoCo or Cobertura?

@stevepeak
Copy link
Author

We already support JaCoCo and Cobertura.

@marcphilipp
Copy link
Member

Sorry, I overlooked @kcooney's comment.

</execution>
<execution>
<id>report</id>
<phase>test</phase>
Copy link
Member

Choose a reason for hiding this comment

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

Can we make running coverage optional?

@marcphilipp
Copy link
Member

@stevepeak I don't think the EMMA report will look substantially different from the JaCoCo one. Have you seen my inline comment above?

@stevepeak
Copy link
Author

Codecov should work with the code from your comment above. I may be confused what the issue is...

@kcooney
Copy link
Member

kcooney commented Dec 14, 2014

I think we have two issues and one question. The issues:

  1. Using sudo
  2. Can we make running coverage optional? I have seen cases where tests fail only when collecting coverage, and sometimes it's tricky to fix, so I agree that collecting coverage should be optional

The question was from be about puzzling results about what lines were covered in Throwables.java, and I was curious whether a different coverage tool (for example, Emma) gave different results.

@stevepeak
Copy link
Author

  • Using Sudo It is on the roadmap
  • We don't support a skipping upload method at the moment...but seem reasonable to add sooner then later

@marcphilipp
Copy link
Member

@stevepeak We don't need a skipping upload method (I think). For the Travis build we can always turn coverage on. For other builds, however, I would like to be able to turn coverage off, e.g. for a release build. I'm sure this can be achieved using a Maven profile. Do you have time to work on that or is this something we should work out on ourselves?

Regarding the sudo "problem": I don't think it's critical since Travis explicitly allows using sudo.

@stevepeak
Copy link
Author

@kcooney the bash uploader is ready, sudo not required. I would love for you to give it a spin! https://github.com/codecov/codecov-bash

@kcooney
Copy link
Member

kcooney commented Sep 18, 2016

(doing a pass at trying to close old issues)

There are still a number of unresolved issues in this pull. Moreover, given the limited time the maintainers have spent on JUnit lately, I'm not sure anyone would take action on code coverage results.

Vote to close

@marcphilipp
Copy link
Member

Agreed.

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.

None yet

3 participants