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

[WIP] Improve unit test coverage for Query Insights #121

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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..");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand Down
Loading