Reduced jacoco exclusions and added more tests (#446)

Signed-off-by: Amit Galitzky <[email protected]>

build.gradle

@@ -478,56 +478,44 @@ task release(type: Copy, group: 'build') {
 List<String> jacocoExclusions = [
         // code for configuration, settings, etc is excluded from coverage
         'org.opensearch.ad.AnomalyDetectorPlugin',
-        'org.opensearch.ad.settings.AnomalyDetectorSettings',
 
-        //TODO: Add more cases for model validation API, both UT and IT
-        //TODO: add more test cases later for these package
-        'org.opensearch.ad.model.*',
+        // rest layer is tested in integration testing mostly, difficult to mock all of it
+        'org.opensearch.ad.rest.*',
+
+        'org.opensearch.ad.model.ModelProfileOnNode',
+        'org.opensearch.ad.model.InitProgressProfile',
         'org.opensearch.ad.rest.*',
-        'org.opensearch.ad.transport.handler.DetectionStateHandler',
         'org.opensearch.ad.AnomalyDetectorJobRunner',
 
-        // Class containing just constants.  Don't need to test
+        // Class containing just constants. Don't need to test
         'org.opensearch.ad.constant.*',
 
-        // mostly skeleton code.  Tested major logic in restful api tests
-        'org.opensearch.ad.settings.EnabledSetting',
-
-        'org.opensearch.ad.common.exception.FeatureNotAvailableException',
-        'org.opensearch.ad.common.exception.AnomalyDetectionException',
+        //'org.opensearch.ad.common.exception.AnomalyDetectionException',
         'org.opensearch.ad.util.ClientUtil',
 
         'org.opensearch.ad.transport.StopDetectorRequest',
         'org.opensearch.ad.transport.StopDetectorResponse',
-        'org.opensearch.ad.transport.StopDetectorTransportAction',
-        'org.opensearch.ad.transport.DeleteDetectorAction',
-        'org.opensearch.ad.transport.CronTransportAction',
         'org.opensearch.ad.transport.CronRequest',
-        'org.opensearch.ad.transport.ADStatsNodesAction',
         'org.opensearch.ad.AnomalyDetectorRunner',
-        'org.opensearch.ad.util.ParseUtils',
 
         // related to transport actions added for security
-        'org.opensearch.ad.transport.StatsAnomalyDetectorTransportAction',
         'org.opensearch.ad.transport.DeleteAnomalyDetectorTransportAction*',
-        'org.opensearch.ad.transport.SearchAnomalyDetectorTransportAction*',
         'org.opensearch.ad.transport.GetAnomalyDetectorTransportAction*',
         'org.opensearch.ad.transport.SearchAnomalyResultTransportAction*',
         'org.opensearch.ad.transport.SearchAnomalyDetectorInfoTransportAction*',
 
-
         // TODO: unified flow caused coverage drop
         'org.opensearch.ad.transport.DeleteAnomalyResultsTransportAction',
         // TODO: fix unstable code coverage caused by null NodeClient issue
         // https://github.com/opensearch-project/anomaly-detection/issues/241
         'org.opensearch.ad.task.ADBatchTaskRunner',
         'org.opensearch.ad.task.ADTaskManager',
-        
+
         //TODO: custom result index caused coverage drop
-        'org.opensearch.ad.indices.AnomalyDetectionIndices',
         'org.opensearch.ad.transport.handler.AnomalyResultBulkIndexHandler'
 ]
 
+
 jacocoTestCoverageVerification {
     violationRules {
         rule {

src/main/java/org/opensearch/ad/model/EntityProfileName.java

@@ -15,6 +15,7 @@
 import java.util.Set;
 
 import org.opensearch.ad.Name;
+import org.opensearch.ad.constant.CommonErrorMessages;
 import org.opensearch.ad.constant.CommonName;
 
 public enum EntityProfileName implements Name {
@@ -50,7 +51,7 @@ public static EntityProfileName getName(String name) {
             case CommonName.MODELS:
                 return MODELS;
             default:
-                throw new IllegalArgumentException("Unsupported profile types");
+                throw new IllegalArgumentException(CommonErrorMessages.UNSUPPORTED_PROFILE_TYPE);
         }
     }
 

src/main/java/org/opensearch/ad/rest/handler/ModelValidationActionHandler.java

@@ -82,7 +82,7 @@
  * different varying intervals in order to find the best interval for the data. If no interval is found with all
  * configuration applied then each configuration is tested sequentially for sparsity</p>
  */
-// TODO: potentially change where this is located
+// TODO: Add more UT and IT
 public class ModelValidationActionHandler {
     protected static final String AGG_NAME_TOP = "top_agg";
     protected static final String AGGREGATION = "agg";

src/test/java/org/opensearch/ad/AnomalyDetectorPluginTests.java

@@ -78,4 +78,9 @@ public void testDeserializeRCFBufferPool() throws Exception {
         assertTrue(null != buffer);
     }
 
+    public void testOverriddenJobTypeAndIndex() {
+        assertEquals("opendistro_anomaly_detector", plugin.getJobType());
+        assertEquals(".opendistro-anomaly-detector-jobs", plugin.getJobIndex());
+    }
+
 }

src/test/java/org/opensearch/ad/AnomalyDetectorProfileRunnerTests.java

@@ -634,4 +634,12 @@ public void testFailRCFPolling() throws IOException, InterruptedException {
         }), stateNError);
         assertTrue(inProgressLatch.await(100, TimeUnit.SECONDS));
     }
+
+    public void testInitProgressProfile() {
+        InitProgressProfile progressOne = new InitProgressProfile("0%", 0, requiredSamples);
+        InitProgressProfile progressTwo = new InitProgressProfile("0%", 0, requiredSamples);
+        InitProgressProfile progressThree = new InitProgressProfile("96%", 2, requiredSamples);
+        assertTrue(progressOne.equals(progressTwo));
+        assertFalse(progressOne.equals(progressThree));
+    }
 }

src/test/java/org/opensearch/ad/TestHelpers.java

@@ -57,6 +57,7 @@
 import org.opensearch.action.search.SearchRequest;
 import org.opensearch.action.search.SearchResponse;
 import org.opensearch.action.search.ShardSearchFailure;
+import org.opensearch.ad.constant.CommonErrorMessages;
 import org.opensearch.ad.constant.CommonName;
 import org.opensearch.ad.constant.CommonValue;
 import org.opensearch.ad.feature.Features;
@@ -1485,4 +1486,15 @@ public static DetectorValidationIssue randomDetectorValidationIssueWithSubIssues
         );
         return issue;
     }
