Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Improve Test coverage #251

Merged
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
6 changes: 3 additions & 3 deletions .github/workflows/gradle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ jobs:
- name: Build PA and run Unit Tests
working-directory: ./tmp/pa
run: ./gradlew build
- name: Run Integration Tests
working-directory: ./tmp/pa
run: ./gradlew integTest -Dtests.enableIT -Dtests.useDockerCluster
- name: Generate Jacoco coverage report
working-directory: ./tmp/pa
run: ./gradlew jacocoTestReport
Expand All @@ -59,3 +56,6 @@ jobs:
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
run: bash <(curl -s https://codecov.io/bash) -f ./build/reports/jacoco/test/jacocoTestReport.xml
- name: Run Integration Tests
working-directory: ./tmp/pa
run: ./gradlew integTest -Dtests.enableIT -Dtests.useDockerCluster
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ jacocoTestCoverageVerification {
violationRules {
rule {
limit {
minimum = 0.19
minimum = 0.31
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ static class CacheMaxSizeStatus extends MetricStatus {
private final String cacheType;

@JsonInclude(Include.NON_NULL)
private final long cacheMaxSize;
private final Long cacheMaxSize;

CacheMaxSizeStatus(String cacheType, Long cacheMaxSize) {
this.cacheType = cacheType;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package com.amazon.opendistro.elasticsearch.performanceanalyzer.collectors;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Missing license header.


import com.amazon.opendistro.elasticsearch.performanceanalyzer.ESResources;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.collectors.CacheConfigMetricsCollector.CacheMaxSizeStatus;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.AllMetrics.CacheType;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.MetricsConfiguration;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.reader_writer_shared.Event;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.util.TestUtil;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.module.paranamer.ParanamerModule;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.test.ESSingleNodeTestCase;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

public class CacheConfigMetricsCollectorTests extends ESSingleNodeTestCase {
private static final String TEST_INDEX = "test";
private CacheConfigMetricsCollector collector;

@Before
public void init() {
IndicesService indicesService = getInstanceFromNode(IndicesService.class);
ESResources.INSTANCE.setIndicesService(indicesService);

MetricsConfiguration.CONFIG_MAP.put(CacheConfigMetricsCollector.class, MetricsConfiguration.cdefault);
collector = new CacheConfigMetricsCollector();
}

@After
public void tearDown() throws Exception {
super.tearDown();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add saveMetricsValues test here as well?

@Test
public void testCollectMetrics() throws IOException {
long startTimeInMills = 1153721339;

createIndex(TEST_INDEX);
collector.collectMetrics(startTimeInMills);

List<CacheMaxSizeStatus> metrics = readMetrics();
assertEquals(2, metrics.size());
CacheMaxSizeStatus filedDataCache = metrics.get(0);
CacheMaxSizeStatus shardRequestCache = metrics.get(1);
assertEquals(CacheType.FIELD_DATA_CACHE.toString(), filedDataCache.getCacheType());
assertEquals(CacheType.SHARD_REQUEST_CACHE.toString(), shardRequestCache.getCacheType());
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we assert values for Cache Sizes as well?

}

private List<CacheMaxSizeStatus> readMetrics() throws IOException {
List<Event> metrics = TestUtil.readEvents();
assert metrics.size() == 1;
ObjectMapper objectMapper = new ObjectMapper().registerModule(new ParanamerModule());

List<CacheMaxSizeStatus> list = new ArrayList<>();
String[] jsonStrs = metrics.get(0).value.split("\n");
assert jsonStrs.length == 3;
for (int i = 1; i < 3; i++) {
list.add(objectMapper.readValue(jsonStrs[i], CacheMaxSizeStatus.class));
}
return list;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package com.amazon.opendistro.elasticsearch.performanceanalyzer.collectors;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: missing license header


import static org.mockito.MockitoAnnotations.initMocks;

import com.amazon.opendistro.elasticsearch.performanceanalyzer.ESResources;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.collectors.NodeDetailsCollector.NodeDetailsStatus;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.config.overrides.ConfigOverridesWrapper;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.AllMetrics.NodeRole;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.MetricsConfiguration;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.reader_writer_shared.Event;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.util.TestUtil;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.module.paranamer.ParanamerModule;
import java.io.IOException;
import java.util.Collections;
import java.util.List;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.test.ClusterServiceUtils;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.threadpool.TestThreadPool;
import org.elasticsearch.threadpool.ThreadPool;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;

public class NodeDetailsCollectorTests extends ESTestCase {
private static final String NODE_ID = "testNode";
private NodeDetailsCollector collector;
private ThreadPool threadPool;

@Mock
private ConfigOverridesWrapper configOverrides;

@Before
public void init() {
initMocks(this);

DiscoveryNode testNode = new DiscoveryNode(NODE_ID, ESTestCase.buildNewFakeTransportAddress(), Collections
.emptyMap(),
DiscoveryNodeRole.BUILT_IN_ROLES, Version.CURRENT);

threadPool = new TestThreadPool("test");
ClusterService clusterService = ClusterServiceUtils.createClusterService(threadPool, testNode);
ESResources.INSTANCE.setClusterService(clusterService);

MetricsConfiguration.CONFIG_MAP.put(NodeDetailsCollector.class, MetricsConfiguration.cdefault);
collector = new NodeDetailsCollector(configOverrides);
}

@After
public void tearDown() throws Exception {
threadPool.shutdownNow();
super.tearDown();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add saveMetricsValues test here as well?

@Test
public void testCollectMetrics() throws IOException {
long startTimeInMills = 1153721339;
collector.collectMetrics(startTimeInMills);
NodeDetailsStatus nodeDetailsStatus = readMetrics();

assertEquals(NODE_ID, nodeDetailsStatus.getID());
assertEquals("0.0.0.0", nodeDetailsStatus.getHostAddress());
assertEquals(NodeRole.DATA.role(), nodeDetailsStatus.getRole());
assertTrue(nodeDetailsStatus.getIsMasterNode());
}

private NodeDetailsStatus readMetrics() throws IOException {
List<Event> metrics = TestUtil.readEvents();
assert metrics.size() == 1;
ObjectMapper objectMapper = new ObjectMapper().registerModule(new ParanamerModule());
String[] jsonStrs = metrics.get(0).value.split("\n");
assert jsonStrs.length == 4;
return objectMapper.readValue(jsonStrs[3], NodeDetailsStatus.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,64 +15,120 @@

package com.amazon.opendistro.elasticsearch.performanceanalyzer.collectors;

import org.junit.Ignore;
import org.junit.Test;
import static org.mockito.MockitoAnnotations.initMocks;

import com.amazon.opendistro.elasticsearch.performanceanalyzer.CustomMetricsLocationTestBase;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.config.PluginSettings;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.ESResources;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.config.PerformanceAnalyzerController;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.AllMetrics.ShardStatsValue;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.MetricsConfiguration;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.PerformanceAnalyzerMetrics;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

@Ignore
public class NodeStatsFixedShardsMetricsCollectorTests extends CustomMetricsLocationTestBase {
import com.amazon.opendistro.elasticsearch.performanceanalyzer.reader_writer_shared.Event;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.util.TestUtil;
import java.util.List;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.test.ESSingleNodeTestCase;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.Mockito;

@Test
public void testNodeStatsMetrics() {
System.setProperty("performanceanalyzer.metrics.log.enabled", "False");
long startTimeInMills = 1253722339;
public class NodeStatsFixedShardsMetricsCollectorTests extends ESSingleNodeTestCase {
private static final String TEST_INDEX = "test";
private static long startTimeInMills = 1153721339;
private NodeStatsFixedShardsMetricsCollector collector;

MetricsConfiguration.CONFIG_MAP.put(NodeStatsFixedShardsMetricsCollector.class, MetricsConfiguration.cdefault);
@Mock
private PerformanceAnalyzerController controller;

NodeStatsFixedShardsMetricsCollector nodeStatsFixedShardsMetricsCollector = new NodeStatsFixedShardsMetricsCollector(null);
nodeStatsFixedShardsMetricsCollector.saveMetricValues("89123.23", startTimeInMills, "NodesStatsIndex", "55");
@Before
public void init() {
initMocks(this);

IndicesService indicesService = getInstanceFromNode(IndicesService.class);
ESResources.INSTANCE.setIndicesService(indicesService);

String fetchedValue = PerformanceAnalyzerMetrics.getMetric(
PluginSettings.instance().getMetricsLocation()
+ PerformanceAnalyzerMetrics.getTimeInterval(startTimeInMills)+"/indices/NodesStatsIndex/55/");
PerformanceAnalyzerMetrics.removeMetrics(PluginSettings.instance().getMetricsLocation()
+ PerformanceAnalyzerMetrics.getTimeInterval(startTimeInMills));
assertEquals("89123.23", fetchedValue);
MetricsConfiguration.CONFIG_MAP.put(NodeStatsAllShardsMetricsCollector.class, MetricsConfiguration.cdefault);
collector = new NodeStatsFixedShardsMetricsCollector(controller);
}

@After
public void tearDown() throws Exception {
super.tearDown();
}

@Test
public void testNodeStatsMetrics() {
try {
nodeStatsFixedShardsMetricsCollector.saveMetricValues("89123.23", startTimeInMills, "NodesStatsIndex");
collector.saveMetricValues("89123.23", startTimeInMills, "NodesStatsIndex");
assertTrue("Negative scenario test: Should have been a RuntimeException", true);
} catch (RuntimeException ex) {
//- expecting exception...only 1 values passed; 2 expected
}

try {
nodeStatsFixedShardsMetricsCollector.saveMetricValues("89123.23", startTimeInMills);
collector.saveMetricValues("89123.23", startTimeInMills);
assertTrue("Negative scenario test: Should have been a RuntimeException", true);
} catch (RuntimeException ex) {
//- expecting exception...only 0 values passed; 2 expected
}

try {
nodeStatsFixedShardsMetricsCollector.saveMetricValues("89123.23", startTimeInMills, "NodesStatsIndex", "55", "123");
collector.saveMetricValues("89123.23", startTimeInMills, "NodesStatsIndex", "55", "123");
assertTrue("Negative scenario test: Should have been a RuntimeException", true);
} catch (RuntimeException ex) {
//- expecting exception...only 3 values passed; 2 expected
}

try {
nodeStatsFixedShardsMetricsCollector.getNodeIndicesStatsByShardField();
collector.getNodeIndicesStatsByShardField();
} catch (Exception exception) {
assertTrue("There shouldn't be any exception in the code; Please check the reflection code for any changes", true);
}

collector = new NodeStatsFixedShardsMetricsCollector(null);
try {
collector.collectMetrics(startTimeInMills);
} catch (Exception exception) {
assertTrue("There shouldn't be any exception in the code; Please check the reflection code for any changes", true);
}
}

@Test
public void testCollectMetrics() {
createIndex(TEST_INDEX);
Mockito.when(controller.getNodeStatsShardsPerCollection()).thenReturn(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a test to check if all the metrics for all the shards are populated in subsequent runs of this collector?

collector.collectMetrics(startTimeInMills);

//cannot make NodeStatsMetricsFixedShardsPerCollectionStatus static to deserialize it, so check with jsonString
String jsonStr = readMetricsInJsonString();
assertTrue(jsonStr.contains(ShardStatsValue.Constants.INDEXING_THROTTLE_TIME_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.REFRESH_COUNT_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.REFRESH_TIME_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.FLUSH_COUNT_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.FLUSH_TIME_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.MERGE_COUNT_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.MERGE_TIME_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.MERGE_CURRENT_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.INDEX_BUFFER_BYTES_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.SEGMENTS_COUNT_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.SEGMENTS_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.TERMS_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.STORED_FIELDS_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.TERM_VECTOR_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.NORMS_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.POINTS_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.DOC_VALUES_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.INDEX_WRITER_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.VERSION_MAP_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.BITSET_MEMORY_VALUE));
assertTrue(jsonStr.contains(ShardStatsValue.Constants.SHARD_SIZE_IN_BYTES_VALUE));
}

private String readMetricsInJsonString() {
List<Event> metrics = TestUtil.readEvents();
assert metrics.size() == 1;
String[] jsonStrs = metrics.get(0).value.split("\n");
assert jsonStrs.length == 2;
return jsonStrs[1];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.amazon.opendistro.elasticsearch.performanceanalyzer.collectors;

import static org.junit.Assert.assertEquals;
import static org.mockito.MockitoAnnotations.initMocks;

import com.amazon.opendistro.elasticsearch.performanceanalyzer.CustomMetricsLocationTestBase;
import com.amazon.opendistro.elasticsearch.performanceanalyzer.ESResources;
Expand All @@ -31,12 +32,10 @@
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.threadpool.ThreadPoolStats;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.Mockito;

@Ignore
public class ThreadPoolMetricsCollectorTests extends CustomMetricsLocationTestBase {

private ThreadPoolMetricsCollector threadPoolMetricsCollector;
Expand All @@ -46,7 +45,8 @@ public class ThreadPoolMetricsCollectorTests extends CustomMetricsLocationTestBa

@Before
public void init() {
mockThreadPool = Mockito.mock(ThreadPool.class);
initMocks(this);

ESResources.INSTANCE.setThreadPool(mockThreadPool);
System.setProperty("performanceanalyzer.metrics.log.enabled", "False");
MetricsConfiguration.CONFIG_MAP.put(ThreadPoolMetricsCollector.class, MetricsConfiguration.cdefault);
Expand Down