Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

set up jacoco for code coverage #234

Merged
merged 8 commits into from
Nov 4, 2020

Conversation

yu-sun-77
Copy link
Contributor

@yu-sun-77 yu-sun-77 commented Nov 3, 2020

Fixes #, if available: #236

Description of changes:

  1. add jacoco to set up code coverage.
  2. show code coverage report for every pull request in future.
  3. integrate with "codecov" to show latest code coverage badge in REAMDE file.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@yu-sun-77 yu-sun-77 changed the base branch from code-coverage to master November 3, 2020 01:36
@codecov
Copy link

codecov bot commented Nov 3, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@b66011b). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #234   +/-   ##
=========================================
  Coverage          ?   14.09%           
  Complexity        ?       65           
=========================================
  Files             ?       39           
  Lines             ?     2015           
  Branches          ?      150           
=========================================
  Hits              ?      284           
  Misses            ?     1722           
  Partials          ?        9           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b66011b...5936dea. Read the comment docs.

build.gradle Outdated
violationRules {
rule {
limit {
minimum = 0.8
Copy link
Contributor

Choose a reason for hiding this comment

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

This will fail if you set it to 80% as we don't have that coverage today, right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems you're referring to outdated version of code, I've changed to 0.0 for now. Can you please refresh and review again? Thanks.

build.gradle Outdated

jacocoTestReport {
reports {
xml.enabled false
Copy link
Contributor

Choose a reason for hiding this comment

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

We should enable at least one of the reports, is html enabled by default ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Latest code has been changed to true. Can you please refresh and review again? Thanks.

build.gradle Outdated
}

rule {
enabled = false
Copy link
Contributor

Choose a reason for hiding this comment

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

We want to keep verification to false for now ?

@yojs
Copy link
Contributor

yojs commented Nov 3, 2020

@yu-sun-77 can we create an issue and add it in the Fixes #, if available: section.
And also a description, what this change does

@yu-sun-77 yu-sun-77 linked an issue Nov 3, 2020 that may be closed by this pull request
@yu-sun-77
Copy link
Contributor Author

@yu-sun-77 can we create an issue and add it in the Fixes #, if available: section.
And also a description, what this change does

I've created an issue and update description. thanks!

@yu-sun-77 yu-sun-77 requested a review from rguo-aws November 3, 2020 22:53
Copy link
Contributor

@rguo-aws rguo-aws left a comment

Choose a reason for hiding this comment

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

LGTM

@yu-sun-77 yu-sun-77 merged commit 2b9ef9c into opendistro-for-elasticsearch:master Nov 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add jacoco to set up code coverage
3 participants