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

Code coverage #104

Merged
merged 10 commits into from
Sep 1, 2022
Merged

Code coverage #104

merged 10 commits into from
Sep 1, 2022

Conversation

mloufra
Copy link
Contributor

@mloufra mloufra commented Aug 26, 2022

Description

The purpose for this PR is to use JaCoCo to collect these results and push them to codecov.io.
The following step is my idea to hit the mark.

  • Add JaCoCo Gradle plugin to generate code coverage report for opensearch-sdk-java
  • Upload code coverage report to codecov.io through build workflow (Not sure we need to create another yml file to handle that, so I pick the most relevant build.yml to add the code after reviewing each yml file in workflow folder)
  • Add status badge to show the code coverage
  • Add covdecov integration in pull request

Issues Resolved

#48

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

.codecov.yml Outdated Show resolved Hide resolved
.codecov.yml Outdated Show resolved Hide resolved
.codecov.yml Outdated Show resolved Hide resolved
@mloufra mloufra requested review from owaiskazi19 and saratvemulapalli and removed request for owaiskazi19 and saratvemulapalli August 29, 2022 22:31
@saratvemulapalli
Copy link
Member

@mloufra if it's ready for review can you flip the PR from draft?

Also the suggestion from @dbwiddis makes a lot sense. Can we use auto so that the threshold always goes up.

@mloufra
Copy link
Contributor Author

mloufra commented Aug 30, 2022

@mloufra if it's ready for review can you flip the PR from draft?

Also the suggestion from @dbwiddis makes a lot sense. Can we use auto so that the threshold always goes up.

After I make change on target percent to atuo, I will flip the PR from draft

@mloufra mloufra marked this pull request as ready for review August 30, 2022 17:04
@mloufra mloufra requested a review from a team August 30, 2022 17:04
@owaiskazi19
Copy link
Member

@mloufra can you link an example of the codecov report being published as a PR Comment?

README.md Show resolved Hide resolved
@mloufra
Copy link
Contributor Author

mloufra commented Aug 30, 2022

@mloufra can you link an example of the codecov report being published as a PR Comment?

I will try to make one today

@owaiskazi19
Copy link
Member

Failure in build:

FAILURE: Build failed with an exception.

* What went wrong:
Problem configuring task :check from command line.
> Unknown command-line option '--coverage'.

@mloufra
Copy link
Contributor Author

mloufra commented Sep 1, 2022

Failure in build:

FAILURE: Build failed with an exception.

* What went wrong:
Problem configuring task :check from command line.
> Unknown command-line option '--coverage'.

The unknown command-line option --coverage has been deleted. This Failure will not happen any more.

Copy link
Member

@owaiskazi19 owaiskazi19 left a comment

Choose a reason for hiding this comment

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

Approving it to see if codecov will work once codecov.yml is in. We can always revert this change if it doesn’t :)

@mloufra
Copy link
Contributor Author

mloufra commented Sep 1, 2022

Approving it to see if codecov will work once codecov.yml is in. We can always revert this change if it doesn’t :)

I agree with this idea. Let me explain little more about what's going on. For now, We are able to generate the code coverage report for our branch, and push the report to Codecov.io. (Through this Link we can see the report has been pushed successfully). The problem is that Codecov.io does not show up our code coverage report on its page. It keeps showing "No data available", it means the repo is not setup successfully. In general step, we should ask for organization access to setup repo. After I doing research and contact with Tianli, opensearch-project organization has denied this requirement before and most team who succeed to setup repo also do not have this access. And I check the Guideline for code coverage, it has no description of this situation. So I think we can try to approve it to see what happen.

@saratvemulapalli saratvemulapalli merged commit b0b5224 into opensearch-project:main Sep 1, 2022
@owaiskazi19
Copy link
Member

owaiskazi19 commented Sep 2, 2022

It worked on README. @mloufra The next step is to monitor if codecov is commenting the report on the PRs.

@dbwiddis
Copy link
Member

dbwiddis commented Sep 2, 2022

I like our report and the badge that gets us quickly to it! Will really help us audit our code coverage.

@mloufra
Copy link
Contributor Author

mloufra commented Sep 6, 2022

The Codecov comment is running successfully. Here is the link.

@dbwiddis
Copy link
Member

dbwiddis commented Sep 6, 2022

The Codecov comment is running successfully. Here is the link.

Glad it was my awesome well-covered PR that proves it's working. 😀

kokibas pushed a commit to kokibas/opensearch-sdk-java that referenced this pull request Mar 17, 2023
* issue opensearch-project#28

Signed-off-by: mloufra <[email protected]>

* Update the lastest coomit

Signed-off-by: mloufra <[email protected]>

* Rename the method and fix the conflict

Signed-off-by: mloufra <[email protected]>

* fix merge conflict

Signed-off-by: mloufra <[email protected]>

* Add code coverage report

Signed-off-by: mloufra <[email protected]>

* Rebase the lastest commit

Signed-off-by: mloufra <[email protected]>

* Add code coverage report

Signed-off-by: mloufra <[email protected]>

* delete --coverage

Signed-off-by: mloufra <[email protected]>

Signed-off-by: mloufra <[email protected]>
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.

4 participants