Skip to content

Commit 9503a71

Browse files
retashiv0408
reta
authored and
shiv0408
committed
Fix test cluster behavior @ClusterScope(scope = Scope.SUITE) and @OpenSearchIntegTestCase.SuiteScopeTestCase for parameterized test cases (opensearch-project#12000)
* Fix test cluster behavior @ClusterScope(scope = Scope.SUITE) and @OpenSearchIntegTestCase.SuiteScopeTestCase for parameterized test cases Signed-off-by: Andriy Redko <andriy.redko@aiven.io> * Address code review comments Signed-off-by: Andriy Redko <andriy.redko@aiven.io> * Address code review comments Signed-off-by: Andriy Redko <andriy.redko@aiven.io> * Added test cases for all supported test cluster scopes Signed-off-by: Andriy Redko <andriy.redko@aiven.io> --------- Signed-off-by: Andriy Redko <andriy.redko@aiven.io> Signed-off-by: Shivansh Arora <hishiv@amazon.com>
1 parent 691f8cf commit 9503a71

8 files changed

+344
-5
lines changed

‎CHANGELOG.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
209209
- 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))
210210
- Added Support for dynamically adding SearchRequestOperationsListeners with SearchRequestOperationsCompositeListenerFactory ([#11526](https://github.com/opensearch-project/OpenSearch/pull/11526))
211211
- Ensure Jackson default maximums introduced in 2.16.0 do not conflict with OpenSearch settings ([#11890](https://github.com/opensearch-project/OpenSearch/pull/11890))
212-
- Extract cluster management for integration tests into JUnit test rule out of OpenSearchIntegTestCase ([#11877](https://github.com/opensearch-project/OpenSearch/pull/11877))
212+
- 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))
213213
- 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))
214214

215215
### Deprecated

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

+35-4
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@
5656
* on the way cluster settings are being managed.
5757
*/
5858
class OpenSearchTestClusterRule implements MethodRule {
59+
// Maps each TestCluster instance to the exact test suite instance that triggered its creation
60+
private final Map<TestCluster, OpenSearchIntegTestCase> suites = new IdentityHashMap<>();
5961
private final Map<Class<?>, TestCluster> clusters = new IdentityHashMap<>();
6062
private final Logger logger = LogManager.getLogger(getClass());
6163

@@ -86,7 +88,13 @@ void afterClass() throws Exception {
8688
printTestMessage("cleaning up after");
8789
afterInternal(true, null);
8890
OpenSearchTestCase.checkStaticState(true);
89-
clusters.remove(getTestClass());
91+
synchronized (clusters) {
92+
final TestCluster cluster = clusters.remove(getTestClass());
93+
IOUtils.closeWhileHandlingException(cluster);
94+
if (cluster != null) {
95+
suites.remove(cluster);
96+
}
97+
}
9098
}
9199
StrictCheckSpanProcessor.validateTracingStateOnShutdown();
92100
} finally {
@@ -226,8 +234,11 @@ private static boolean isSuiteScopedTest(Class<?> clazz) {
226234
return clazz.getAnnotation(SuiteScopeTestCase.class) != null;
227235
}
228236

229-
private boolean hasParametersChanged(final ParameterizedOpenSearchIntegTestCase target) {
230-
return !((ParameterizedOpenSearchIntegTestCase) suiteInstance).hasSameParametersAs(target);
237+
private static boolean hasParametersChanged(
238+
final ParameterizedOpenSearchIntegTestCase instance,
239+
final ParameterizedOpenSearchIntegTestCase target
240+
) {
241+
return !instance.hasSameParametersAs(target);
231242
}
232243

233244
private boolean runTestScopeLifecycle() {
@@ -242,8 +253,24 @@ private TestCluster buildAndPutCluster(Scope currentClusterScope, long seed, Ope
242253
clearClusters(); // all leftovers are gone by now... this is really just a double safety if we miss something somewhere
243254
switch (currentClusterScope) {
244255
case SUITE:
256+
if (testCluster != null && target instanceof ParameterizedOpenSearchIntegTestCase) {
257+
final OpenSearchIntegTestCase instance = suites.get(testCluster);
258+
if (instance != null) {
259+
assert instance instanceof ParameterizedOpenSearchIntegTestCase;
260+
if (hasParametersChanged(
261+
(ParameterizedOpenSearchIntegTestCase) instance,
262+
(ParameterizedOpenSearchIntegTestCase) target
263+
)) {
264+
IOUtils.closeWhileHandlingException(testCluster);
265+
printTestMessage("new instance of parameterized test class, recreating test cluster for suite");
266+
testCluster = null;
267+
}
268+
}
269+
}
270+
245271
if (testCluster == null) { // only build if it's not there yet
246272
testCluster = buildWithPrivateContext(currentClusterScope, seed, target);
273+
suites.put(testCluster, target);
247274
}
248275
break;
249276
case TEST:
@@ -310,6 +337,7 @@ private void clearClusters() throws Exception {
310337
synchronized (clusters) {
311338
if (!clusters.isEmpty()) {
312339
IOUtils.close(clusters.values());
340+
suites.clear();
313341
clusters.clear();
314342
}
315343
}
@@ -363,7 +391,10 @@ private void initializeSuiteScope(OpenSearchIntegTestCase target, FrameworkMetho
363391
// Catching the case when parameterized test cases are run: the test class stays the same but the test instances changes.
364392
if (target instanceof ParameterizedOpenSearchIntegTestCase) {
365393
assert suiteInstance instanceof ParameterizedOpenSearchIntegTestCase;
366-
if (hasParametersChanged((ParameterizedOpenSearchIntegTestCase) target)) {
394+
if (hasParametersChanged(
395+
(ParameterizedOpenSearchIntegTestCase) suiteInstance,
396+
(ParameterizedOpenSearchIntegTestCase) target
397+
)) {
367398
printTestMessage("new instance of parameterized test class, recreating cluster scope", method);
368399
afterClass();
369400
beforeClass();

‎test/framework/src/test/java/org/opensearch/test/ParameterizedDynamicSettingsOpenSearchIntegTests.java

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.test;
10+
11+
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
12+
13+
import org.opensearch.action.admin.cluster.state.ClusterStateResponse;
14+
import org.opensearch.common.settings.Settings;
15+
import org.opensearch.common.util.FeatureFlags;
16+
17+
import java.io.IOException;
18+
import java.util.Arrays;
19+
import java.util.Collection;
20+
21+
import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING;
22+
import static org.hamcrest.CoreMatchers.equalTo;
23+
24+
public class ParameterizedDynamicSettingsOpenSearchIntegTests extends ParameterizedDynamicSettingsOpenSearchIntegTestCase {
25+
public ParameterizedDynamicSettingsOpenSearchIntegTests(Settings dynamicSettings) {
26+
super(dynamicSettings);
27+
}
28+
29+
@ParametersFactory
30+
public static Collection<Object[]> parameters() {
31+
return Arrays.asList(
32+
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), false).build() },
33+
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), true).build() }
34+
);
35+
}
36+
37+
@Override
38+
protected Settings featureFlagSettings() {
39+
return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.CONCURRENT_SEGMENT_SEARCH, "true").build();
40+
}
41+
42+
public void testSettings() throws IOException {
43+
final ClusterStateResponse cluster = client().admin().cluster().prepareState().all().get();
44+
assertThat(
45+
CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.get(cluster.getState().getMetadata().settings()),
46+
equalTo(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.get(settings))
47+
);
48+
}
49+
}

‎test/framework/src/test/java/org/opensearch/test/ParameterizedStaticSettingsOpenSearchIntegTests.java

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.test;
10+
11+
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
12+
13+
import org.opensearch.action.admin.cluster.node.info.NodeInfo;
14+
import org.opensearch.action.admin.cluster.node.info.NodesInfoResponse;
15+
import org.opensearch.common.settings.Settings;
16+
import org.opensearch.common.util.FeatureFlags;
17+
18+
import java.io.IOException;
19+
import java.util.Arrays;
20+
import java.util.Collection;
21+
22+
import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING;
23+
import static org.hamcrest.CoreMatchers.equalTo;
24+
25+
public class ParameterizedStaticSettingsOpenSearchIntegTests extends ParameterizedStaticSettingsOpenSearchIntegTestCase {
26+
27+
public ParameterizedStaticSettingsOpenSearchIntegTests(Settings staticSettings) {
28+
super(staticSettings);
29+
}
30+
31+
@ParametersFactory
32+
public static Collection<Object[]> parameters() {
33+
return Arrays.asList(
34+
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), false).build() },
35+
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), true).build() }
36+
);
37+
}
38+
39+
@Override
40+
protected Settings featureFlagSettings() {
41+
return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.CONCURRENT_SEGMENT_SEARCH, "true").build();
42+
}
43+
44+
public void testSettings() throws IOException {
45+
final NodesInfoResponse nodes = client().admin().cluster().prepareNodesInfo().get();
46+
for (final NodeInfo node : nodes.getNodes()) {
47+
assertThat(
48+
CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.get(node.getSettings()),
49+
equalTo(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.get(settings))
50+
);
51+
}
52+
}
53+
}