+
+    public static DetectorValidationIssue randomDetectorValidationIssueWithDetectorIntervalRec(long intervalRec) {
+        DetectorValidationIssue issue = new DetectorValidationIssue(
+            ValidationAspect.MODEL,
+            DetectorValidationIssueType.DETECTION_INTERVAL,
+            CommonErrorMessages.DETECTOR_INTERVAL_REC + intervalRec,
+            null,
+            new IntervalTimeConfiguration(intervalRec, ChronoUnit.MINUTES)
+        );
+        return issue;
+    }
 }

src/test/java/org/opensearch/ad/common/exception/ADValidationExceptionTests.java

@@ -11,18 +11,26 @@
 
 package org.opensearch.ad.common.exception;
 
+import org.opensearch.ad.constant.CommonName;
 import org.opensearch.ad.model.DetectorValidationIssueType;
 import org.opensearch.ad.model.ValidationAspect;
 import org.opensearch.test.OpenSearchTestCase;
 
 public class ADValidationExceptionTests extends OpenSearchTestCase {
-    public void testConstructor() {
+    public void testConstructorDetector() {
         String message = randomAlphaOfLength(5);
         ADValidationException exception = new ADValidationException(message, DetectorValidationIssueType.NAME, ValidationAspect.DETECTOR);
         assertEquals(DetectorValidationIssueType.NAME, exception.getType());
         assertEquals(ValidationAspect.DETECTOR, exception.getAspect());
     }
 
+    public void testConstructorModel() {
+        String message = randomAlphaOfLength(5);
+        ADValidationException exception = new ADValidationException(message, DetectorValidationIssueType.CATEGORY, ValidationAspect.MODEL);
+        assertEquals(DetectorValidationIssueType.CATEGORY, exception.getType());
+        assertEquals(ValidationAspect.getName(CommonName.MODEL_ASPECT), exception.getAspect());
+    }
+
     public void testToString() {
         String message = randomAlphaOfLength(5);
         ADValidationException exception = new ADValidationException(message, DetectorValidationIssueType.NAME, ValidationAspect.DETECTOR);

src/test/java/org/opensearch/ad/model/ADEntityTaskProfileTests.java

@@ -31,63 +31,91 @@ protected NamedWriteableRegistry writableRegistry() {
         return getInstanceFromNode(NamedWriteableRegistry.class);
     }
 
-    public void testADEntityTaskProfileSerialization() throws IOException {
-        ADEntityTaskProfile entityTask = new ADEntityTaskProfile(
-            1,
-            23L,
-            false,
-            1,
-            2L,
-            "1234",
-            null,
-            "4321",
-            ADTaskType.HISTORICAL_HC_ENTITY.name()
-        );
-        BytesStreamOutput output = new BytesStreamOutput();
-        entityTask.writeTo(output);
-        NamedWriteableAwareStreamInput input = new NamedWriteableAwareStreamInput(output.bytes().streamInput(), writableRegistry());
-        ADEntityTaskProfile parsedEntityTask = new ADEntityTaskProfile(input);
-        assertEquals(entityTask, parsedEntityTask);
+    private ADEntityTaskProfile createADEntityTaskProfile() {
+        Entity entity = createEntityAndAttributes();
+        return new ADEntityTaskProfile(1, 23L, false, 1, 2L, "1234", entity, "4321", ADTaskType.HISTORICAL_HC_ENTITY.name());
     }
 
-    public void testParseADEntityTaskProfile() throws IOException {
+    private Entity createEntityAndAttributes() {
         TreeMap<String, String> attributes = new TreeMap<>();
         String name1 = "host";
         String val1 = "server_2";
         String name2 = "service";
         String val2 = "app_4";
         attributes.put(name1, val1);
         attributes.put(name2, val2);
-        Entity entity = Entity.createEntityFromOrderedMap(attributes);
+        return Entity.createEntityFromOrderedMap(attributes);
+    }
+
+    public void testADEntityTaskProfileSerialization() throws IOException {
+        ADEntityTaskProfile entityTask = createADEntityTaskProfile();
+        BytesStreamOutput output = new BytesStreamOutput();
+        entityTask.writeTo(output);
+        NamedWriteableAwareStreamInput input = new NamedWriteableAwareStreamInput(output.bytes().streamInput(), writableRegistry());
+        ADEntityTaskProfile parsedEntityTask = new ADEntityTaskProfile(input);
+        assertEquals(entityTask, parsedEntityTask);
+    }
+
+    public void testParseADEntityTaskProfile() throws IOException {
+        ADEntityTaskProfile entityTask = createADEntityTaskProfile();
+        String adEntityTaskProfileString = TestHelpers
+            .xContentBuilderToString(entityTask.toXContent(TestHelpers.builder(), ToXContent.EMPTY_PARAMS));
+        ADEntityTaskProfile parsedEntityTask = ADEntityTaskProfile.parse(TestHelpers.parser(adEntityTaskProfileString));
+        assertEquals(entityTask, parsedEntityTask);
+    }
+
+    public void testParseADEntityTaskProfileWithNullEntity() throws IOException {
         ADEntityTaskProfile entityTask = new ADEntityTaskProfile(
             1,
             23L,
             false,
             1,
             2L,
             "1234",
-            entity,
+            null,
             "4321",
             ADTaskType.HISTORICAL_HC_ENTITY.name()
         );
+        assertEquals(Integer.valueOf(1), entityTask.getShingleSize());
+        assertEquals(23L, (long) entityTask.getRcfTotalUpdates());
+        assertNull(entityTask.getEntity());
         String adEntityTaskProfileString = TestHelpers
             .xContentBuilderToString(entityTask.toXContent(TestHelpers.builder(), ToXContent.EMPTY_PARAMS));
         ADEntityTaskProfile parsedEntityTask = ADEntityTaskProfile.parse(TestHelpers.parser(adEntityTaskProfileString));
         assertEquals(entityTask, parsedEntityTask);
     }
 
-    public void testParseADEntityTaskProfileWithNullEntity() throws IOException {
-        ADEntityTaskProfile entityTask = new ADEntityTaskProfile(
-            1,
-            23L,
+    public void testADEntityTaskProfileEqual() {
+        ADEntityTaskProfile entityTaskOne = createADEntityTaskProfile();
+        ADEntityTaskProfile entityTaskTwo = createADEntityTaskProfile();
+        ADEntityTaskProfile entityTaskThree = new ADEntityTaskProfile(
+            null,
+            null,
             false,
             1,
-            2L,
+            null,
             "1234",
             null,
             "4321",
             ADTaskType.HISTORICAL_HC_ENTITY.name()
         );
+        assertTrue(entityTaskOne.equals(entityTaskTwo));
+        assertFalse(entityTaskOne.equals(entityTaskThree));
+    }
+
+    public void testParseADEntityTaskProfileWithMultipleNullFields() throws IOException {
+        Entity entity = createEntityAndAttributes();
+        ADEntityTaskProfile entityTask = new ADEntityTaskProfile(
+            null,
+            null,
+            false,
+            1,
+            null,
+            "1234",
+            entity,
+            "4321",
+            ADTaskType.HISTORICAL_HC_ENTITY.name()
+        );
         String adEntityTaskProfileString = TestHelpers
             .xContentBuilderToString(entityTask.toXContent(TestHelpers.builder(), ToXContent.EMPTY_PARAMS));
         ADEntityTaskProfile parsedEntityTask = ADEntityTaskProfile.parse(TestHelpers.parser(adEntityTaskProfileString));

src/test/java/org/opensearch/ad/model/AnomalyResultBucketTests.java

@@ -31,9 +31,21 @@ public void testSerializeAnomalyResultBucket() throws IOException {
         assertTrue(parsedAnomalyResultBucket.equals(anomalyResultBucket));
     }
 
+    public void testAnomalyResultBucketEquals() {
+        Map<String, Object> keyOne = new HashMap<>();
+        keyOne.put("test-field-1", "test-value-1");
+        Map<String, Object> keyTwo = new HashMap<>();
+        keyTwo.put("test-field-2", "test-value-2");
+        AnomalyResultBucket testBucketOne = new AnomalyResultBucket(keyOne, 3, 0.5);
+        AnomalyResultBucket testBucketTwo = new AnomalyResultBucket(keyOne, 5, 0.75);
+        AnomalyResultBucket testBucketThree = new AnomalyResultBucket(keyTwo, 7, 0.2);
+        assertFalse(testBucketOne.equals(testBucketTwo));
+        assertFalse(testBucketTwo.equals(testBucketThree));
+    }
+
     @SuppressWarnings("unchecked")
     public void testToXContent() throws IOException {
-        Map<String, Object> key = new HashMap<String, Object>() {
+        Map<String, Object> key = new HashMap<>() {
             {
                 put("test-field-1", "test-value-1");
             }

src/test/java/org/opensearch/ad/model/DetectorInternalStateTests.java

@@ -24,4 +24,18 @@ public void testToXContentDetectorInternalState() throws IOException {
         DetectorInternalState parsedInternalState = DetectorInternalState.parse(TestHelpers.parser(internalStateString));
         assertEquals(internalState, parsedInternalState);
     }
+
+    public void testClonedDetectorInternalState() throws IOException {
+        DetectorInternalState originalState = new DetectorInternalState.Builder()
+            .lastUpdateTime(Instant.ofEpochMilli(100L))
+            .error("error-test")
+            .build();
+        DetectorInternalState clonedState = (DetectorInternalState) originalState.clone();
+        // parse original InternalState
+        String internalStateString = TestHelpers
+            .xContentBuilderToString(originalState.toXContent(TestHelpers.builder(), ToXContent.EMPTY_PARAMS));
+        DetectorInternalState parsedInternalState = DetectorInternalState.parse(TestHelpers.parser(internalStateString));
+        // compare parsed to cloned
+        assertEquals(clonedState, parsedInternalState);
+    }
 }

src/test/java/org/opensearch/ad/model/DetectorProfileTests.java

@@ -41,6 +41,7 @@ private DetectorProfile createRandomDetectorProfile() {
             )
             .shingleSize(randomInt())
             .coordinatingNode(randomAlphaOfLength(10))
+            .totalSizeInBytes(-1)
             .totalEntities(randomLong())
             .activeEntities(randomLong())
             .adTaskProfile(
@@ -87,8 +88,27 @@ public void testDetectorProfileToXContent() throws IOException {
     }
 
     public void testDetectorProfileName() throws IllegalArgumentException {
-        DetectorProfileName.getName(CommonName.AD_TASK);
+        assertEquals("ad_task", DetectorProfileName.getName(CommonName.AD_TASK).getName());
+        assertEquals("state", DetectorProfileName.getName(CommonName.STATE).getName());
+        assertEquals("error", DetectorProfileName.getName(CommonName.ERROR).getName());
+        assertEquals("coordinating_node", DetectorProfileName.getName(CommonName.COORDINATING_NODE).getName());
+        assertEquals("shingle_size", DetectorProfileName.getName(CommonName.SHINGLE_SIZE).getName());
+        assertEquals("total_size_in_bytes", DetectorProfileName.getName(CommonName.TOTAL_SIZE_IN_BYTES).getName());
+        assertEquals("models", DetectorProfileName.getName(CommonName.MODELS).getName());
+        assertEquals("init_progress", DetectorProfileName.getName(CommonName.INIT_PROGRESS).getName());
+        assertEquals("total_entities", DetectorProfileName.getName(CommonName.TOTAL_ENTITIES).getName());
+        assertEquals("active_entities", DetectorProfileName.getName(CommonName.ACTIVE_ENTITIES).getName());
         IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> DetectorProfileName.getName("abc"));
         assertEquals(exception.getMessage(), CommonErrorMessages.UNSUPPORTED_PROFILE_TYPE);
     }
+
+    public void testDetectorProfileSet() throws IllegalArgumentException {
+        DetectorProfile detectorProfileOne = createRandomDetectorProfile();
+        detectorProfileOne.setShingleSize(20);
+        assertEquals(20, detectorProfileOne.getShingleSize());
+        detectorProfileOne.setActiveEntities(10L);
+        assertEquals(10L, (long) detectorProfileOne.getActiveEntities());
+        detectorProfileOne.setModelCount(10L);
+        assertEquals(10L, (long) detectorProfileOne.getActiveEntities());
+    }
 }

src/test/java/org/opensearch/ad/model/EntityProfileTests.java

@@ -27,9 +27,7 @@
 public class EntityProfileTests extends AbstractADTest {
     public void testMerge() {
         EntityProfile profile1 = new EntityProfile(null, -1, -1, null, null, EntityState.INIT);
-
         EntityProfile profile2 = new EntityProfile(null, -1, -1, null, null, EntityState.UNKNOWN);
-
         profile1.merge(profile2);
         assertEquals(profile1.getState(), EntityState.INIT);
         assertTrue(profile1.toString().contains(EntityState.INIT.toString()));
@@ -52,4 +50,22 @@ public void testToXContent() throws IOException, JsonPathNotFoundException {
 
         assertTrue(false == JsonDeserializer.hasChildNode(json, CommonName.STATE));
     }
+
+    public void testToXContentTimeStampAboveZero() throws IOException, JsonPathNotFoundException {
+        EntityProfile profile1 = new EntityProfile(null, 1, 1, null, null, EntityState.INIT);
+
+        XContentBuilder builder = jsonBuilder();
+        profile1.toXContent(builder, ToXContent.EMPTY_PARAMS);
+        String json = Strings.toString(builder);
+
+        assertEquals("INIT", JsonDeserializer.getTextValue(json, CommonName.STATE));
+
+        EntityProfile profile2 = new EntityProfile(null, 1, 1, null, null, EntityState.UNKNOWN);
+
+        builder = jsonBuilder();
+        profile2.toXContent(builder, ToXContent.EMPTY_PARAMS);
+        json = Strings.toString(builder);
+
+        assertTrue(false == JsonDeserializer.hasChildNode(json, CommonName.STATE));
+    }
 }