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

Report test coverage #850

Closed
saratvemulapalli opened this issue Jun 16, 2021 · 8 comments
Closed

Report test coverage #850

saratvemulapalli opened this issue Jun 16, 2021 · 8 comments
Assignees
Labels
CI CI related enhancement Enhancement or improvement to existing feature or request help wanted Extra attention is needed

Comments

@saratvemulapalli
Copy link
Member

saratvemulapalli commented Jun 16, 2021

Report test coverage for all the tests done in OpenSearch.

One of the options include using https://github.com/codecov

@saratvemulapalli saratvemulapalli added enhancement Enhancement or improvement to existing feature or request CI CI related help wanted Extra attention is needed labels Jun 16, 2021
@tlfeng
Copy link
Collaborator

tlfeng commented Jul 8, 2021

Currently code coverage for specific tests can be obtained through Intellij IDEA when running the test manually.
As far as I know, the biggest obstacle to get code coverage report of Elasticsearch using JaCoCo Gradle plugin that mentioned in [ https://github.com/elastic/elasticsearch/issues/ 28867#issuecomment-372714244 ]
has been resolved in elastic/elasticsearch@2b2a3f5.
So using JaCoCo Gradle plugin to collect code coverage for unit tests of OpenSearch is possible, however there are still some known issues in using JaCoCo for integration tests.

JaCoCo Gradle plugin will make all Gradle tasks of type "org.gradle.api.tasks.testing.Test" to provide coverage information.

In OpenSearch, there are several Gradle tasks to run tests:
test, integTest, javaRestTest, yamlRestTest, internalClusterTest, forbiddenApisTest, largeBlobYamlRestTest, mixedClusterTest and singleNodeIntegTest

All the above tasks, except "forbiddenApisTest", are extended by "org.gradle.api.tasks.testing.Test" class, so they can be enbanced by JaCoCo plugin to provide coverage information.

Note:

  1. Gradle "test" task in OpenSearch will execute unit tests (the class filename ends with "Tests"), which is defined by OpenSearchTestBasePlugin.
  2. Gradle "integTest" task is created by OpenSearch in RestTestPlugin, and it includes all class files ends with "IT".

@tlfeng
Copy link
Collaborator

tlfeng commented Jul 13, 2021

There are several sub-projects in the whole repository that don't have tests, which can be found by searching for test.enabled = false in the code base.

@saratvemulapalli
Copy link
Member Author

This is nice. Couple of questions:

  1. How many tests do we have for forbiddenApisTest
  2. How many and which subprojects do not have test.enabled = false

@tlfeng
Copy link
Collaborator

tlfeng commented Aug 9, 2021

How many tests do we have for forbiddenApisTest

I realized that forbiddenApisTest is not a Gradle task defined by OpenSearch, and not used for running test either. So we can ignore it from the test code coverage.

forbiddenApisTest "Runs forbiddenApis checks on 'test' classes.", which is written in the comment in this link ( https://github.com/policeman-tools/forbidden-apis/issues/ 68#issuecomment-138122778 ),

It's defined by https://github.com/policeman-tools/forbidden-apis, for its Gradle plugin, it will register a separate task for each defined sourceSet. "For default Java projects, two tasks are created: forbiddenApisMain and forbiddenApisTest."
(For detail: https://github.com/policeman-tools/forbidden-apis/wiki/GradleUsage
https://github.com/policeman-tools/forbidden-apis/blob/3.1/src/main/resources/de/thetaphi/forbiddenapis/gradle/plugin-init.groovy#L36)

How many and which subprojects do not have test.enabled = false

Sorry my above statement was not correct, the sub-projects that disabled test task (test.enabled = false ) means they don't have unit test, but they may have integration tests.
The sub-projects that don't have unit tests are:

:benchmarks
:rest-api-spec (but has yamlRestTest task)
:client:benchmark
:client:client-benchmark-noop-api-plugin
:client:test
:example-plugins:custom-suggester (but has yamlRestTest task)
:example-plugins:painless-whitelist (but has yamlRestTest task)
:example-plugins:rest-handler (but has javaRestTest and yamlRestTest task)
:example-plugins:script-expert-scoring (but has yamlRestTest task)
:libs:opensearch-cli
:libs:opensearch-plugin-classloader
:modules:geo (but has yamlRestTest task)
:plugins:discovery-azure-classic (but has internalClusterTest and yamlRestTest task)
:plugins:mapper-size (but has internalClusterTest and yamlRestTest task)
:qa:die-with-dignity (but has javaRestTest task)
:qa:os
:qa:wildfly (but has integTest task)
:distribution:tools:java-version-checker
:modules:lang-painless:spi
:test:fixtures:azure-fixture
:test:fixtures:gcs-fixture
:test:fixtures:hdfs-fixture
:test:fixtures:old-elasticsearch
:test:fixtures:s3-fixture

@tlfeng
Copy link
Collaborator

tlfeng commented Sep 30, 2021

@dreamer-89
Copy link
Member

@tlfeng : Thanks for the deep dive and summary of actions to be done. With jenkins still choice of infra; I think we are unblocked on #1320

@dreamer-89
Copy link
Member

dreamer-89 commented Jun 14, 2022

Generated codecov report on local machine and uploaded on server against forked repository. The results shows ~70% code coverage. Waiting on infra side changes for enabling code cov on OpenSearch repo ETA June 24.

https://app.codecov.io/gh/dreamer-89/OpenSearch/

Screen Shot 2022-06-14 at 4 06 47 PM

@peterzhuamazon
Copy link
Member

Hi @dreamer-89 this is complete for now:

ritty27 pushed a commit to ritty27/OpenSearch that referenced this issue May 12, 2024
* Add Support for Hybrid Query Type

Signed-off-by: Varun Jain <[email protected]>

* Add samples, guide and integ tests

Signed-off-by: Varun Jain <[email protected]>

* Removing wildcard imports

Signed-off-by: Varun Jain <[email protected]>

* Adding import

Signed-off-by: Varun Jain <[email protected]>

* Adding import

Signed-off-by: Varun Jain <[email protected]>

---------

Signed-off-by: Varun Jain <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI CI related enhancement Enhancement or improvement to existing feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants