Fix issue #1497: Set up code coverage for Robolectric tests #2466

peculiaruc · 2021-01-12T15:59:29Z

Explanation

Code Coverage SetUp: Configure the build,gradle file to use Jacoco to generate Code coverage report.
Choose coverage runner as Jacoco in Edit Configuration in Android Studio

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.

BenHenning · 2021-01-13T02:30:37Z

@peculiaruc thanks for uploading this. A couple of things to note:

When creating PRs that aren't yet ready for review/production, we should mark them as 'drafts'
When giving instructions, try to list them as exact steps in a bullet or numbered list, e.g. 1. Do X, 2. Navigate to menu Y and click Z, etc. to keep things as deterministic as possible.

Going to try to run the code coverage now and see how it goes.

anandwana001 · 2021-01-13T04:03:51Z

Adding a screenshot of what I got after running jacocoTestDebugUnitTestResult gradle task.

peculiaruc · 2021-01-13T08:55:17Z

@peculiaruc thanks for uploading this. A couple of things to note:
1. When creating PRs that aren't yet ready for review/production, we should mark them as 'drafts'

2. When giving instructions, try to list them as exact steps in a bullet or numbered list, e.g. 1. Do X, 2. Navigate to menu Y and click Z, etc. to keep things as deterministic as possible.
Going to try to run the code coverage now and see how it goes.

Noted. Thank

peculiaruc · 2021-01-13T09:22:53Z

Adding a screenshot of what I got after running jacocoTestDebugUnitTestResult gradle task.

This shows TestDebugUnitTestResult. it didn't give the result i am looking for which is showing the percentage of code covered. I used createDebugCoverageReport, the HTML results this

anandwana001 · 2021-01-13T11:39:23Z

peculiaruc · 2021-01-13T13:12:50Z

Yes this is the result i am looking for. Thanks

anandwana001 · 2021-01-13T13:15:15Z

Just an update, this result is not from this pr, I tried something else a bit. Will try this pr also.

peculiaruc · 2021-01-13T13:18:17Z

Just an update, this result is not from this pr, I tried something else a bit. Will try this pr also.

I see, that thing you tried is what i am missing, i think

BenHenning · 2021-01-13T17:15:02Z

Thanks for following up on this @anandwana001. Haven't had a chance to run this myself.

BenHenning · 2021-01-13T21:05:30Z

To capture some of my findings when looking into this today:

I couldn't get Jacoco to actually report within Android Studio correct results. However, I made a few changes to your configuration and I was able to get generated HTML results using the CLI command: ./gradlew :domain:jacocoTestReport. Note that I needed to change the jacocoTestReport to use a mainSrc of "$project.projectDir/src/main/java" (since that matches our directory structure), and I commented out the fileFilter list for now to avoid filtering anything out. This works well for CLI, but not for AS itself.
Running code coverage from within AS using Idea instead of JaCoCo does produce results
The results from JaCoCo seem incomplete: they aren't covering suspend functions correctly, so the coverage is under-reporting
I started looking into trying to get coverage to work with Bazel instead since this is starting to feel extremely Gradle-specific and we're actively moving off of Gradle, but unfortunately it appears that Bazel does not support coverage for android_local_test and the team is actively working to introduce new Starlark rules for Android but we may need to wait a few months

