From b2ed39115d5f9ffa00db531f9c315b9fddd28ef5 Mon Sep 17 00:00:00 2001 From: Yu Sun <11154022+yu-sun-77@users.noreply.github.com> Date: Sun, 6 Dec 2020 21:20:27 -0800 Subject: [PATCH] Improve Test coverage (#251) * add UT for NodeStatsFixedShardsMetricsCollectorTests * add UT for NodeDetailsCollectorTests * add UT for CacheConfigMetricsCollectorTests * don't ignore ThreadPoolMetricsCollectorTests * run jacoco task before integration test * change min jacoco coverage to 0.31 * clean code * add newLine * add negative scenario tests * use @Mock annotation for codebase consistency --- .github/workflows/gradle.yml | 6 +- build.gradle | 2 +- .../CacheConfigMetricsCollector.java | 2 +- .../CacheConfigMetricsCollectorTests.java | 66 +++++++++++ .../collectors/NodeDetailsCollectorTests.java | 80 +++++++++++++ ...StatsFixedShardsMetricsCollectorTests.java | 110 +++++++++++++----- .../ThreadPoolMetricsCollectorTests.java | 6 +- 7 files changed, 237 insertions(+), 35 deletions(-) create mode 100644 src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/CacheConfigMetricsCollectorTests.java create mode 100644 src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/NodeDetailsCollectorTests.java diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml index 96641222..2258359e 100644 --- a/.github/workflows/gradle.yml +++ b/.github/workflows/gradle.yml @@ -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 @@ -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 diff --git a/build.gradle b/build.gradle index 88a965e4..1ec2963a 100644 --- a/build.gradle +++ b/build.gradle @@ -165,7 +165,7 @@ jacocoTestCoverageVerification { violationRules { rule { limit { - minimum = 0.19 + minimum = 0.31 } } } diff --git a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/CacheConfigMetricsCollector.java b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/CacheConfigMetricsCollector.java index 502cbe00..159cf9b0 100644 --- a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/CacheConfigMetricsCollector.java +++ b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/CacheConfigMetricsCollector.java @@ -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; diff --git a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/CacheConfigMetricsCollectorTests.java b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/CacheConfigMetricsCollectorTests.java new file mode 100644 index 00000000..74cafe3e --- /dev/null +++ b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/CacheConfigMetricsCollectorTests.java @@ -0,0 +1,66 @@ +package com.amazon.opendistro.elasticsearch.performanceanalyzer.collectors; + +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(); + } + + @Test + public void testCollectMetrics() throws IOException { + long startTimeInMills = 1153721339; + + createIndex(TEST_INDEX); + collector.collectMetrics(startTimeInMills); + + List 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()); + } + + private List readMetrics() throws IOException { + List metrics = TestUtil.readEvents(); + assert metrics.size() == 1; + ObjectMapper objectMapper = new ObjectMapper().registerModule(new ParanamerModule()); + + List 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; + } +} diff --git a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/NodeDetailsCollectorTests.java b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/NodeDetailsCollectorTests.java new file mode 100644 index 00000000..9dab133d --- /dev/null +++ b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/NodeDetailsCollectorTests.java @@ -0,0 +1,80 @@ +package com.amazon.opendistro.elasticsearch.performanceanalyzer.collectors; + +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(); + } + + @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 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); + } +} diff --git a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/NodeStatsFixedShardsMetricsCollectorTests.java b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/NodeStatsFixedShardsMetricsCollectorTests.java index 926e6f23..f64390ce 100644 --- a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/NodeStatsFixedShardsMetricsCollectorTests.java +++ b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/NodeStatsFixedShardsMetricsCollectorTests.java @@ -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); + 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 metrics = TestUtil.readEvents(); + assert metrics.size() == 1; + String[] jsonStrs = metrics.get(0).value.split("\n"); + assert jsonStrs.length == 2; + return jsonStrs[1]; } } diff --git a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/ThreadPoolMetricsCollectorTests.java b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/ThreadPoolMetricsCollectorTests.java index 851821d3..caedf153 100644 --- a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/ThreadPoolMetricsCollectorTests.java +++ b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/collectors/ThreadPoolMetricsCollectorTests.java @@ -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; @@ -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; @@ -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);