opensearch-sdk-java
opensearch-sdk-java copied to clipboard
[Testing Confirmation] Confirm current testing requirements
As part of the discussion around implementing an organization-wide testing policy, I am visiting each repo to see what tests they currently perform. I am conducting this work on GitHub so that it is easy to reference.
Looking at the Opensearch-SDK-Java repository, it appears there is
| Repository | Unit Tests | Integration Tests | Backwards Compatibility Tests | Additional Tests | Link |
|---|---|---|---|---|---|
| Opensearch-SDK-Java | Validate Gradle Wrapper | https://github.com/opensearch-project/opensearch-sdk-java/issues/470 |
I don't see any requirements for code coverage in the testing documentation. If there are any specific requirements could you respond to this issue to let me know?
If there are any tests I missed or anything you think all repositories in OpenSearch should have for testing please respond to this issue with details.
Hey @scrawfor99 we definitely have unit tests and view coverage:
- jacoco runs during gradle check as part of our CI workflow and codecov creates comments on the PR
- we have a code coverage badge on our README that links to the codecov report for main:
- we have a
gradle diffCoverageoption available for developers to use to test the diff of their code coverage prior to submitting it
We do not have any specific minimum requirements to test code coverage.
- I created a discussion issue #250 regarding testing coverage in diffs, outlining my personal opposition to an arbitrary coverage threshold as a metric, and nobody objected to my philosophy. TLDR: meeting a minimum threshold makes you think you're fine when you're not, and you might fail due to the impossibility of meeting a threshold in which case you end up suppressing the threshold for a lot of classes and it completely loses its benefit.
- Between myself and @saratvemulapalli we frequently visit the project report linked at the badge above, and review the classes, adding tests to improve coverage when needed (for example, #295) and all maintainers review the codecov comments on PRs which show coverage on the diff.
As for Integration tests, yes, we know we need them: https://github.com/opensearch-project/opensearch-sdk-java/issues?q=is%3Aissue+is%3Aopen+integration+label%3Atesting
Hi @dbwiddis,
Thank you for your detailed response. Apologies on incorrectly labeling the table before but thank you for clearing that up. I went ahead and updated things to be accurate. I will also add notes about your other comments and appreciate your help on this front.
Thanks again.