kiwix / java-libkiwix

Libkiwix binding for Java & Kotlin
https://central.sonatype.com/artifact/org.kiwix/libkiwix
GNU General Public License v3.0
3 stars 4 forks source link

Better testing (coverage) #33

Closed mgautierfr closed 1 year ago

mgautierfr commented 1 year ago

Jacoco don't track native methods, so our code coverage don't include all the native methods, and it is exactly want we want to check.

By introducing a test package wrapping all native methods with a small java function, we have the coverage of the wrapping methods.

codecov-commenter commented 1 year ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (main@8cc3627). Click here to learn what that means. Patch has no changes to coverable lines.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #33 +/- ## ======================================= Coverage ? 41.55% Complexity ? 37 ======================================= Files ? 28 Lines ? 154 Branches ? 6 ======================================= Hits ? 64 Misses ? 88 Partials ? 2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

mgautierfr commented 1 year ago

This PR doesn't had exhaustive tests, but "fix" the test coverage feature. From now one, codecov will correctly report coverage (and improvement of the coverage)

mgautierfr commented 1 year ago

That's the plan, but we cannot for now. The archive is passed to Book.update and it takes a Archive.

We should continue the current work and "clone" all the classes in ork.kiwix.test to have a full coverage reporting and then extend all tests.

mgautierfr commented 1 year ago

That's the plan, but we cannot for now.

This is done now.

MohitMaliFtechiz commented 1 year ago

This is done now.

@mgautierfr, Thanks, it looks good now.

We should continue the current work and "clone" all the classes in ork.kiwix.test to have a full coverage reporting and then extend all tests.

Now only Searcher is left to move in the test package. Are you planning to re-enable Searcher related test in this PR?

mgautierfr commented 1 year ago

Are you planning to re-enable Searcher related test in this PR?

No, it is not necessary to this PR. And it is better to have small PR.