integ tests for exporter n reader #267

KishoreKicha14 · 2025-03-13T11:51:54Z

Description

This PR improves the QueryInsightsExporterIT test by adding the following updates:

Create Document
Configure Top Queries Exporter
Perform Search
Set Latency Window Size
Check the local index creation
Disable and Re-enable Exporter
Set Exporter to Debug Mode

Issues Resolved

Closes [#233]

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

ansjcy · 2025-03-17T21:03:40Z

Thanks! These cases make the exporter tests much more complete. I have several suggestions:

In checkLocalIndices, Can we actually check the content of the indices?
Since we introduced the default index template in Add default index template for query insights local index #254, can we also test if the index template is successfully created?
Can we also test get the historical data from reader? i.e. Call the top queries API with from and end to see if you can get the same data.

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

KishoreKicha14 · 2025-03-21T03:23:00Z

Thanks! These cases make the exporter tests much more complete. I have several suggestions:

In checkLocalIndices, Can we actually check the content of the indices?

Since we introduced the default index template in Add default index template for query insights local index #254, can we also test if the index template is successfully created?

Can we also test get the historical data from reader? i.e. Call the top queries API with from and end to see if you can get the same data.

Added these changes and also added a step to check topqueries-* is green

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

deshsidd · 2025-04-02T19:45:39Z

Triggering the tests again to make sure they are not flaky

ansjcy · 2025-04-02T23:28:58Z

src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterIT.java

    }
