[Backport 3.7] Skip AnalyticsEngineCompatIT when analytics-engine plugin is absent

[ahkcs]

Backport of #5510 to the 3.7 branch.

Description

AnalyticsEngineCompatIT is a coexistence smoke test whose only assertion is that the cluster boots. It is meant to run only in the dedicated :integ-test:analyticsEngineCompatIT Gradle task, which bundles the arrow + analytics-engine plugin stack on a non-security cluster.

The distribution integ-test pipeline behaves differently: it scans every *IT class against the built distribution and runs the with-security variant without the analytics-engine plugin. In that lane the test fails before any test body runs:

org.opensearch.sql.plugin.AnalyticsEngineCompatIT.testClusterStarted
  → org.apache.hc.core5.http.ConnectionClosedException: Connection closed by peer
      at OpenSearchRestTestCase.initClient(OpenSearchRestTestCase.java:216)

Root cause: the test extends a bare OpenSearchRestTestCase, which speaks plain HTTP. Against a TLS-secured cluster the security plugin closes the connection during client init. The existing build.gradle exclude only governs this repo's own integTest task — it does not cover the external distribution pipeline that discovers the class generically.

Fix

Make the test self-inert no matter how it is discovered:

• Extend OpenSearchSQLRestTestCase so the REST client honours the https / user / password system properties of a secured cluster (TLS trust + basic auth), instead of failing the connection.
• Skip via an assumption (assumeTrue) when the analytics-engine plugin is not installed — verified through _cat/plugins. A build without the plugin now reports the test as skipped rather than failed.

Resulting behaviour:

Lane	Before	After
analyticsEngineCompatIT task (plugin present)	pass	pass
Distribution build, no security, no analytics-engine	n/a	skipped
Distribution build, with security, no analytics-engine	FAIL (connection closed)	skipped

Check List

• New functionality includes testing — change is itself an integration test; behaviour validated against secured / unsecured / plugin-absent lanes.
• Commits are signed per the DCO using --signoff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 44c7ed1)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 44c7ed1
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Handle connection failures gracefully The client().performRequest() call can throw IOException which is declared in the method signature, but the test should handle potential connection failures gracefully. Consider wrapping the plugin check in a try-catch block to provide a clearer skip message if the cluster is unreachable. integ-test/src/test/java/org/opensearch/sql/plugin/AnalyticsEngineCompatIT.java [47-51] -Response response = client().performRequest(new Request("GET", "/_cat/plugins?h=component")); -String installedPlugins = TestUtils.getResponseBody(response, true); -assumeTrue( - "analytics-engine plugin not installed — skipping coexistence smoke test", - installedPlugins.contains("analytics-engine")); +try { + Response response = client().performRequest(new Request("GET", "/_cat/plugins?h=component")); + String installedPlugins = TestUtils.getResponseBody(response, true); + assumeTrue( + "analytics-engine plugin not installed — skipping coexistence smoke test", + installedPlugins.contains("analytics-engine")); +} catch (IOException e) { + assumeTrue("Unable to check plugin status: " + e.getMessage(), false); +} Suggestion importance[1-10]: 3 __ Why: The suggestion is technically valid but offers minimal value. The method already declares IOException in its signature (line 46), which is the standard JUnit pattern. Adding a try-catch to convert an IOException into an assumption failure doesn't improve error handling meaningfully, as the exception would already cause the test to fail with a clear stack trace showing the connection issue.	Low

---

Previous suggestions

Suggestions up to commit 44c7ed1

Category	Suggestion	Impact
Possible issue	Add null-safety check for plugin response The plugin check may fail if the response body is null or the API returns an unexpected format. Add null-safety validation for installedPlugins before calling contains() to prevent potential NullPointerException. integ-test/src/test/java/org/opensearch/sql/plugin/AnalyticsEngineCompatIT.java [47-51] Response response = client().performRequest(new Request("GET", "/_cat/plugins?h=component")); String installedPlugins = TestUtils.getResponseBody(response, true); assumeTrue( "analytics-engine plugin not installed — skipping coexistence smoke test", - installedPlugins.contains("analytics-engine")); + installedPlugins != null && installedPlugins.contains("analytics-engine")); Suggestion importance[1-10]: 7 __ Why: Valid defensive programming suggestion that prevents potential NullPointerException if TestUtils.getResponseBody() returns null. While the existing code may work in practice, adding the null check improves robustness and follows best practices for null-safety.	Medium

[github-actions]

Persistent review updated to latest commit 44c7ed1