From b2b7f3c7ed7e45384acaa3338624127646424925 Mon Sep 17 00:00:00 2001 From: Neetika Singhal Date: Tue, 15 Aug 2023 11:02:37 -0700 Subject: [PATCH] Add base class for parameterizing the search based tests (#9083) Signed-off-by: Neetika Singhal Signed-off-by: Shivansh Arora --- CHANGELOG.md | 1 + ...ConcurrentSegmentSearchCancellationIT.java | 26 ---------- .../ConcurrentSegmentSearchTimeoutIT.java | 27 ----------- .../search/SearchCancellationIT.java | 24 +++++++++- .../opensearch/search/SearchTimeoutIT.java | 23 ++++++++- .../ParameterizedOpenSearchIntegTestCase.java | 47 +++++++++++++++++++ 6 files changed, 93 insertions(+), 55 deletions(-) delete mode 100644 server/src/internalClusterTest/java/org/opensearch/search/ConcurrentSegmentSearchCancellationIT.java delete mode 100644 server/src/internalClusterTest/java/org/opensearch/search/ConcurrentSegmentSearchTimeoutIT.java create mode 100644 test/framework/src/main/java/org/opensearch/test/ParameterizedOpenSearchIntegTestCase.java diff --git a/CHANGELOG.md b/CHANGELOG.md index fed7b16f08785..f231d1db8036b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,6 +135,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Make MultiBucketConsumerService thread safe to use across slices during search ([#9047](https://github.com/opensearch-project/OpenSearch/pull/9047)) - Change shard_size and shard_min_doc_count evaluation to happen in shard level reduce phase ([#9085](https://github.com/opensearch-project/OpenSearch/pull/9085)) - Add attributes to startSpan methods ([#9199](https://github.com/opensearch-project/OpenSearch/pull/9199)) +- Add base class for parameterizing the search based tests #9083 ([#9083](https://github.com/opensearch-project/OpenSearch/pull/9083)) ### Deprecated diff --git a/server/src/internalClusterTest/java/org/opensearch/search/ConcurrentSegmentSearchCancellationIT.java b/server/src/internalClusterTest/java/org/opensearch/search/ConcurrentSegmentSearchCancellationIT.java deleted file mode 100644 index 3c50627e342dd..0000000000000 --- a/server/src/internalClusterTest/java/org/opensearch/search/ConcurrentSegmentSearchCancellationIT.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.search; - -import org.opensearch.common.settings.FeatureFlagSettings; -import org.opensearch.common.settings.Setting; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; - -public class ConcurrentSegmentSearchCancellationIT extends SearchCancellationIT { - @Override - protected Settings featureFlagSettings() { - Settings.Builder featureSettings = Settings.builder(); - for (Setting builtInFlag : FeatureFlagSettings.BUILT_IN_FEATURE_FLAGS) { - featureSettings.put(builtInFlag.getKey(), builtInFlag.getDefaultRaw(Settings.EMPTY)); - } - featureSettings.put(FeatureFlags.CONCURRENT_SEGMENT_SEARCH, "true"); - return featureSettings.build(); - } -} diff --git a/server/src/internalClusterTest/java/org/opensearch/search/ConcurrentSegmentSearchTimeoutIT.java b/server/src/internalClusterTest/java/org/opensearch/search/ConcurrentSegmentSearchTimeoutIT.java deleted file mode 100644 index c19f762679fb0..0000000000000 --- a/server/src/internalClusterTest/java/org/opensearch/search/ConcurrentSegmentSearchTimeoutIT.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.search; - -import org.opensearch.common.settings.FeatureFlagSettings; -import org.opensearch.common.settings.Setting; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; - -public class ConcurrentSegmentSearchTimeoutIT extends SearchTimeoutIT { - - @Override - protected Settings featureFlagSettings() { - Settings.Builder featureSettings = Settings.builder(); - for (Setting builtInFlag : FeatureFlagSettings.BUILT_IN_FEATURE_FLAGS) { - featureSettings.put(builtInFlag.getKey(), builtInFlag.getDefaultRaw(Settings.EMPTY)); - } - featureSettings.put(FeatureFlags.CONCURRENT_SEGMENT_SEARCH, "true"); - return featureSettings.build(); - } -} diff --git a/server/src/internalClusterTest/java/org/opensearch/search/SearchCancellationIT.java b/server/src/internalClusterTest/java/org/opensearch/search/SearchCancellationIT.java index b186da7f92a26..cd6671a5041e3 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/SearchCancellationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/SearchCancellationIT.java @@ -32,6 +32,7 @@ package org.opensearch.search; +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import org.apache.logging.log4j.LogManager; import org.junit.After; @@ -52,6 +53,7 @@ import org.opensearch.common.unit.TimeValue; import org.opensearch.core.common.Strings; import org.opensearch.core.xcontent.MediaTypeRegistry; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.plugins.Plugin; import org.opensearch.plugins.PluginsService; import org.opensearch.script.MockScriptPlugin; @@ -61,9 +63,11 @@ import org.opensearch.core.tasks.TaskCancelledException; import org.opensearch.tasks.TaskInfo; import org.opensearch.test.OpenSearchIntegTestCase; +import org.opensearch.test.ParameterizedOpenSearchIntegTestCase; import org.opensearch.transport.TransportException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -77,6 +81,7 @@ import static org.opensearch.action.search.TransportSearchAction.SEARCH_CANCEL_AFTER_TIME_INTERVAL_SETTING_KEY; import static org.opensearch.index.query.QueryBuilders.scriptQuery; import static org.opensearch.search.SearchCancellationIT.ScriptedBlockPlugin.SCRIPT_NAME; +import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING; import static org.opensearch.search.SearchService.NO_TIMEOUT; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertFailures; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertNoFailures; @@ -86,12 +91,29 @@ import static org.hamcrest.Matchers.notNullValue; @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.SUITE) -public class SearchCancellationIT extends OpenSearchIntegTestCase { +public class SearchCancellationIT extends ParameterizedOpenSearchIntegTestCase { private TimeValue requestCancellationTimeout = TimeValue.timeValueSeconds(1); private TimeValue clusterCancellationTimeout = TimeValue.timeValueMillis(1500); private TimeValue keepAlive = TimeValue.timeValueSeconds(5); + public SearchCancellationIT(Settings settings) { + super(settings); + } + + @ParametersFactory + public static Collection parameters() { + return Arrays.asList( + new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), false).build() }, + new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), true).build() } + ); + } + + @Override + protected Settings featureFlagSettings() { + return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.CONCURRENT_SEGMENT_SEARCH, "true").build(); + } + @Override protected Collection> nodePlugins() { return Collections.singleton(ScriptedBlockPlugin.class); diff --git a/server/src/internalClusterTest/java/org/opensearch/search/SearchTimeoutIT.java b/server/src/internalClusterTest/java/org/opensearch/search/SearchTimeoutIT.java index aa8ef3f29c989..88d35e5ba785a 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/SearchTimeoutIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/SearchTimeoutIT.java @@ -32,16 +32,20 @@ package org.opensearch.search; +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import org.opensearch.OpenSearchException; import org.opensearch.action.search.SearchResponse; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.plugins.Plugin; import org.opensearch.script.MockScriptPlugin; import org.opensearch.script.Script; import org.opensearch.script.ScriptType; import org.opensearch.test.OpenSearchIntegTestCase; +import org.opensearch.test.ParameterizedOpenSearchIntegTestCase; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Map; @@ -50,10 +54,27 @@ import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; import static org.opensearch.index.query.QueryBuilders.scriptQuery; +import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING; import static org.opensearch.search.SearchTimeoutIT.ScriptedTimeoutPlugin.SCRIPT_NAME; @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.SUITE) -public class SearchTimeoutIT extends OpenSearchIntegTestCase { +public class SearchTimeoutIT extends ParameterizedOpenSearchIntegTestCase { + public SearchTimeoutIT(Settings settings) { + super(settings); + } + + @ParametersFactory + public static Collection parameters() { + return Arrays.asList( + new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), false).build() }, + new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), true).build() } + ); + } + + @Override + protected Settings featureFlagSettings() { + return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.CONCURRENT_SEGMENT_SEARCH, "true").build(); + } @Override protected Collection> nodePlugins() { diff --git a/test/framework/src/main/java/org/opensearch/test/ParameterizedOpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/ParameterizedOpenSearchIntegTestCase.java new file mode 100644 index 0000000000000..a41a17d35e163 --- /dev/null +++ b/test/framework/src/main/java/org/opensearch/test/ParameterizedOpenSearchIntegTestCase.java @@ -0,0 +1,47 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.test; + +import org.junit.After; +import org.junit.Before; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsModule; + +/** + * Base class for running the tests with parameterization of the dynamic settings + * For any class that wants to use parameterization, use @ParametersFactory to generate + * different params only for dynamic settings. Refer SearchCancellationIT for an example. + * Note: this doesn't work for the parameterization of feature flag/static settings. + */ +public abstract class ParameterizedOpenSearchIntegTestCase extends OpenSearchIntegTestCase { + + private final Settings dynamicSettings; + + public ParameterizedOpenSearchIntegTestCase(Settings dynamicSettings) { + this.dynamicSettings = dynamicSettings; + } + + @Before + public void beforeTests() { + SettingsModule settingsModule = new SettingsModule(dynamicSettings); + for (String key : dynamicSettings.keySet()) { + assertTrue( + settingsModule.getClusterSettings().isDynamicSetting(key) || settingsModule.getIndexScopedSettings().isDynamicSetting(key) + ); + } + client().admin().cluster().prepareUpdateSettings().setPersistentSettings(dynamicSettings).get(); + } + + @After + public void afterTests() { + final Settings.Builder settingsToUnset = Settings.builder(); + dynamicSettings.keySet().forEach(settingsToUnset::putNull); + client().admin().cluster().prepareUpdateSettings().setPersistentSettings(settingsToUnset).get(); + } +}