+
+    private void createIndexTemplate() throws IOException {


Why do we need to create an index template here? Index template should be created by the exporter.

ansjcy · 2025-04-02T23:31:35Z

src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterIT.java

        Response response = client().performRequest(request);
        Assert.assertEquals(200, response.getStatusLine().getStatusCode());
+        setLatencyWindowSize("1m");
+        createIndexTemplate();
+        waitForWindowToPass(10);


I don't think we need this waitForWindowToPass(10);

Added because giving some time to create the document

ansjcy · 2025-04-02T23:32:26Z

src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterIT.java

+        checkQueryInsightsIndexTemplate();
+        disableLocalIndexExporter();
+        reEnableLocalIndexExporter();
+        setLocalIndexToDebug();


What's the usage of this function? For debugging only?

To check whether it is able to change from local to debug

ansjcy · 2025-04-02T23:35:18Z

src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterIT.java

 import org.junit.Assert;
 import org.opensearch.client.Request;
 import org.opensearch.client.Response;
 import org.opensearch.client.ResponseException;
 import org.opensearch.plugin.insights.QueryInsightsRestTestCase;

-/** Rest Action tests for query  */
+/** Rest Action tests for query */
 public class QueryInsightsExporterIT extends QueryInsightsRestTestCase {


Overall, both of the tests only check the "happy path", but we also need to check for example:

Edge cases with invalid date parameters

Error handling with malformed requests

Filtering functionality with the id parameter (which exists in LocalIndexReader.java)

Other filtering and types.

Detailed Response structure validation

Also:

Date ranges spanning multiple indices

Very large date ranges

ansjcy · 2025-04-02T23:36:04Z

src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterIT.java

+        waitForWindowToPass(10);
+        performSearch();
+
+        waitForWindowToPass(70);


These hardcoded sleep values make the test potentially flaky. Why 10 seconds? Why 70 seconds? Can we add some explainations for future debugging?

Added Explanation

ansjcy · 2025-04-02T23:40:20Z

src/test/java/org/opensearch/plugin/insights/core/reader/QueryInsightsReaderIT.java

+        assertTrue("Expected search results for title='Test Document'", responseContent.contains("\"Test Document\""));
+    }
+
+    private void createDocument() throws IOException {


A lot of these functions are duplicated between Exporter tests and Reader tests, can we make combine them into one to reduce code duplication?

Move all common utility methods into QueryInsightsRestTestCase

ansjcy · 2025-04-02T23:40:48Z

src/test/java/org/opensearch/plugin/insights/core/reader/QueryInsightsReaderIT.java

+
+        try {
+            client().performRequest(new Request("DELETE", "/top_queries"));
+        } catch (ResponseException ignored) {}


We should at least log something here instead of silently ignoring the errors

Added Logger

ansjcy · 2025-04-02T23:42:12Z

src/test/java/org/opensearch/plugin/insights/core/reader/QueryInsightsReaderIT.java

+    }
+
+    private void cleanup(String fullIndexName) throws IOException, InterruptedException {
+        Thread.sleep(10000);


In exporter it sleeps 3 seconds but in here it's 10 seconds, why's the difference?

I think 10 seconds is needed for reader otherwise it is not able to find the Local Index (only for Reader)

ansjcy · 2025-04-02T23:43:34Z

src/test/java/org/opensearch/plugin/insights/core/exporter/QueryInsightsExporterIT.java

+        String fetchResponseContent = new String(fetchResponse.getEntity().getContent().readAllBytes(), StandardCharsets.UTF_8);
+        assertTrue(
+            "Expected title field with value 'Test Document' in query insights data in local Indices",
+            fetchResponseContent.contains("\"Test Document\"")


Can you check more detailed content/fields instead of this simple contains?
Maybe parse it to JSON format first to validate the structure.

Updated the code

ansjcy · 2025-04-02T23:47:04Z

src/test/java/org/opensearch/plugin/insights/core/reader/QueryInsightsReaderIT.java

+import org.opensearch.plugin.insights.QueryInsightsRestTestCase;
+
+public class QueryInsightsReaderIT extends QueryInsightsRestTestCase {
+    public void testQueryInsightsReaderSettings() throws IOException, InterruptedException {


This function doesn't do what the name suggest. We should rename it.

Changed to testQueryInsightsHistoricalTopQueriesRead

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Kishore Kumaar Natarajan added 2 commits March 13, 2025 03:53

Updated integ test for exporter

c063c2b

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Updated Tests

33d6bf4

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

KishoreKicha14 requested review from ansjcy, jainankitk, deshsidd and dzane17 as code owners March 13, 2025 11:51

Checking index content and index template content

2a56543

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

KishoreKicha14 force-pushed the Feat-integ-tests-for-exporter-n-reader branch from ccfd35f to 2a56543 Compare March 20, 2025 20:51

Kishore Kumaar Natarajan added 2 commits March 20, 2025 17:12

Updated Reader IT

197f871

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Updated Exporter and Reader IT

397fcc0

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Kishore Kumaar Natarajan added 5 commits March 24, 2025 23:05

Exporter and Reader Deleted Indices after IT

aafe33f

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Exporter and Reader Deleted Indices after IT

3778062

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Exporter and Reader Deleted Indices after IT

9f89faf

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Updated Exporter and Reader

861f6ae

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Updated Exporter and Reader

779bb3f

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

KishoreKicha14 force-pushed the Feat-integ-tests-for-exporter-n-reader branch from 7930dcc to 779bb3f Compare March 25, 2025 09:17

Kishore Kumaar Natarajan added 12 commits March 25, 2025 02:20

Updated Exporter and Reader

688fd54

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Updated Exporter and Reader

19ff816

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Updated Exporter and Reader

3398489

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Updated Exporter and Reader

865f525

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Updated Exporter and Reader

9135570

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Increased the wait time in Cleanup

bd1ae0e

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Increased the wait time in Cleanup

adee049

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Increased the wait time in Cleanup

a82fd11

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Added logger

3992a1e

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Added logger

ae5b829

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Added logger

066fcc0

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Removed Creating Template

5c882d8

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Kishore Kumaar Natarajan added 14 commits April 1, 2025 14:48

Removed Creating Template

a03589e

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Removed Creating Template

af48ac7

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Removed Creating Template

7d99005

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Removed Creating Template

30d96b0

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Removed Creating Template

44317e2

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Removed Creating Template

75bf1c6

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Removed Creating Template

3302ce6

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Removed Creating Template

3df4ef5

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Removed Creating Template

21a1560

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Removed Creating Template

411d9c6

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Removed Creating Template

af4086b

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Removed Creating Template

473d666

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Removed Logger and add Check Index

1886502

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Removed Logger and add Check Index

9f23db8

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

ansjcy reviewed Apr 3, 2025

View reviewed changes

Kishore Kumaar Natarajan added 2 commits April 7, 2025 13:27

Updated the Check Content

6837408

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

Updated the Check Content

f96251f

Signed-off-by: Kishore Kumaar Natarajan <[email protected]>

KishoreKicha14 force-pushed the Feat-integ-tests-for-exporter-n-reader branch from 67582ac to f96251f Compare April 7, 2025 20:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

integ tests for exporter n reader #267

integ tests for exporter n reader #267

KishoreKicha14 commented Mar 13, 2025

ansjcy commented Mar 17, 2025 •

edited

Loading

KishoreKicha14 commented Mar 21, 2025

deshsidd commented Apr 2, 2025

ansjcy Apr 2, 2025

KishoreKicha14 Apr 7, 2025

ansjcy Apr 2, 2025

KishoreKicha14 Apr 7, 2025

ansjcy Apr 2, 2025

KishoreKicha14 Apr 7, 2025 •

edited

Loading

ansjcy Apr 2, 2025

ansjcy Apr 2, 2025 •

edited

Loading

ansjcy Apr 2, 2025

KishoreKicha14 Apr 7, 2025 •

edited

Loading

ansjcy Apr 2, 2025

KishoreKicha14 Apr 7, 2025

ansjcy Apr 2, 2025

KishoreKicha14 Apr 7, 2025

ansjcy Apr 2, 2025

KishoreKicha14 Apr 7, 2025

ansjcy Apr 2, 2025

KishoreKicha14 Apr 7, 2025

ansjcy Apr 2, 2025

KishoreKicha14 Apr 7, 2025

integ tests for exporter n reader #267

Are you sure you want to change the base?

integ tests for exporter n reader #267

Conversation

KishoreKicha14 commented Mar 13, 2025

Description

Issues Resolved

ansjcy commented Mar 17, 2025 • edited Loading

KishoreKicha14 commented Mar 21, 2025

deshsidd commented Apr 2, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KishoreKicha14 Apr 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ansjcy Apr 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KishoreKicha14 Apr 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ansjcy commented Mar 17, 2025 •

edited

Loading

KishoreKicha14 Apr 7, 2025 •

edited

Loading

ansjcy Apr 2, 2025 •

edited

Loading

KishoreKicha14 Apr 7, 2025 •

edited

Loading