‎test/framework/src/test/java/org/opensearch/test/SuiteScopedParameterizedDynamicSettingsOpenSearchIntegTests.java

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.test;
10+
11+
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
12+
13+
import org.opensearch.action.admin.cluster.state.ClusterStateResponse;
14+
import org.opensearch.common.settings.Settings;
15+
import org.opensearch.common.util.FeatureFlags;
16+
17+
import java.io.IOException;
18+
import java.util.Arrays;
19+
import java.util.Collection;
20+
21+
import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING;
22+
import static org.hamcrest.CoreMatchers.equalTo;
23+
24+
@OpenSearchIntegTestCase.SuiteScopeTestCase
25+
public class SuiteScopedParameterizedDynamicSettingsOpenSearchIntegTests extends ParameterizedDynamicSettingsOpenSearchIntegTestCase {
26+
public SuiteScopedParameterizedDynamicSettingsOpenSearchIntegTests(Settings dynamicSettings) {
27+
super(dynamicSettings);
28+
}
29+
30+
@ParametersFactory
31+
public static Collection<Object[]> parameters() {
32+
return Arrays.asList(
33+
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), false).build() },
34+
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), true).build() }
35+
);
36+
}
37+
38+
@Override
39+
protected Settings featureFlagSettings() {
40+
return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.CONCURRENT_SEGMENT_SEARCH, "true").build();
41+
}
42+
43+
public void testSettings() throws IOException {
44+
final ClusterStateResponse cluster = client().admin().cluster().prepareState().all().get();
45+
assertThat(
46+
CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.get(cluster.getState().getMetadata().settings()),
47+
equalTo(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.get(settings))
48+
);
49+
}
50+
}

