Fix the filter of integTestWithSecurity

[LantaoJin]

Description

Currently, the integTestWithSecurity only enabled the CrossClusterSearchIT and PPLPermissionsIT, but we have other classes such as CalciteCrossClusterSearchIT and CrossClusterCoalesceIT in code which are not enabled due to the integTestWithSecurity task only work with JUnit5.

This PR change them to JUnit4 framework which could trigger all ITs in the task.

Related Issues

Resolves #5097

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• Public documentation issue/PR created.

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.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Chores
  • Refactored test infrastructure for improved cross-cluster testing setup and initialization patterns.
  • Consolidated test base class logic to reduce redundant multi-cluster configuration boilerplate across test suites.
  • Optimized test initialization to prevent unnecessary re-initialization of remote client connections.

Walkthrough

This PR consolidates multi-cluster test initialization logic by introducing a new CrossClusterTestBase class and migrating cross-cluster security integration tests to use it. It also generalizes test filtering in the build configuration and adds a null-check guard for remote client initialization.

Changes

Cohort / File(s)	Summary
Build Configuration integ-test/build.gradle	Removed explicit JUnit Platform usage from integTestWithSecurity task and generalized test filtering from specific test classes to wildcard pattern matching all tests under org.opensearch.sql.security.*.
Test Infrastructure integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java	Added null check guard for remoteClient initialization in configureMultiClusters to ensure idempotent initialization.
Cross-Cluster Test Base integ-test/src/test/java/org/opensearch/sql/security/CrossClusterTestBase.java	Introduced new test base class that determines remote cluster name from system properties and centralizes multi-cluster configuration. Exposes REMOTE_CLUSTER constant and remote index references (TEST_INDEX_\*_REMOTE) for use by subclasses.
Cross-Cluster Test Classes integ-test/src/test/java/org/opensearch/sql/security/CalciteCrossClusterSearchIT.java, CrossClusterCoalesceIT.java, CrossClusterSearchIT.java, PPLPermissionsIT.java	Migrated test classes to extend CrossClusterTestBase instead of PPLIntegTestCase. Removed legacy multi-cluster setup boilerplate (static REMOTE_CLUSTER fields, @BeforeEach initialization) and consolidated initialization into overridden init() methods. Updated test assertions and data preparation to reflect new initialization flow (e.g., additional internal columns in results, adjusted numeric representations, guarded role/user creation).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• #5085 — Touches the same cross-cluster test classes and adjusts Calcite-only test initialization and relocation.

Suggested labels

maintenance

Suggested reviewers

• penghuo
• yuancu
• Swiddis
• vamsimanohar
• RyanL1997
• anirudha
• ps48
• qianheng-aws

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and specifically refers to the main change: fixing the filter of integTestWithSecurity to include all relevant security integration tests.
Description check	✅ Passed	The description clearly explains that integTestWithSecurity was missing CalciteCrossClusterSearchIT and CrossClusterCoalesceIT tests, and the PR fixes this by generalizing the filter.
Linked Issues check	✅ Passed	The PR addresses issue #5097 by generalizing the integTestWithSecurity filter from two specific test classes to a wildcard pattern that includes all security integration tests, ensuring previously missing tests are now included.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the test filter and refactoring test base classes to support cross-cluster testing. No unrelated changes detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

integ-test/src/test/java/org/opensearch/sql/security/PPLPermissionsIT.java (1)

60-99: ⚠️ Potential issue | 🟡 Minor

Use a static initialized flag to prevent redundant cluster-scoped security setup.

The instance field initialized is ineffective across test instances since JUnit creates a new instance per @Before method. With setUpIndices() calling init() before each test, the guard will always evaluate to false, causing security roles and users to be recreated for every test method. While createRole() and createUser() are idempotent PUT requests that don't break test independence, this pattern is inefficient and misleading. Follow the pattern used in GeoIpFunctionsIT—use private static boolean initialized—to correctly guard one-time cluster-scoped setup across all test methods in the class.

🤖 Fix all issues with AI agents

In
`@integ-test/src/test/java/org/opensearch/sql/security/CrossClusterTestBase.java`:
- Around line 16-27: The static initializer in CrossClusterTestBase uses
System.getProperty("cluster.names").split(",") which can NPE if the property is
missing; modify the static block that computes REMOTE_CLUSTER to defensively
handle a null property by reading System.getProperty("cluster.names") into a
local variable, check for null/blank and substitute a sensible default (e.g.,
"remoteCluster" or a single-entry string) or throw a clear
IllegalStateException, then split that safe value and proceed to pick the first
cluster that startsWith("remote") (referencing the REMOTE_CLUSTER symbol and the
local variables clusterNames/remote in the static block).

🧹 Nitpick comments (2)

integ-test/src/test/java/org/opensearch/sql/security/CalciteCrossClusterSearchIT.java (2)

135-136: Inconsistent literal types in verifyDataRows call.

The rows() arguments mix int (451) and Long (504L, 45L) literals. If the matcher performs strict type checking, this inconsistency could cause unexpected test failures. Consider using consistent types.

♻️ Proposed fix for type consistency

-    verifyDataRows(result, rows(451, "20.0-30.0"), rows(504L, "30.0-40.0"), rows(45L, "40.0-50.0"));
+    verifyDataRows(result, rows(451L, "20.0-30.0"), rows(504L, "30.0-40.0"), rows(45L, "40.0-50.0"));

---

188-189: Same type inconsistency as above.

♻️ Proposed fix for type consistency

-    verifyDataRows(result, rows(451, "20-30"), rows(504L, "30-40"), rows(45L, "40-50"));
+    verifyDataRows(result, rows(451L, "20-30"), rows(504L, "30-40"), rows(45L, "40-50"));