Skip to content

Enable code coverage#8303

Merged
gianm merged 4 commits intoapache:masterfrom
ccaominh:code-coverage
Aug 20, 2019
Merged

Enable code coverage#8303
gianm merged 4 commits intoapache:masterfrom
ccaominh:code-coverage

Conversation

@ccaominh
Copy link
Copy Markdown
Contributor

@ccaominh ccaominh commented Aug 15, 2019

Fixes #6009.

Description

Code coverage was disabled via #3122 due to an issue with
cobertura in Travis CI. Switch code coverage tool from cobertura to
jacoco to avoid issue and re-enable coveralls for Travis CI.


This PR has:

  • been self-reviewed.

Code coverage was disabled via
apache#3122 due to an issue with
cobertura in Travis CI. Switch code coverage tool from cobertura to
jacoco to avoid issue and re-enable coveralls for Travis CI.
@ccaominh ccaominh changed the title [WIP] Enable code coverage Enable code coverage Aug 16, 2019
Comment thread pom.xml
<!-- Ignore generated code -->
<exclude>org/apache/druid/math/expr/antlr/Expr*</exclude> <!-- core -->
<exclude>org/apache/druid/**/generated/*Benchmark*</exclude> <!-- benchmarks -->
<exclude>org/apache/druid/data/input/influx/InfluxLineProtocol*</exclude> <!-- extensions-contrib/influx-extensions -->
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if maybe we should ignore all of benchmarks instead of just the generated stuff, and also integration-tests since neither contain production code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added benchmarks and integration-tests to the exclusions

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, it looks like you could add org/testng/DruidTestRunnerFactory too

Screen Shot 2019-08-20 at 12 48 44 PM

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added

Copy link
Copy Markdown
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

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

lgtm, 👍

@gianm gianm merged commit 6fa22f6 into apache:master Aug 20, 2019
@gianm gianm added this to the 0.16.0 milestone Aug 20, 2019
@gianm gianm mentioned this pull request Aug 20, 2019
8 tasks
@ccaominh ccaominh deleted the code-coverage branch August 20, 2019 22:46
@himanshug
Copy link
Copy Markdown
Contributor

In #8359 coverage build failed due to drop by 0.09% (report says coverage of files not touched in the PR dropped)
also in any case, do we really want to fail the overall build on code coverage drop of 0.09% .

@clintropolis @gianm
while code coverage is an interesting metric , I don't think it is a metric to chase for and fail the builds.

@ccaominh
Copy link
Copy Markdown
Contributor Author

@himanshug I'm hoping #8374 will fix the issue with the fluctuating coverage reports

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.

Coveralls badge is pointing to old repo

4 participants