Skip to content

Add Coverage Report for PR#350

Merged
andrewtavis merged 8 commits intoscribe-org:mainfrom
angrezichatterbox:feat/coverage-report
Apr 4, 2025
Merged

Add Coverage Report for PR#350
andrewtavis merged 8 commits intoscribe-org:mainfrom
angrezichatterbox:feat/coverage-report

Conversation

@angrezichatterbox
Copy link
Member

Contributor checklist


Description

The PR introduces coverage report for the PR.

Related issue

@github-actions
Copy link

Thank you for the pull request! ❤️

The Scribe-Android team will do our best to address your contribution as soon as we can. If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Android rooms once you're in. Also consider attending our bi-weekly Saturday dev syncs. It'd be great to meet you 😊

@github-actions
Copy link

github-actions bot commented Mar 28, 2025

Maintainer Checklist

The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

  • The linting and formatting workflows within the PR checks do not indicate new errors in the files changed

  • The CHANGELOG has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

@angrezichatterbox
Copy link
Member Author

angrezichatterbox commented Mar 28, 2025

I would like to know should all the individual packages be shown their coverage amount.

From Jacoco I would get package wise coverage report in the main document which would include the coverage for each package.

@angrezichatterbox angrezichatterbox linked an issue Mar 28, 2025 that may be closed by this pull request
2 tasks
@angrezichatterbox
Copy link
Member Author

Alicia has setup the basis for this long back. I had to add some sample tests for going through the classes to make it work properly. I also noticed that the repository doesn't allow git actions to add comments. So should I make the output from coverage tests run more clean.

@angrezichatterbox
Copy link
Member Author

Some minor changes are required. I will make it and comment in here when it is ready for review :)

@andrewtavis
Copy link
Member

I guess that a massive coverage report for all the files being commented in would be a bit much, so keeping it in the action output and checking a threshold to fail against if it's below it would be enough? What do you think, @angrezichatterbox? :)

@angrezichatterbox
Copy link
Member Author

I guess that a massive coverage report for all the files being commented in would be a bit much, so keeping it in the action output and checking a threshold to fail against if it's below it would be enough? What do you think, @angrezichatterbox? :)

So I would want to make it fail if the overall coverage is less than a certain value right ?

@andrewtavis
Copy link
Member

Exactly, and for now just use the current coverage rounded to the highest 5%. We're likely not that high anyway, so hopefully we can at least do 5%, but if not let's just set it to 0 and increase it with new testing issues as they're finished ✅

@angrezichatterbox
Copy link
Member Author

Exactly, and for now just use the current coverage rounded to the highest 5%. We're likely not that high anyway, so hopefully we can at least do 5%, but if not let's just set it to 0 and increase it with new testing issues as they're finished ✅

I think it is currently 0 or 0.15 %.

Jacoco bot currently return the code coverage of the entire codebase and the changed file. The thing is git actions bot is not able to comment it right now which I am not sure why. I would made the output more clear and will bring in the updated changes by tomorrow.

@andrewtavis
Copy link
Member

Ok :) It's also totally fine if we need to check the workflow for the coverage percent, but I trust you to find the appropriate solution 😊

@angrezichatterbox
Copy link
Member Author

So this is what I have done.

I am returning the coverage percentage current. I have not made it fail since currently it's 0 percent. I have also returned the coverage report as an artifact.

@andrewtavis
Copy link
Member

Is it possible to also get the coverage report to be printed in the terminal of the job so we can check it directly, @angrezichatterbox? I checked the courage report workflow and did see the artifact being uploaded, but it would also be nice to see the results directly :)

@angrezichatterbox
Copy link
Member Author

Is it possible to also get the coverage report to be printed in the terminal of the job so we can check it directly, @angrezichatterbox? I checked the courage report workflow and did see the artifact being uploaded, but it would also be nice to see the results directly :)

Okay I will add that change and sent in the updated PR.

@angrezichatterbox angrezichatterbox force-pushed the feat/coverage-report branch 6 times, most recently from 590885d to 2d8b2fc Compare March 30, 2025 18:01
@angrezichatterbox angrezichatterbox force-pushed the feat/coverage-report branch 2 times, most recently from f74545e to 8320c8d Compare March 30, 2025 18:14
@angrezichatterbox
Copy link
Member Author

Currently I am getting an XML, I could also have a html doc. The idea which comes to my mind is to parse this XML doc and print it as a proper output using a python script. This is what comes to my mind. The existing git actions present in marketplace are now outdated which does the same purpose. Others just give for changed file and the overall coverage.

@andrewtavis
Copy link
Member

Doing a Python or Kotlin file to parse the results would be totally fine, @angrezichatterbox! Maybe doing a Python file instead of a Swift file for scribe-org/Scribe-iOS#532 might also make sense as for some reason the Swift file isn't working 🤔 Feel free to add that in and then we'll be good to go!

@angrezichatterbox
Copy link
Member Author

Doing a Python or Kotlin file to parse the results would be totally fine, @angrezichatterbox! Maybe doing a Python file instead of a Swift file for scribe-org/Scribe-iOS#532 might also make sense as for some reason the Swift file isn't working 🤔 Feel free to add that in and then we'll be good to go!

I will try using Kotlin but I guess python would be better. If Kotlin does not work I would use python.

@andrewtavis
Copy link
Member

Also feel free to just use a Python file. I honestly think that that might be the problem with my workflow for iSO 🤔

@angrezichatterbox
Copy link
Member Author

I have parsed the report and print it on the actions console. It is ready for review :)

Copy link
Member

@andrewtavis andrewtavis left a comment

Choose a reason for hiding this comment

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

Amazing work here, @angrezichatterbox! Great that we're getting this done so early in the app's history, and also great that we have it done right before a sync and can discuss next steps 😊 Really awesome that we'll be able to be so directed in our test development, and from there we can work towards test-driven development 🥳

@andrewtavis andrewtavis merged commit 83d97e5 into scribe-org:main Apr 4, 2025
5 checks passed
@andrewtavis andrewtavis mentioned this pull request Apr 4, 2025
2 tasks
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.

Add coverage report to PRs

2 participants