Fix test cluster behavior @ClusterScope(scope = Scope.SUITE) and @Ope…

…nSearchIntegTestCase.SuiteScopeTestCase for parameterized test cases

Signed-off-by: Andriy Redko <[email protected]>

CHANGELOG.md

@@ -209,7 +209,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Use slice_size == shard_size heuristic in terms aggs for concurrent segment search and properly calculate the doc_count_error ([#11732](https://github.com/opensearch-project/OpenSearch/pull/11732))
 - Added Support for dynamically adding SearchRequestOperationsListeners with SearchRequestOperationsCompositeListenerFactory ([#11526](https://github.com/opensearch-project/OpenSearch/pull/11526))
 - Ensure Jackson default maximums introduced in 2.16.0 do not conflict with OpenSearch settings ([#11890](https://github.com/opensearch-project/OpenSearch/pull/11890))
-- Extract cluster management for integration tests into JUnit test rule out of OpenSearchIntegTestCase ([#11877](https://github.com/opensearch-project/OpenSearch/pull/11877))
+- Extract cluster management for integration tests into JUnit test rule out of OpenSearchIntegTestCase ([#11877](https://github.com/opensearch-project/OpenSearch/pull/11877)), ([#12000](https://github.com/opensearch-project/OpenSearch/pull/12000))
 - Workaround for https://bugs.openjdk.org/browse/JDK-8323659 regression, introduced in JDK-21.0.2 ([#11968](https://github.com/opensearch-project/OpenSearch/pull/11968))
 
 ### Deprecated

test/framework/src/main/java/org/opensearch/test/OpenSearchTestClusterRule.java

@@ -56,6 +56,7 @@
  * on the way cluster settings are being managed.
  */
 class OpenSearchTestClusterRule implements MethodRule {
+    private final Map<TestCluster, OpenSearchIntegTestCase> suites = new IdentityHashMap<>();
     private final Map<Class<?>, TestCluster> clusters = new IdentityHashMap<>();
     private final Logger logger = LogManager.getLogger(getClass());
 
@@ -86,7 +87,13 @@ void afterClass() throws Exception {
                 printTestMessage("cleaning up after");
                 afterInternal(true, null);
                 OpenSearchTestCase.checkStaticState(true);
-                clusters.remove(getTestClass());
+                synchronized (clusters) {
+                    final TestCluster cluster = clusters.remove(getTestClass());
+                    IOUtils.closeWhileHandlingException(cluster);
+                    if (cluster != null) {
+                        suites.remove(cluster);
+                    }
+                }
             }
             StrictCheckSpanProcessor.validateTracingStateOnShutdown();
         } finally {
@@ -226,8 +233,8 @@ private static boolean isSuiteScopedTest(Class<?> clazz) {
         return clazz.getAnnotation(SuiteScopeTestCase.class) != null;
     }
 
-    private boolean hasParametersChanged(final ParameterizedOpenSearchIntegTestCase target) {
-        return !((ParameterizedOpenSearchIntegTestCase) suiteInstance).hasSameParametersAs(target);
+    private static boolean hasParametersChanged(final OpenSearchIntegTestCase instance, final ParameterizedOpenSearchIntegTestCase target) {
+        return !((ParameterizedOpenSearchIntegTestCase) instance).hasSameParametersAs(target);
     }
 
     private boolean runTestScopeLifecycle() {
@@ -242,8 +249,24 @@ private TestCluster buildAndPutCluster(Scope currentClusterScope, long seed, Ope
             clearClusters(); // all leftovers are gone by now... this is really just a double safety if we miss something somewhere
             switch (currentClusterScope) {
                 case SUITE:
+                    if (testCluster != null && target instanceof ParameterizedOpenSearchIntegTestCase) {
+                        final OpenSearchIntegTestCase instance = suites.get(testCluster);
+                        if (instance != null) {
+                            assert instance instanceof ParameterizedOpenSearchIntegTestCase;
+                            if (hasParametersChanged(
+                                (ParameterizedOpenSearchIntegTestCase) instance,
+                                (ParameterizedOpenSearchIntegTestCase) target
+                            )) {
+                                IOUtils.closeWhileHandlingException(testCluster);
+                                printTestMessage("new instance of parameterized test class, recreating test cluster for suite");
+                                testCluster = null;
+                            }
+                        }
+                    }
+
                     if (testCluster == null) { // only build if it's not there yet
                         testCluster = buildWithPrivateContext(currentClusterScope, seed, target);
+                        suites.put(testCluster, target);
                     }
                     break;
                 case TEST:
@@ -310,6 +333,7 @@ private void clearClusters() throws Exception {
         synchronized (clusters) {
             if (!clusters.isEmpty()) {
                 IOUtils.close(clusters.values());
+                suites.clear();
                 clusters.clear();
             }
         }
@@ -363,7 +387,7 @@ private void initializeSuiteScope(OpenSearchIntegTestCase target, FrameworkMetho
             // Catching the case when parameterized test cases are run: the test class stays the same but the test instances changes.
             if (target instanceof ParameterizedOpenSearchIntegTestCase) {
                 assert suiteInstance instanceof ParameterizedOpenSearchIntegTestCase;
-                if (hasParametersChanged((ParameterizedOpenSearchIntegTestCase) target)) {
+                if (hasParametersChanged(suiteInstance, (ParameterizedOpenSearchIntegTestCase) target)) {
                     printTestMessage("new instance of parameterized test class, recreating cluster scope", method);
                     afterClass();
                     beforeClass();