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

[draft] added jacoco coverage to oppia-android #2744

Closed
wants to merge 18 commits into from
Closed

[draft] added jacoco coverage to oppia-android #2744

wants to merge 18 commits into from

Conversation

MaskedCarrot
Copy link
Contributor

Explanation

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
  • The PR follows the style guide.
  • The PR does not contain any unnecessary auto-generated code from Android Studio.
  • The PR is made from a branch that's not called "develop".
  • The PR is made from a branch that is up-to-date with "develop".
  • The PR's branch is based on "develop" and not on any other branch.
  • The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

@anandwana001
Copy link
Contributor

Please check these comments and let me know if need any help
#2466 (comment)
#2466 (comment)

@MaskedCarrot
Copy link
Contributor Author

Screenshot from 2021-02-20 22-49-30
Screenshot from 2021-02-20 22-31-43
Screenshot from 2021-02-20 22-30-35

I ran the coverage task from Gradle.
Jacco is showing 0% coverage even though 100% of the tests are passing. Can you guide me in the right direction to fix the error?

@anandwana001
Copy link
Contributor

0% are the missed instructions and missed branches. Look for what percentage of lines are covered and try to open the particular file then you will see which line is covered and which line is not covered.

@MaskedCarrot
Copy link
Contributor Author

generated a coverage report which shows 80% coverage for app package.
Screenshot from 2021-02-23 19-48-02
@anandwana001 PTAL

@MaskedCarrot
Copy link
Contributor Author

MaskedCarrot commented Feb 23, 2021

Jacoco is giving the following results

package coverage
app 80%
domain 0%
testing 81%
data 0%
utility 0%

@anandwana001
Copy link
Contributor

@MaskedCarrot is this code coverage for the Test package or the main codebase?
Currently, we need to have code coverage for a test package to understand are we covering all the test cases correctly or not.

Also, an additional point if you could try out, After generating a report using Jacoco, try to upload them to codecov and check the result.

@BenHenning BenHenning added the PR don't merge - NEEDS UPDATE Corresponds to PRs that need to be updated with the latest develop changes before they can be merged label Feb 24, 2021
@MaskedCarrot
Copy link
Contributor Author

@MaskedCarrot is this code coverage for the Test package or the main codebase?

@anandwana001 this code coverage is for the main codebase.

Currently, we need to have code coverage for a test package to understand are we covering all the test cases correctly or not.

What I understand is that we have to create a package testPackage with some sample tests and see if jacoco report shows coverage or not.

@BenHenning BenHenning removed the PR don't merge - NEEDS UPDATE Corresponds to PRs that need to be updated with the latest develop changes before they can be merged label Feb 24, 2021
@BenHenning
Copy link
Member

Closing & reopening to restart now that codecov/codecov-action@v1 has been enabled for actions.

@BenHenning BenHenning closed this Mar 4, 2021
@BenHenning BenHenning reopened this Mar 4, 2021
@MaskedCarrot
Copy link
Contributor Author

MaskedCarrot commented Mar 4, 2021

@BenHenning just to inform you, this is not the main PR. Peculiar is assigned to this issue I am helping her with this. I talked to her about this and we both are facing the same issue(0 coverage in domain).

@BenHenning
Copy link
Member

With the latest re-run I see org.gradle.execution.TaskSelectionException: Task 'testDebugUnitTestCoverage' not found in project ':app'.. Is this using 0.7 or 0.8?

@BenHenning
Copy link
Member

Thanks for clarifying @MaskedCarrot. Can we maybe close this issue, then, and consolidate on that PR so that there's a single source of truth?

@MaskedCarrot
Copy link
Contributor Author

With the latest re-run I see org.gradle.execution.TaskSelectionException: Task 'testDebugUnitTestCoverage' not found in project ':app'.. Is this using 0.7 or 0.8?

i also have another pr which I did not open in oppia-android. It ran the action to upload the report to codecov. it has a different task name. I have updated the jacoco_coverage.yml It should work now.

@MaskedCarrot
Copy link
Contributor Author

MaskedCarrot commented Mar 4, 2021

Thanks for clarifying @MaskedCarrot. Can we maybe close this issue, then, and consolidate on that PR so that there's a single source of truth?

@BenHenning should I close it after the report is uploaded to codecov? Because peculiar has not setup the yml yet.
Also, tests are failing in github action with the same error now.

@MaskedCarrot
Copy link
Contributor Author

MaskedCarrot commented Mar 4, 2021

closing this issue. further work on this issue will continue in peculiar's PR

@peculiaruc
Copy link
Contributor

@BenHenning just to inform you, this is not the main PR. Peculiar is assigned to this issue I am helping her with this. I talked to her about this and we both are facing the same issue(0 coverage in domain). here is the link to peculiar's pr.

Please I don't understand the issue you are referring here @MaskedCarrot

@MaskedCarrot
Copy link
Contributor Author

@peculiaruc as per our chats on gitter you said that 100 percent of the tests are passing for the domain module but you are getting 0 percent coverage. This is the issue I am referring here

@peculiaruc
Copy link
Contributor

@peculiaruc as per our chats on gitter you said that 100 percent of the tests are passing for the domain module but you are getting 0 percent coverage. This is the issue I am referring here

As I was about getting clearification of your comments. It was a question which I resolved and asked you never mind

@MaskedCarrot
Copy link
Contributor Author

MaskedCarrot commented Mar 5, 2021

@peculiaruc okay but I think a pr once referenced cannot be removed.

@MaskedCarrot
Copy link
Contributor Author

@peculiaruc as per our chats on gitter you said that 100 percent of the tests are passing for the domain module but you are getting 0 percent coverage. This is the issue I am referring here

As I was about getting clearification of your comments. It was a question which I resolved and asked you never mind

@peculiaruc can you tell me how did you resolve this issue ?

@peculiaruc
Copy link
Contributor

@peculiaruc okay but I think a pr once referenced cannot be removed.

Is just to resolved the difference @BenHenning mentioned

@peculiaruc
Copy link
Contributor

@peculiaruc as per our chats on gitter you said that 100 percent of the tests are passing for the domain module but you are getting 0 percent coverage. This is the issue I am referring here

As I was about getting clearification of your comments. It was a question which I resolved and asked you never mind

@peculiaruc can you tell me how did you resolve this issue ?

The issue I mentioned here is for ./gradlew :domain:jacocoTestReport.

You told @BenHenning you are helping but you keep asking me questions as the reviewers. I think you should be providing solutions not just asking me questions.

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