Improve unit test coverage for Query Insights

Signed-off-by: Chenyang Ji <[email protected]>
opensearch-project · Sep 18, 2024 · c79308d · c79308d
1 parent 290c701
commit c79308d
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 13 deletions.
diff --git a/src/main/java/org/opensearch/plugin/insights/core/exporter/DebugExporter.java b/src/main/java/org/opensearch/plugin/insights/core/exporter/DebugExporter.java
@@ -20,12 +20,23 @@ public final class DebugExporter implements QueryInsightsExporter {
     /**
      * Logger of the debug exporter
      */
-    private final Logger logger = LogManager.getLogger();
+    private final Logger logger;
 
     /**
-     * Constructor of DebugExporter
+     * Private constructor for singleton pattern.
      */
-    private DebugExporter() {}
+    private DebugExporter() {
+        this.logger = LogManager.getLogger();
+    }
+
+    /**
+     * Package-private constructor for injecting a custom logger, used for testing
+     *
+     * @param logger
+     */
+    DebugExporter(Logger logger) {
+        this.logger = logger;
+    }
 
     private static class InstanceHolder {
         private static final DebugExporter INSTANCE = new DebugExporter();
@@ -34,7 +45,7 @@ private static class InstanceHolder {
     /**
      Get the singleton instance of DebugExporter
      *
-     @return DebugExporter instance
+     * @return DebugExporter instance
      */
     public static DebugExporter getInstance() {
         return InstanceHolder.INSTANCE;

diff --git a/src/test/java/org/opensearch/plugin/insights/core/exporter/DebugExporterTests.java b/src/test/java/org/opensearch/plugin/insights/core/exporter/DebugExporterTests.java
@@ -9,28 +9,45 @@
 package org.opensearch.plugin.insights.core.exporter;
 
 import java.util.List;
+import java.util.Locale;
+
+import org.apache.logging.log4j.core.Logger;
 import org.junit.Before;
 import org.opensearch.plugin.insights.QueryInsightsTestUtils;
 import org.opensearch.plugin.insights.rules.model.SearchQueryRecord;
 import org.opensearch.test.OpenSearchTestCase;
 
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
 /**
- * Granular tests for the {@link DebugExporterTests} class.
+ * Unit tests for the {@link DebugExporterTests} class.
  */
 public class DebugExporterTests extends OpenSearchTestCase {
     private DebugExporter debugExporter;
+    private Logger mockLogger = mock(Logger.class);
 
     @Before
     public void setup() {
-        debugExporter = DebugExporter.getInstance();
+        debugExporter = new DebugExporter(mockLogger);
+    }
+
+    public void testGetInstance() {
+        DebugExporter instance1 = DebugExporter.getInstance();
+        DebugExporter instance2 = DebugExporter.getInstance();
+        assertEquals(instance1, instance2);
+    }
+
+    public void testExport() {;
+        List<SearchQueryRecord> records = QueryInsightsTestUtils.generateQueryInsightRecords(1);
+        debugExporter.export(records);
+        // Verify that the logger received the expected debug message
+        verify(mockLogger).debug(String.format(Locale.ROOT, "QUERY_INSIGHTS_RECORDS: %s", records));
     }
 
-    public void testExport() {
-        List<SearchQueryRecord> records = QueryInsightsTestUtils.generateQueryInsightRecords(2);
-        try {
-            debugExporter.export(records);
-        } catch (Exception e) {
-            fail("No exception should be thrown when exporting query insights data");
-        }
+    public void testClose() {
+        debugExporter.close();
+        // Verify that the logger received the expected debug message
+        verify(mockLogger).debug("Closing the DebugExporter..");
     }
 }
diff --git a/src/test/java/org/opensearch/plugin/insights/core/exporter/LocalIndexExporterTests.java b/src/test/java/org/opensearch/plugin/insights/core/exporter/LocalIndexExporterTests.java
@@ -12,8 +12,11 @@
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.util.Arrays;
 import java.util.List;
 import org.joda.time.format.DateTimeFormat;
 import org.joda.time.format.DateTimeFormatter;
@@ -82,6 +85,14 @@ public void testExportRecordsWithError() {
         }
     }
 
+    public void testExportWithNoRecords() {
+        // Call the export method with no records
+        localIndexExporter.export(null);
+        localIndexExporter.export(List.of());
+        // Verify no interactions with the client
+        verify(client, times(0)).prepareBulk();
+    }
+
     public void testClose() {
         try {
             localIndexExporter.close();

diff --git a/src/test/java/org/opensearch/plugin/insights/core/service/TopQueriesServiceTests.java b/src/test/java/org/opensearch/plugin/insights/core/service/TopQueriesServiceTests.java
@@ -8,8 +8,11 @@
 
 package org.opensearch.plugin.insights.core.service;
 
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 import org.junit.Before;
@@ -19,6 +22,7 @@
 import org.opensearch.plugin.insights.core.exporter.QueryInsightsExporterFactory;
 import org.opensearch.plugin.insights.core.reader.QueryInsightsReaderFactory;
 import org.opensearch.plugin.insights.rules.model.GroupingType;
+import org.opensearch.plugin.insights.rules.model.Measurement;
 import org.opensearch.plugin.insights.rules.model.MetricType;
 import org.opensearch.plugin.insights.rules.model.SearchQueryRecord;
 import org.opensearch.plugin.insights.settings.QueryInsightsSettings;
@@ -42,6 +46,43 @@ public void setup() {
         topQueriesService.setEnabled(true);
     }
 
+    public void testSetAndGetTopNSize() {
+        int newSize = 5;
+        topQueriesService.setTopNSize(newSize);
+        assertEquals(newSize, topQueriesService.getTopNSize());
+    }
+
+    public void testConsumeRecords() {
+        // Prepare some mock SearchQueryRecords
+        SearchQueryRecord record1 = mock(SearchQueryRecord.class);
+        when(record1.getTimestamp()).thenReturn(System.currentTimeMillis());
+        when(record1.getMeasurements()).thenReturn(Collections.singletonMap(MetricType.LATENCY, new Measurement(1000L)));
+        when(record1.getMeasurement(any())).thenReturn(1000L);
+        SearchQueryRecord record2 = mock(SearchQueryRecord.class);
+        when(record2.getTimestamp()).thenReturn(System.currentTimeMillis());
+        when(record2.getMeasurements()).thenReturn(Collections.singletonMap(MetricType.LATENCY, new Measurement(2000L)));
+        when(record2.getMeasurement(any())).thenReturn(2000L);
+        // Consume records
+        topQueriesService.consumeRecords(List.of(record1, record2));
+        // Verify that the topQueriesStore contains the records
+        List<SearchQueryRecord> snapshot = topQueriesService.getTopQueriesCurrentSnapshot();
+        assertEquals(2, snapshot.size());
+    }
+
+    public void testGetTopQueriesRecords() {
+        topQueriesService.setEnabled(true);
+        // Prepare some mock SearchQueryRecords and add to service
+        SearchQueryRecord record1 = mock(SearchQueryRecord.class);
+        when(record1.getTimestamp()).thenReturn(System.currentTimeMillis());
+        when(record1.getMeasurements()).thenReturn(Collections.singletonMap(MetricType.LATENCY, new Measurement(1000L)));
+        topQueriesService.consumeRecords(List.of(record1));
+        // Fetch records
+        List<SearchQueryRecord> result = topQueriesService.getTopQueriesRecords(false, null, null);
+        // Verify the result
+        assertNotNull(result);
+        assertEquals(1, result.size());
+    }
+
     public void testIngestQueryDataWithLargeWindow() {
         final List<SearchQueryRecord> records = QueryInsightsTestUtils.generateQueryInsightRecords(10);
         topQueriesService.consumeRecords(records);