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

[HUDI-529] Added cobertura coverage reporting support. #1232

Closed

Conversation

prashantwason
Copy link
Member

@prashantwason prashantwason commented Jan 15, 2020

What is the purpose of the pull request

Hudi project has code coverage enabled via the jacoco plugin. Jenkins has better support for coverage reporting using the Jenkins Cobertura plugin.

This enhancement provides a way to convert the jacoco coverage report to cobertura format at the end of the unit test runs.

Brief change log

  1. Fixed the jacoco coverage settings
    • the jacoco agent was not being loaded as the SureFire plugins argLine was not set correctly
  2. The jacoco coverage report is converted to cobertura report format after test completion using the gradle jacobo plugin.
  3. There is a specific gradle job defined for the conversion. Modules which do not run tests use a "noop" job.
  4. When tests are skipped (-DskipTests) the gradle job is set to noop to prevent the conversion.

Verify this pull request

This pull request is a trivial rework / code cleanup without any test coverage.
It has been tested to generate the coverage reports after a maven test.

Committer checklist

  • Has a corresponding JIRA in PR title & commit

  • Commit message is descriptive of the change

  • CI is green

  • Necessary doc changes done or have another open PR

  • For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

@n3nash n3nash self-assigned this Jan 17, 2020
@leesf
Copy link
Contributor

leesf commented Jan 17, 2020

Would you please fix the travis failure first? @prashantwason

@prashantwason
Copy link
Member Author

The build fails as gradle could not be downloaded from the http location.

Access denied to: http://repo.gradle.org/gradle/libs-releases-local/org/gradle/gradle-tooling-api/2.13/gradle-tooling-api-2.13.pom , ReasonPhrase:Forbidden. -> [Help 1]

I will look into upgrading the plugin as the http access is not allowed.

Copy link
Member

@vinothchandar vinothchandar left a comment

Choose a reason for hiding this comment

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

@prashantwason I am not sure we want to introduce gradle into the project for supporting cobertura.. Can we avoid this?

IMO this will be a blocker for this PR..

1. Coverage is tracked using jacoco java-agent which running the tests. The jacoco coverage report is converted to cobertura report format after test completion using the gradle jacobo plugin.
2. There is a specific gradle job defined for the conversion. Modules which do not run tests use a "noop" job.
3. When tests are skipped (-DskipTests) the gradle job is set to noop to prevent the conversion.
@n3nash
Copy link
Contributor

n3nash commented Jan 27, 2020

@vinothchandar could you share your concerns with introducing gradle as a plugin please ?

@vinothchandar
Copy link
Member

@n3nash just want to avoid bringing in a new build system just for this, when things like this exist https://www.mojohaus.org/cobertura-maven-plugin/

Slowly overtime, people will add things to gradle.. and I am concerned it will get out of hands

@prashantwason
Copy link
Member Author

We can only use one of jacoco or cobertura plugins as they both work by adding a java-agent to the unit test. I have found that using cobertura plugin doubles the test run time. Also, cobertura seems to be less maintained (last release was in 2015). So its better to use jacoco.

@vinothchandar
Copy link
Member

I see you want gradle mostly because of jacobo? Lets hash out the need for bringing in gradle, which seems like an overkill..
you can just an extra step in jenkins in a script like this ? https://github.com/rix0rrr/cover2cover (don't know if this specific one works. but you get my general point) .

@prashantwason
Copy link
Member Author

We have equally good online code coverage reporting at CodeCov now. So this change is not required.

@prashantwason prashantwason deleted the pw_enable_cobertura_reporting branch August 5, 2021 07:32
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.

None yet

4 participants