opensearch-project / anomaly-detection

Identify atypical data and receive automatic notifications
https://opensearch.org/docs/latest/monitoring-plugins/ad/index/
Apache License 2.0
66 stars 74 forks source link

[BUG] jacocoExclusions is very long #424

Open dblock opened 2 years ago

dblock commented 2 years ago

Describe the bug

The jacocoExclusions list is quite long, https://github.com/opensearch-project/anomaly-detection/blob/main/build.gradle#L478. Maybe we're just ignoring coverage thresholds, or they are too high? In either case those thresholds are likely not useful.

Either fix the TODOs and reduce the list, or remove/reduce the coverage thresholds.

Zhangxunmt commented 2 years ago

The restful action layer is covered by integration tests, so they are excluded in the UT. The transport action layer are still missing some coverage. I am adding more unit tests for the following classes in transport layer for AD.

    'org.opensearch.ad.transport.StopDetectorRequest',
    'org.opensearch.ad.transport.StopDetectorResponse',
    'org.opensearch.ad.transport.CronRequest',
    'org.opensearch.ad.AnomalyDetectorRunner',

    'org.opensearch.ad.transport.DeleteAnomalyDetectorTransportAction*',
    'org.opensearch.ad.transport.GetAnomalyDetectorTransportAction*',
    'org.opensearch.ad.transport.SearchAnomalyResultTransportAction*',
    'org.opensearch.ad.transport.SearchAnomalyDetectorInfoTransportAction*',
    'org.opensearch.ad.transport.AnomalyDetectorJobRequest',

    'org.opensearch.ad.transport.DeleteAnomalyResultsTransportAction',
    'org.opensearch.ad.transport.handler.AnomalyResultBulkIndexHandler'

Also, these 2 task runners also needs more tests. We can address them in the next. // https://github.com/opensearch-project/anomaly-detection/issues/241 'org.opensearch.ad.task.ADBatchTaskRunner', 'org.opensearch.ad.task.ADTaskManager',

Zhangxunmt commented 2 years ago

break down this issue into several sub-issues in 492 and 493. The classes that will be removed out of the jacocoExclusions are identified in the above two issues. The rest layer classes are tested in integration tests so we will keep them in the jacocoExclusions list.