-
Notifications
You must be signed in to change notification settings - Fork 58
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
Configure PDF Generation #3278
Configure PDF Generation #3278
Conversation
can we add some docs in this PR? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3278 +/- ##
=========================================
- Coverage 28.1% 28.1% -0.1%
- Complexity 718 719 +1
=========================================
Files 265 267 +2
Lines 12885 12948 +63
Branches 2317 2324 +7
=========================================
+ Hits 3630 3641 +11
- Misses 8778 8827 +49
- Partials 477 480 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
c332737
to
dc1484e
Compare
Remaining todos:
|
For some reason, codecov does not detect the coverage from roboelectric test, in this case for: PdfLauncherFragment. |
@ndegwamartin do you have comments on this? |
The same issue might also happen on main, the coverage went from 60% to 28% |
Codecov shows only minimal coverage for that class on your PR , see - PR 3278 on CodeCov @fikri when you run the |
@FikriMilano here's the command that CI runs: ./gradlew clean :quest:fhircoreJacocoReport --stacktrace -Pandroid.testInstrumentationRunnerArguments.notPackage=org.smartregister.fhircore.quest.performance |
@ndegwamartin after running the command, the terminal gave me these, not sure what it means.
|
@FikriMilano I think is just a warning. Did you inspect the generated HTML coverage report and determine whether it has(or doesn't have) the code coverage? |
@ndegwamartin here's the coverage from my local jacoco. We can say that it's not covering the PdfLauncherFragment by much. Is my test the problem or is it jacoco? Btw, we have all tests as |
@FikriMilano could you click on the PdfLauncherFragment class link on the report. Maybe screenshot that as well? |
@ndegwamartin here's the screenshots (my test logs was able to reach line 108, starting from onCreate): |
Yeah so according to Jacoco the |
As discussed w @ndegwamartin So, we'll merge this PR first, then fix the jacoco test report issue in a different PR. cc @pld |
Sounds good let's merge once passing |
IMPORTANT: Where possible all PRs must be linked to a Github issue
Fixes #3235
Engineer Checklist
strings.xml
file./gradlew spotlessApply
and./gradlew spotlessCheck
to check my code follows the project's style guideCode Reviewer Checklist
strings.xml
file