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

[LD-139] chore: test coverage report #162

Merged
merged 10 commits into from
Nov 21, 2023
Merged

Conversation

trOnk12
Copy link
Contributor

@trOnk12 trOnk12 commented Oct 24, 2023

Pull-Request

Description

  • Add per pull request code coverage report generation.

Why?

  • There is no way to monitor the code coverage increase of decrease, in order to maintain high quality code, we want to have a tool allowing us to monitor the changes.

What?

  • Add a gradle configuration allowing to generate the report and add a task correctly configuring the coverage report settings.

Links to related issues

(https://loudius.atlassian.net/browse/LD-139)

Demo

image

How to test

  • Try to add some code uncovered by unit tests
  • Push the changes, there should be a PR comment added indicating a decrease in code coverage

Documentation

Github action used to create a PR comment
Jacoco Library genereting the coverage report

Checklist

  • Functionality is covered by unit tests ( not applicable )
  • Functionality is covered by integration tests ( not applicable )
  • I've updated PR description with relevant information
  • I've done self code review
  • I've manually tested if the code and app works

@trOnk12 trOnk12 changed the title chore: coverage_test_report chore: test coverage report Oct 24, 2023
@github-actions
Copy link

github-actions bot commented Oct 24, 2023

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ ACTION actionlint 1 0 0.06s
✅ COPYPASTE jscpd yes no 2.51s
✅ REPOSITORY checkov yes no 11.92s
✅ REPOSITORY devskim yes no 0.99s
✅ REPOSITORY dustilock yes no 0.23s
✅ REPOSITORY gitleaks yes no 1.25s
✅ REPOSITORY git_diff yes no 0.06s
✅ REPOSITORY secretlint yes no 2.19s
✅ REPOSITORY syft yes no 0.23s
✅ SPELL misspell 1 0 0 0.1s
✅ YAML prettier 1 0 0 0.77s
✅ YAML v8r 1 0 2.09s
✅ YAML yamllint 1 0 0.29s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@trOnk12 trOnk12 marked this pull request as draft October 24, 2023 12:36
@Krzysiudan
Copy link
Collaborator

Overall Project 72.41%

There is no coverage information present for the Files changed

@trOnk12 trOnk12 marked this pull request as ready for review October 25, 2023 09:42
Copy link
Collaborator

@nowakweronika nowakweronika left a comment

Choose a reason for hiding this comment

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

Can you describe a little what the crucial part of code is doing?😅

@@ -36,14 +38,30 @@ jobs:
uses: ./.github/actions/prepare-android-env

- name: Run test
run: ./gradlew test
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question: Why is this removed?

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 is replaced by a task that I created codeCoverage

uses: madrapps/[email protected]
with:
paths: |
*/build/reports/jacoco/codeCoverage/codeCoverage.xml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here we pass the path to the codeCoverage report in xml format to the action

@@ -93,6 +94,50 @@ android {
}
}

tasks.register('codeCoverage', JacocoReport) {
dependsOn 'testDebugUnitTest'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

dependsOn means that it will execute testDebugUnitTest when triggering codeCoverage and then all that is defined here

"src/main/kotlin/**"
]))

executionData.setFrom(fileTree(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The location of "execution data", i.e. the coverage data collected by the unit test runner when running the tests against classes instrumented by the JaCoCo agent.

xml.required.set(true)
}

classDirectories.setFrom(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

: The compiled class files without instrumentation. Execution data references instructions in these classes, so they're necessary for locating the actual instructions.

classDirectories.setFrom(
fileTree(project.buildDir) {
include("**/tmp/kotlin-classes/debug/**")
exclude(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can exclude some classes so that we they are not included in the code coverage report

}
)

sourceDirectories.setFrom(fileTree(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the location of the original source code files. Optional, but allows the reporter to map bytecode instructions back to the original sources in order to include them into the report with coverage annotations.

@Krzysiudan
Copy link
Collaborator

Overall Project 72.41%

There is no coverage information present for the Files changed

@trOnk12 trOnk12 changed the title chore: test coverage report [LD-138] chore: test coverage report Oct 25, 2023
@Krzysiudan
Copy link
Collaborator

Overall Project 72.41%

There is no coverage information present for the Files changed

@trOnk12 trOnk12 changed the title [LD-138] chore: test coverage report [LD-139] chore: test coverage report Oct 25, 2023
with:
paths: |
*/build/reports/jacoco/codeCoverage/codeCoverage.xml
token: ${{ secrets.PAT || secrets.GITHUB_TOKEN }}
Copy link
Member

Choose a reason for hiding this comment

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

question: why we have here: ${{ secrets.PAT || secrets.GITHUB_TOKEN }}?

Copy link

Overall Project 72.53%

There is no coverage information present for the Files changed

Copy link
Member

@jacek-marchwicki jacek-marchwicki left a comment

Choose a reason for hiding this comment

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

lgtm, but keep in mind that we need to continue to work on this subject to add support for merging test reports

@nowakweronika nowakweronika merged commit 28846e2 into develop Nov 21, 2023
10 checks passed
@nowakweronika nowakweronika deleted the chore/report_test_coverage branch January 16, 2024 09:31
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