‎test/framework/src/test/java/org/opensearch/test/SuiteScopedParameterizedStaticSettingsOpenSearchIntegTests.java

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.test;
10+
11+
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
12+
13+
import org.opensearch.action.admin.cluster.node.info.NodeInfo;
14+
import org.opensearch.action.admin.cluster.node.info.NodesInfoResponse;
15+
import org.opensearch.common.settings.Settings;
16+
import org.opensearch.common.util.FeatureFlags;
17+
18+
import java.io.IOException;
19+
import java.util.Arrays;
20+
import java.util.Collection;
21+
22+
import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING;
23+
import static org.hamcrest.CoreMatchers.equalTo;
24+
25+
@OpenSearchIntegTestCase.SuiteScopeTestCase
26+
public class SuiteScopedParameterizedStaticSettingsOpenSearchIntegTests extends ParameterizedStaticSettingsOpenSearchIntegTestCase {
27+
public SuiteScopedParameterizedStaticSettingsOpenSearchIntegTests(Settings staticSettings) {
28+
super(staticSettings);
29+
}
30+
31+
@ParametersFactory
32+
public static Collection<Object[]> parameters() {
33+
return Arrays.asList(
34+
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), false).build() },
35+
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), true).build() }
36+
);
37+
}
38+
39+
@Override
40+
protected Settings featureFlagSettings() {
41+
return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.CONCURRENT_SEGMENT_SEARCH, "true").build();
42+
}
43+
44+
public void testSettings() throws IOException {
45+
final NodesInfoResponse nodes = client().admin().cluster().prepareNodesInfo().get();
46+
for (final NodeInfo node : nodes.getNodes()) {
47+
assertThat(
48+
CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.get(node.getSettings()),
49+
equalTo(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.get(settings))
50+
);
51+
}
52+
}
53+
}

‎test/framework/src/test/java/org/opensearch/test/TestScopedParameterizedDynamicSettingsOpenSearchIntegTests.java

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.test;
10+
11+
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
12+
13+
import org.opensearch.action.admin.cluster.state.ClusterStateResponse;
14+
import org.opensearch.common.settings.Settings;
15+
import org.opensearch.common.util.FeatureFlags;
16+
17+
import java.io.IOException;
18+
import java.util.Arrays;
19+
import java.util.Collection;
20+
21+
import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING;
22+
import static org.hamcrest.CoreMatchers.equalTo;
23+
24+
@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST)
25+
public class TestScopedParameterizedDynamicSettingsOpenSearchIntegTests extends ParameterizedDynamicSettingsOpenSearchIntegTestCase {
26+
public TestScopedParameterizedDynamicSettingsOpenSearchIntegTests(Settings dynamicSettings) {
27+
super(dynamicSettings);
28+
}
29+
30+
@ParametersFactory
31+
public static Collection<Object[]> parameters() {
32+
return Arrays.asList(
33+
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), false).build() },
34+
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), true).build() }
35+
);
36+
}
37+
38+
@Override
39+
protected Settings featureFlagSettings() {
40+
return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.CONCURRENT_SEGMENT_SEARCH, "true").build();
41+
}
42+
43+
public void testSettings() throws IOException {
44+
final ClusterStateResponse cluster = client().admin().cluster().prepareState().all().get();
45+
assertThat(
46+
CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.get(cluster.getState().getMetadata().settings()),
47+
equalTo(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.get(settings))
48+
);
49+
}
50+
}

0 commit comments

Comments
 (0)