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

Reduced jacoco exclusions and added more tests #446

Merged
merged 3 commits into from
Mar 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 9 additions & 21 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refer to this PR to write unit test for REST actions opensearch-project/ml-commons#228?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will take a look! Will try to add those in a separate PR if thats fine

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that's fine.

'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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
12 changes: 12 additions & 0 deletions src/test/java/org/opensearch/ad/TestHelpers.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ private DetectorProfile createRandomDetectorProfile() {
)
.shingleSize(randomInt())
.coordinatingNode(randomAlphaOfLength(10))
.totalSizeInBytes(-1)
.totalEntities(randomLong())
.activeEntities(randomLong())
.adTaskProfile(
Expand Down Expand Up @@ -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());
}
}
20 changes: 18 additions & 2 deletions src/test/java/org/opensearch/ad/model/EntityProfileTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand All @@ -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));
}
}
Loading