This leads us to getting a bit stuck. I think the correct way to do this is to build code coverage support into Bazel while we wait for the new rules (especially since the new rules won't support coverage out of the box), but I just don't have the bandwidth to do that right now. Given that, I think we can still use JaCoCo in the interim:

I suggest using the CLI version since that does generate a report
I suggest proceeding with writing up the documentation for helping other team members run code coverage reports, interpret them, and turn the missing coverage into good tests by mapping lines of code to behaviors

That second bit is the most important thing that we can still create. While we may change the means by which we generate the reports, being able to generate them is still a fundamental first step.

Does this sound good to you @peculiaruc?

peculiaruc · 2021-01-14T01:52:46Z

To capture some of my findings when looking into this today:

I couldn't get Jacoco to actually report within Android Studio correct results. However, I made a few changes to your configuration and I was able to get generated HTML results using the CLI command: ./gradlew :domain:jacocoTestReport. Note that I needed to change the jacocoTestReport to use a mainSrc of "$project.projectDir/src/main/java" (since that matches our directory structure), and I commented out the fileFilter list for now to avoid filtering anything out. This works well for CLI, but not for AS itself.

Running code coverage from within AS using Idea instead of JaCoCo does produce results

The results from JaCoCo seem incomplete: they aren't covering suspend functions correctly, so the coverage is under-reporting

I started looking into trying to get coverage to work with Bazel instead since this is starting to feel extremely Gradle-specific and we're actively moving off of Gradle, but unfortunately it appears that Bazel does not support coverage for android_local_test and the team is actively working to introduce new Starlark rules for Android but we may need to wait a few months

This leads us to getting a bit stuck. I think the correct way to do this is to build code coverage support into Bazel while we wait for the new rules (especially since the new rules won't support coverage out of the box), but I just don't have the bandwidth to do that right now. Given that, I think we can still use JaCoCo in the interim:

I suggest using the CLI version since that does generate a report

I suggest proceeding with writing up the documentation for helping other team members run code coverage reports, interpret them, and turn the missing coverage into good tests by mapping lines of code to behaviors

That second bit is the most important thing that we can still create. While we may change the means by which we generate the reports, being able to generate them is still a fundamental first step.

Does this sound good to you @peculiaruc?

Yes please. Thanks. I going to implement the changes, then finish up the documentation

peculiaruc · 2021-01-15T11:50:55Z

Just an update, this result is not from this pr, I tried something else a bit. Will try this pr also.

I see, that thing you tried is what i am missing, i think

peculiaruc · 2021-01-15T11:54:28Z

Please i mistakenly closed the Pr before. sorry about that

BenHenning · 2021-01-16T01:16:26Z

Ack thanks @peculiaruc. Assigning this back over to you since it seems like per your last response the next steps here are on your plate. Please let me know if you need my input on anything or run into any problems.

Also, I think it would be really helpful if you compiled your findings & analyses in a document & shared that in this PR since it will help bring context to future team members.

peculiaruc · 2021-01-16T13:35:44Z

Ack thanks @peculiaruc. Assigning this back over to you since it seems like per your last response the next steps here are on your plate. Please let me know if you need my input on anything or run into any problems.

Also, I think it would be really helpful if you compiled your findings & analyses in a document & shared that in this PR since it will help bring context to future team members.

Noted . I am on it

peculiaruc · 2021-01-20T02:52:15Z

1.The document below contains my findings about running code coverages to determine the percentage of the app code covered by each unit test.
2. It also contain steps to configure the android studio to get code coverage reports.

https://docs.google.com/document/d/1oSIY-12lGl28MVgGxs0mbuJFTORkjGqypeiOGq95jLc/edit

It also await review from reviewers.

BenHenning · 2021-01-21T05:48:06Z

Thanks @peculiaruc! @anandwana001 can you take a first pass on the document?

BenHenning · 2021-01-29T01:48:15Z

Thanks for clarifying @anandwana001. Your plan SGTM to confirm that code coverage is working. Deferring to you to drive this PR to completion (please loop me in if needed). @peculiaruc feel free to continue to reach out to me if you have questions, otherwise Akshay will probably be your main point of contact as part of finishing this PR.

BenHenning · 2021-01-29T01:48:48Z

@peculiaruc are the next steps clear to you from Akshay's comment above?

peculiaruc · 2021-01-29T02:06:38Z

@ben Henning <henning.benmax@gmail.com> Yes, I have started working on it

…

On Fri, 29 Jan 2021 at 02:49, Ben Henning ***@***.***> wrote: @peculiaruc <https://github.com/peculiaruc> are the next steps clear to you from Akshay's comment above? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2466 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIOVAVYOJRUFKMB5FORMELLS4IHY3ANCNFSM4V7NMR7Q> .

peculiaruc · 2021-02-01T22:59:00Z

Hi! @anandwana001 <https://github.com/anandwana001>. Below is github gist for task no 1-6. The document has also been updated. Please take a look https://gist.github.com/peculiaruc/2e63953249c1af8d731246f737fec588 Thanks Peculiar On Fri, 29 Jan 2021 at 03:06, Peculiar C. Umeh <peculiarumeh02@gmail.com> wrote:

…

@ben Henning ***@***.***> Yes, I have started working on it On Fri, 29 Jan 2021 at 02:49, Ben Henning ***@***.***> wrote: > @peculiaruc <https://github.com/peculiaruc> are the next steps clear to > you from Akshay's comment above? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#2466 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AIOVAVYOJRUFKMB5FORMELLS4IHY3ANCNFSM4V7NMR7Q> > . >

anandwana001 · 2021-02-02T03:51:18Z

Left comment - https://gist.github.com/peculiaruc/2e63953249c1af8d731246f737fec588#gistcomment-3616265

peculiaruc · 2021-02-03T00:46:32Z

Ack thanks @peculiaruc. Assigning this back over to you since it seems like per your last response the next steps here are on your plate. Please let me know if you need my input on anything or run into any problems.

Also, I think it would be really helpful if you compiled your findings & analyses in a document & shared that in this PR since it will help bring context to future team members.

Done

peculiaruc · 2021-02-03T00:48:41Z

1. Match the code coverage result of `jacoco` and android studio. Are we getting the same result or not.
2. In the configuration (Android Studio), we also need to mention the package in the code coverage section. Else we will get the 0% code covered.
3. Added a screenshot showing 20-25% lines are not getting covered.
4. Red and Green colour in the editor showing what has covered and what not.
5. According to these result, we need to update those lines which are in red.
6. Need to figure out a way, how to do this without using Android Studio if `jacoco` doesn't give the same result.

Describing step by step why we need to do this now and what can be the multiple parts of the code coverage:

1. From point 1 above - We need to confirm the result produced from `jacoco` and android studio code coverage tool so that we have a strong confirmation that we are on right track in implementing the `jacoco`.

2. From point 2 above - just an android studio code coverage set up steps, we need this in documentation

3. From point 5 above - Updating/Fixing those red lines for a single file and checking if those lines are getting covered in code coverage or not will help us to understand that our code coverage set up is working correctly. or we can say it like this `Our code coverage set up will give the correct result for both the line of code which is covered and not covered.`

4. From point 6 above - If the code coverage result of `jacoco` and Android Studio is different then we need to check either we have mistaken any set up while setting `jacoco` or does Android Studio works in a different way for covering these line.

The single goal of this PR:

Code coverage setup correctly and giving correct results for both the cases when line are covered and when lines are not covered.

Future Goal after merging this PR:

1. Fix all the code which is not covered and reach up to 90% code coverage
   
   1. We do need documentation work for this on how to fix
   2. Once documentation is done, file multiple issues to reach 90% result
   3. How can we block code coverage to run over auto-generated files

2. How we can do the code coverage in GitHub actions (CI).

3. How Bazel can do the code coverage for us.
   
   1. Set up code coverage using Bazel and remove Gradle

Done.

PTAL
https://gist.github.com/peculiaruc/2e63953249c1af8d731246f737fec588

anandwana001 · 2021-02-03T03:31:48Z

@peculiaruc have you checked my comment here - https://gist.github.com/peculiaruc/2e63953249c1af8d731246f737fec588#gistcomment-3616265

Also, include the AudioTest file mentioned in the gist if you are fixing that file for code coverage.
Also, please remove the .idea/gradle.xml from this PR as we don't change that file.
Please check why the GitHub actions are not running, I guess something is blocking to run the GitHub actions.

peculiaruc · 2021-02-04T17:38:51Z

Alright. This PR stopped running since I observed it was made a draft so i thought that's why it doesn't sync. Please can i get any help with it

…

On Wed, 3 Feb 2021 at 04:32, Akshay Nandwana ***@***.***> wrote: @peculiaruc <https://github.com/peculiaruc> have you checked my comment here - https://gist.github.com/peculiaruc/2e63953249c1af8d731246f737fec588#gistcomment-3616265 Also, include the AudioTest file mentioned in the gist if you are fixing that file for code coverage. Also, please remove the .idea/gradle.xml from this PR as we don't change that file. Please check why the GitHub actions are not running, I guess something is blocking to run the GitHub actions. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2466 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIOVAV6RSCGXRNC5YJFSZATS5C7TFANCNFSM4V7NMR7Q> .

anandwana001 · 2021-02-08T04:26:10Z

Making a draft PR doesn't stop the GitHub actions to run.

I suggest remove the changes made in .idea/gradle.xml
The possible reason I see here is that we are having the update in the Gradle file, this somehow breaks our actions.

anandwana001 · 2021-02-08T04:29:29Z

There are some conflicts in this PR, please resolve them and see if GitHub actions run.

peculiaruc · 2021-02-08T06:21:03Z

Alright

…

On Mon, 8 Feb 2021, 5:29 AM Akshay Nandwana, ***@***.***> wrote: There are some conflicts in this PR, please resolve them and see if GitHub actions run. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2466 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIOVAV4CRQLD7RBHEKQNKELS55SDRANCNFSM4V7NMR7Q> .

anandwana001 · 2021-02-09T03:05:39Z

.idea/gradle.xml

@@ -1,26 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<project version="4">


dont' delete this file, just remove it from this PR only.

peculiaruc · 2021-02-09T15:33:25Z

There are some conflicts in this PR, please resolve them and see if GitHub actions run.

Conflicts are resolved

anandwana001 · 2021-02-09T15:39:44Z

Looks like pr has a lot of unwanted changes, it's showing 4k files.

peculiaruc · 2021-02-10T23:54:13Z

Looks like pr has a lot of unwanted changes, it's showing 4k files.

Yes this happened when i merge develop into my branch. Now my branch is up to date but static checks didn't still pass

BenHenning · 2021-02-11T00:42:22Z

@peculiaruc it looks like you copied in an entirely other project into your branch. None of these files seem relevant to Oppia Android. Did you accidentally merge another project with your Oppia Android repository then push that?

peculiaruc · 2021-02-11T05:07:05Z

This happened accidentally. I didn't know. So sorry about it. I will need to clone a new one

…

On Thu, 11 Feb 2021, 1:42 AM Ben Henning, ***@***.***> wrote: @peculiaruc <https://github.com/peculiaruc> it looks like you copied in an entirely other project into your branch. None of these files seem relevant to Oppia Android. Did you accidentally merge another project with your Oppia Android repository then push that? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2466 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIOVAVZSL5BIOHLNTNJOILDS6MRXZANCNFSM4V7NMR7Q> .

Code Coverage Setup

e3c1d39

peculiaruc requested a review from BenHenning as a code owner January 12, 2021 15:59

peculiaruc assigned BenHenning Jan 12, 2021

BenHenning marked this pull request as draft January 13, 2021 02:29

BenHenning assigned peculiaruc and unassigned BenHenning Jan 13, 2021

peculiaruc assigned BenHenning Jan 15, 2021

peculiaruc closed this Jan 15, 2021

peculiaruc reopened this Jan 15, 2021

BenHenning removed their assignment Jan 16, 2021

peculiaruc added 2 commits January 18, 2021 16:11

Code Coverage Setup

925a669

Code Coverage Setup

4613438

peculiaruc assigned BenHenning Jan 20, 2021

BenHenning removed their assignment Jan 29, 2021

peculiaruc added 4 commits February 5, 2021 23:49

Code Coverage Setup

5e39547

Code Coverage Setup

a8156a3

Code Coverage Setup

b7627d0

Code Coverage Setup

82a273f

anandwana001 marked this pull request as ready for review February 8, 2021 04:28

anandwana001 requested a review from vinitamurthi as a code owner February 8, 2021 04:28

peculiaruc added 2 commits February 9, 2021 03:46

Removed Changes from idea/gradle.xml

8931a49

Removed Changes from idea/gradle.xml

65f08a4

anandwana001 suggested changes Feb 9, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/develop' into code_coverae

ed0b0b8

peculiaruc closed this Feb 13, 2021

anandwana001 mentioned this pull request Feb 19, 2021

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

Closed

8 tasks

		@@ -1,26 +0,0 @@
		<?xml version="1.0" encoding="UTF-8"?>
		<project version="4">

Fix issue #1497: Set up code coverage for Robolectric tests #2466

Fix issue #1497: Set up code coverage for Robolectric tests #2466

Conversation

peculiaruc commented Jan 12, 2021 • edited by anandwana001 Loading

Explanation

Checklist

BenHenning commented Jan 13, 2021

anandwana001 commented Jan 13, 2021

peculiaruc commented Jan 13, 2021

peculiaruc commented Jan 13, 2021 • edited Loading

anandwana001 commented Jan 13, 2021

peculiaruc commented Jan 13, 2021

anandwana001 commented Jan 13, 2021

peculiaruc commented Jan 13, 2021

BenHenning commented Jan 13, 2021

BenHenning commented Jan 13, 2021

peculiaruc commented Jan 14, 2021

peculiaruc commented Jan 15, 2021

peculiaruc commented Jan 15, 2021 • edited Loading

BenHenning commented Jan 16, 2021 • edited Loading

peculiaruc commented Jan 16, 2021

peculiaruc commented Jan 20, 2021 • edited Loading

BenHenning commented Jan 21, 2021

BenHenning commented Jan 29, 2021 • edited Loading

BenHenning commented Jan 29, 2021

peculiaruc commented Jan 29, 2021 via email

peculiaruc commented Feb 1, 2021 via email

anandwana001 commented Feb 2, 2021

peculiaruc commented Feb 3, 2021

peculiaruc commented Feb 3, 2021

The single goal of this PR:

Future Goal after merging this PR:

anandwana001 commented Feb 3, 2021 • edited Loading

peculiaruc commented Feb 4, 2021 via email • edited Loading

anandwana001 commented Feb 8, 2021

anandwana001 commented Feb 8, 2021

peculiaruc commented Feb 8, 2021 via email

anandwana001 Feb 9, 2021

Choose a reason for hiding this comment

peculiaruc Feb 9, 2021

Choose a reason for hiding this comment

peculiaruc commented Feb 9, 2021

anandwana001 commented Feb 9, 2021

peculiaruc commented Feb 10, 2021

BenHenning commented Feb 11, 2021

peculiaruc commented Feb 11, 2021 via email

peculiaruc commented Jan 12, 2021 •

edited by anandwana001

Loading

peculiaruc commented Jan 13, 2021 •

edited

Loading

peculiaruc commented Jan 15, 2021 •

edited

Loading

BenHenning commented Jan 16, 2021 •

edited

Loading

peculiaruc commented Jan 20, 2021 •

edited

Loading

BenHenning commented Jan 29, 2021 •

edited

Loading

anandwana001 commented Feb 3, 2021 •

edited

Loading

peculiaruc commented Feb 4, 2021 via email •

edited

Loading