From cea88cbeecb84d9459bed90bbaefd5ea4e4c1979 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenzo=20Dematt=C3=A9?= Date: Wed, 3 Jan 2024 09:29:40 +0100 Subject: [PATCH] Replace Version with String in YAML framework, pushing Version parsing where is actually needed (#103311) * Pushing down node versions as strings Deferring Version parsing to the actual places where a minimum node version/common cluster version is needed; eventually this will be completely lazy and/or replaced by other checks (e.g. features). Combine versions, oses and features in multi-cluster YAML test contexts. --- .../rest/yaml/CcsCommonYamlTestSuiteIT.java | 75 +++++++++++----- .../yaml/RcsCcsCommonYamlTestSuiteIT.java | 75 +++++++++++----- .../MultiClusterSearchYamlTestSuiteIT.java | 41 +++++---- .../test/rest/ESRestTestCase.java | 2 +- .../test/rest/ESRestTestFeatureService.java | 65 ++++++++++++++ .../test/rest/TestFeatureService.java | 56 +----------- .../yaml/ClientYamlTestExecutionContext.java | 89 ++++--------------- .../rest/yaml/ESClientYamlSuiteTestCase.java | 47 +++++----- .../test/rest/yaml/section/DoSection.java | 13 ++- .../ClientYamlTestExecutionContextTests.java | 27 +++--- .../rest/yaml/section/DoSectionTests.java | 4 +- 11 files changed, 263 insertions(+), 231 deletions(-) create mode 100644 test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestFeatureService.java diff --git a/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/CcsCommonYamlTestSuiteIT.java b/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/CcsCommonYamlTestSuiteIT.java index 7ac817d386da..88740edffc09 100644 --- a/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/CcsCommonYamlTestSuiteIT.java +++ b/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/CcsCommonYamlTestSuiteIT.java @@ -16,7 +16,6 @@ import org.apache.http.HttpHost; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.tests.util.TimeUnits; -import org.elasticsearch.Version; import org.elasticsearch.client.NodeSelector; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; @@ -30,6 +29,7 @@ import org.elasticsearch.test.cluster.FeatureFlag; import org.elasticsearch.test.cluster.local.LocalClusterConfigProvider; import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.test.rest.ObjectPath; +import org.elasticsearch.test.rest.TestFeatureService; import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi; import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestSpec; import org.elasticsearch.test.rest.yaml.section.ClientYamlTestSection; @@ -45,12 +45,15 @@ import org.junit.rules.RuleChain; import org.junit.rules.TestRule; import java.io.IOException; +import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.function.BiPredicate; -import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; import static java.util.Collections.unmodifiableList; @@ -286,29 +289,55 @@ public class CcsCommonYamlTestSuiteIT extends ESClientYamlSuiteTestCase { protected ClientYamlTestExecutionContext createRestTestExecutionContext( ClientYamlTestCandidate clientYamlTestCandidate, ClientYamlTestClient clientYamlTestClient, - final Version esVersion, - final Predicate clusterFeaturesPredicate, - final String os + final Set nodesVersions, + final TestFeatureService testFeatureService, + final Set osSet ) { - // depending on the API called, we either return the client running against the "write" or the "search" cluster here - - // TODO: reconcile and provide unified features, os, version(s), based on both clientYamlTestClient and searchYamlTestClient - return new ClientYamlTestExecutionContext( - clientYamlTestCandidate, - clientYamlTestClient, - randomizeContentType(), - esVersion, - ESRestTestCase::clusterHasFeature, - os - ) { - protected ClientYamlTestClient clientYamlTestClient(String apiName) { - if (CCS_APIS.contains(apiName)) { - return searchYamlTestClient; - } else { - return super.clientYamlTestClient(apiName); + try { + // Ensure the test specific initialization is run by calling it explicitly (@Before annotations on base-derived class may + // be called in a different order) + initSearchClient(); + // Reconcile and provide unified features, os, version(s), based on both clientYamlTestClient and searchYamlTestClient + var searchOs = readOsFromNodesInfo(adminSearchClient); + var searchNodeVersions = readVersionsFromNodesInfo(adminSearchClient); + var semanticNodeVersions = searchNodeVersions.stream() + .map(ESRestTestCase::parseLegacyVersion) + .flatMap(Optional::stream) + .collect(Collectors.toSet()); + final TestFeatureService searchTestFeatureService = createTestFeatureService( + getClusterStateFeatures(adminSearchClient), + semanticNodeVersions + ); + final TestFeatureService combinedTestFeatureService = new TestFeatureService() { + @Override + public boolean clusterHasFeature(String featureId) { + return testFeatureService.clusterHasFeature(featureId) && searchTestFeatureService.clusterHasFeature(featureId); } - } - }; + }; + final Set combinedOsSet = Stream.concat(osSet.stream(), Stream.of(searchOs)).collect(Collectors.toSet()); + final Set combinedNodeVersions = Stream.concat(nodesVersions.stream(), searchNodeVersions.stream()) + .collect(Collectors.toSet()); + + return new ClientYamlTestExecutionContext( + clientYamlTestCandidate, + clientYamlTestClient, + randomizeContentType(), + combinedNodeVersions, + combinedTestFeatureService, + combinedOsSet + ) { + // depending on the API called, we either return the client running against the "write" or the "search" cluster here + protected ClientYamlTestClient clientYamlTestClient(String apiName) { + if (CCS_APIS.contains(apiName)) { + return searchYamlTestClient; + } else { + return super.clientYamlTestClient(apiName); + } + } + }; + } catch (IOException e) { + throw new UncheckedIOException(e); + } } @AfterClass diff --git a/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/RcsCcsCommonYamlTestSuiteIT.java b/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/RcsCcsCommonYamlTestSuiteIT.java index 05dea0e95445..a331d6f54cb4 100644 --- a/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/RcsCcsCommonYamlTestSuiteIT.java +++ b/qa/ccs-common-rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/RcsCcsCommonYamlTestSuiteIT.java @@ -15,7 +15,6 @@ import org.apache.http.HttpHost; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.tests.util.TimeUnits; -import org.elasticsearch.Version; import org.elasticsearch.client.Request; import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.Response; @@ -32,6 +31,7 @@ import org.elasticsearch.test.cluster.local.LocalClusterConfigProvider; import org.elasticsearch.test.cluster.util.resource.Resource; import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.test.rest.ObjectPath; +import org.elasticsearch.test.rest.TestFeatureService; import org.elasticsearch.test.rest.yaml.CcsCommonYamlTestSuiteIT.TestCandidateAwareClient; import org.junit.AfterClass; import org.junit.Before; @@ -44,8 +44,11 @@ import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; import static java.util.Collections.unmodifiableList; import static org.elasticsearch.test.rest.yaml.CcsCommonYamlTestSuiteIT.CCS_APIS; @@ -271,29 +274,55 @@ public class RcsCcsCommonYamlTestSuiteIT extends ESClientYamlSuiteTestCase { protected ClientYamlTestExecutionContext createRestTestExecutionContext( ClientYamlTestCandidate clientYamlTestCandidate, ClientYamlTestClient clientYamlTestClient, - final Version esVersion, - final Predicate clusterFeaturesPredicate, - final String os + final Set nodesVersions, + final TestFeatureService testFeatureService, + final Set osSet ) { - // depending on the API called, we either return the client running against the "write" or the "search" cluster here - - // TODO: reconcile and provide unified features, os, version(s), based on both clientYamlTestClient and searchYamlTestClient - return new ClientYamlTestExecutionContext( - clientYamlTestCandidate, - clientYamlTestClient, - randomizeContentType(), - esVersion, - ESRestTestCase::clusterHasFeature, - os - ) { - protected ClientYamlTestClient clientYamlTestClient(String apiName) { - if (CCS_APIS.contains(apiName)) { - return searchYamlTestClient; - } else { - return super.clientYamlTestClient(apiName); + try { + // Ensure the test specific initialization is run by calling it explicitly (@Before annotations on base-derived class may + // be called in a different order) + initSearchClient(); + // Reconcile and provide unified features, os, version(s), based on both clientYamlTestClient and searchYamlTestClient + var searchOs = readOsFromNodesInfo(adminSearchClient); + var searchNodeVersions = readVersionsFromNodesInfo(adminSearchClient); + var semanticNodeVersions = searchNodeVersions.stream() + .map(ESRestTestCase::parseLegacyVersion) + .flatMap(Optional::stream) + .collect(Collectors.toSet()); + final TestFeatureService searchTestFeatureService = createTestFeatureService( + getClusterStateFeatures(adminSearchClient), + semanticNodeVersions + ); + final TestFeatureService combinedTestFeatureService = new TestFeatureService() { + @Override + public boolean clusterHasFeature(String featureId) { + return testFeatureService.clusterHasFeature(featureId) && searchTestFeatureService.clusterHasFeature(featureId); } - } - }; + }; + final Set combinedOsSet = Stream.concat(osSet.stream(), Stream.of(searchOs)).collect(Collectors.toSet()); + final Set combinedNodeVersions = Stream.concat(nodesVersions.stream(), searchNodeVersions.stream()) + .collect(Collectors.toSet()); + + return new ClientYamlTestExecutionContext( + clientYamlTestCandidate, + clientYamlTestClient, + randomizeContentType(), + combinedNodeVersions, + combinedTestFeatureService, + combinedOsSet + ) { + // depending on the API called, we either return the client running against the "write" or the "search" cluster here + protected ClientYamlTestClient clientYamlTestClient(String apiName) { + if (CCS_APIS.contains(apiName)) { + return searchYamlTestClient; + } else { + return super.clientYamlTestClient(apiName); + } + } + }; + } catch (IOException e) { + throw new UncheckedIOException(e); + } } @AfterClass diff --git a/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/MultiClusterSearchYamlTestSuiteIT.java b/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/MultiClusterSearchYamlTestSuiteIT.java index de1c75c4af83..0ebd36ec50f1 100644 --- a/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/MultiClusterSearchYamlTestSuiteIT.java +++ b/qa/multi-cluster-search/src/test/java/org/elasticsearch/search/MultiClusterSearchYamlTestSuiteIT.java @@ -13,26 +13,27 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import com.carrotsearch.randomizedtesting.annotations.TimeoutSuite; import org.apache.lucene.tests.util.TimeUnits; -import org.elasticsearch.Version; -import org.elasticsearch.test.rest.ESRestTestCase; +import org.elasticsearch.test.rest.TestFeatureService; import org.elasticsearch.test.rest.yaml.ClientYamlTestCandidate; import org.elasticsearch.test.rest.yaml.ClientYamlTestClient; import org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext; import org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase; import org.junit.BeforeClass; -import java.util.function.Predicate; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; @TimeoutSuite(millis = 5 * TimeUnits.MINUTE) // to account for slow as hell VMs public class MultiClusterSearchYamlTestSuiteIT extends ESClientYamlSuiteTestCase { - private static Version remoteEsVersion = null; + private static String remoteEsVersion = null; @BeforeClass - public static void determineRemoteClusterMinimumVersion() { + public static void readRemoteClusterVersion() { String remoteClusterVersion = System.getProperty("tests.rest.remote_cluster_version"); if (remoteClusterVersion != null) { - remoteEsVersion = Version.fromString(remoteClusterVersion); + remoteEsVersion = remoteClusterVersion; } } @@ -40,26 +41,32 @@ public class MultiClusterSearchYamlTestSuiteIT extends ESClientYamlSuiteTestCase protected ClientYamlTestExecutionContext createRestTestExecutionContext( ClientYamlTestCandidate clientYamlTestCandidate, ClientYamlTestClient clientYamlTestClient, - final Version esVersion, - final Predicate clusterFeaturesPredicate, - final String os + final Set nodesVersions, + final TestFeatureService testFeatureService, + final Set osSet ) { /* * Since the esVersion is used to skip tests in ESClientYamlSuiteTestCase, we also take into account the - * remote cluster version here and return it if it is lower than the local client version. This is used to - * skip tests if some feature isn't available on the remote cluster yet. + * remote cluster version here. This is used to skip tests if some feature isn't available on the remote cluster yet. */ - final Version commonEsVersion = remoteEsVersion != null && remoteEsVersion.before(esVersion) ? remoteEsVersion : esVersion; - - // TODO: same for os and features + final Set commonVersions; + if (remoteEsVersion == null || nodesVersions.contains(remoteEsVersion)) { + commonVersions = nodesVersions; + } else { + var versionsCopy = new HashSet<>(nodesVersions); + versionsCopy.add(remoteEsVersion); + commonVersions = Collections.unmodifiableSet(versionsCopy); + } + // TODO: same for os and features. Better to do that once this test(s) have been migrated to the new ElasticsearchCluster-based + // framework. See CcsCommonYamlTestSuiteIT for example. return new ClientYamlTestExecutionContext( clientYamlTestCandidate, clientYamlTestClient, randomizeContentType(), - commonEsVersion, - ESRestTestCase::clusterHasFeature, - os + commonVersions, + testFeatureService, + osSet ); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java index 83c2775347f9..a21a4bd3f672 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java @@ -337,7 +337,7 @@ public abstract class ESRestTestCase extends ESTestCase { ? List.of(new RestTestLegacyFeatures(), new ESRestTestCaseHistoricalFeatures()) : List.of(new RestTestLegacyFeatures()); - return new TestFeatureService( + return new ESRestTestFeatureService( hasHistoricalFeaturesInformation, providers, semanticNodeVersions, diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestFeatureService.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestFeatureService.java new file mode 100644 index 000000000000..25a5da93804c --- /dev/null +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestFeatureService.java @@ -0,0 +1,65 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.test.rest; + +import org.elasticsearch.Version; +import org.elasticsearch.core.Strings; +import org.elasticsearch.features.FeatureData; +import org.elasticsearch.features.FeatureSpecification; + +import java.util.Collection; +import java.util.List; +import java.util.NavigableMap; +import java.util.Set; +import java.util.function.Predicate; + +class ESRestTestFeatureService implements TestFeatureService { + private final Predicate historicalFeaturesPredicate; + private final Set clusterStateFeatures; + + ESRestTestFeatureService( + boolean hasHistoricalFeaturesInformation, + List specs, + Collection nodeVersions, + Set clusterStateFeatures + ) { + var minNodeVersion = nodeVersions.stream().min(Version::compareTo); + var featureData = FeatureData.createFromSpecifications(specs); + var historicalFeatures = featureData.getHistoricalFeatures(); + var allHistoricalFeatures = historicalFeatures.lastEntry() == null ? Set.of() : historicalFeatures.lastEntry().getValue(); + + var errorMessage = hasHistoricalFeaturesInformation + ? "Check the feature has been added to the correct FeatureSpecification in the relevant module or, if this is a " + + "legacy feature used only in tests, to a test-only FeatureSpecification" + : "This test is running on the legacy test framework; historical features from production code will not be available." + + " You need to port the test to the new test plugins in order to use historical features from production code." + + " If this is a legacy feature used only in tests, you can add it to a test-only FeatureSpecification"; + this.historicalFeaturesPredicate = minNodeVersion.>map(v -> featureId -> { + assert allHistoricalFeatures.contains(featureId) : Strings.format("Unknown historical feature %s: %s", featureId, errorMessage); + return hasHistoricalFeature(historicalFeatures, v, featureId); + }).orElse(featureId -> { + // We can safely assume that new non-semantic versions (serverless) support all historical features + assert allHistoricalFeatures.contains(featureId) : Strings.format("Unknown historical feature %s: %s", featureId, errorMessage); + return true; + }); + this.clusterStateFeatures = clusterStateFeatures; + } + + private static boolean hasHistoricalFeature(NavigableMap> historicalFeatures, Version version, String featureId) { + var features = historicalFeatures.floorEntry(version); + return features != null && features.getValue().contains(featureId); + } + + public boolean clusterHasFeature(String featureId) { + if (clusterStateFeatures.contains(featureId)) { + return true; + } + return historicalFeaturesPredicate.test(featureId); + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/TestFeatureService.java b/test/framework/src/main/java/org/elasticsearch/test/rest/TestFeatureService.java index 74c31f5b9478..ab6c97843355 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/TestFeatureService.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/TestFeatureService.java @@ -8,58 +8,6 @@ package org.elasticsearch.test.rest; -import org.elasticsearch.Version; -import org.elasticsearch.core.Strings; -import org.elasticsearch.features.FeatureData; -import org.elasticsearch.features.FeatureSpecification; - -import java.util.Collection; -import java.util.List; -import java.util.NavigableMap; -import java.util.Set; -import java.util.function.Predicate; - -public class TestFeatureService { - private final Predicate historicalFeaturesPredicate; - private final Set clusterStateFeatures; - - TestFeatureService( - boolean hasHistoricalFeaturesInformation, - List specs, - Collection nodeVersions, - Set clusterStateFeatures - ) { - var minNodeVersion = nodeVersions.stream().min(Version::compareTo); - var featureData = FeatureData.createFromSpecifications(specs); - var historicalFeatures = featureData.getHistoricalFeatures(); - var allHistoricalFeatures = historicalFeatures.lastEntry() == null ? Set.of() : historicalFeatures.lastEntry().getValue(); - - var errorMessage = hasHistoricalFeaturesInformation - ? "Check the feature has been added to the correct FeatureSpecification in the relevant module or, if this is a " - + "legacy feature used only in tests, to a test-only FeatureSpecification" - : "This test is running on the legacy test framework; historical features from production code will not be available." - + " You need to port the test to the new test plugins in order to use historical features from production code." - + " If this is a legacy feature used only in tests, you can add it to a test-only FeatureSpecification"; - this.historicalFeaturesPredicate = minNodeVersion.>map(v -> featureId -> { - assert allHistoricalFeatures.contains(featureId) : Strings.format("Unknown historical feature %s: %s", featureId, errorMessage); - return hasHistoricalFeature(historicalFeatures, v, featureId); - }).orElse(featureId -> { - // We can safely assume that new non-semantic versions (serverless) support all historical features - assert allHistoricalFeatures.contains(featureId) : Strings.format("Unknown historical feature %s: %s", featureId, errorMessage); - return true; - }); - this.clusterStateFeatures = clusterStateFeatures; - } - - private static boolean hasHistoricalFeature(NavigableMap> historicalFeatures, Version version, String featureId) { - var features = historicalFeatures.floorEntry(version); - return features != null && features.getValue().contains(featureId); - } - - public boolean clusterHasFeature(String featureId) { - if (clusterStateFeatures.contains(featureId)) { - return true; - } - return historicalFeaturesPredicate.test(featureId); - } +public interface TestFeatureService { + boolean clusterHasFeature(String featureId); } diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java index 3c523c6607db..10bf2fb4b0a9 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java @@ -15,11 +15,8 @@ import org.apache.http.entity.ContentType; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.util.BytesRef; -import org.elasticsearch.Version; import org.elasticsearch.client.NodeSelector; -import org.elasticsearch.common.VersionId; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.test.rest.Stash; import org.elasticsearch.test.rest.TestFeatureService; import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi; @@ -33,10 +30,8 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.function.BiPredicate; -import java.util.function.Predicate; /** * Execution context passed across the REST tests. @@ -56,43 +51,21 @@ public class ClientYamlTestExecutionContext { private ClientYamlTestResponse response; - private final Version esVersion; + private final Set nodesVersions; - private final String os; - private final Predicate clusterFeaturesPredicate; + private final Set osSet; + private final TestFeatureService testFeatureService; private final boolean randomizeContentType; private final BiPredicate pathPredicate; - // TODO: this will become the ctor once work is done on serverless side and https://github.com/elastic/elasticsearch/pull/103311/ - // has been merged public ClientYamlTestExecutionContext( ClientYamlTestCandidate clientYamlTestCandidate, ClientYamlTestClient clientYamlTestClient, boolean randomizeContentType, final Set nodesVersions, final TestFeatureService testFeatureService, - final Set osList, - BiPredicate pathPredicate - ) { - this( - clientYamlTestCandidate, - clientYamlTestClient, - randomizeContentType, - getEsVersion(nodesVersions), - testFeatureService::clusterHasFeature, - osList.iterator().next(), - pathPredicate - ); - } - - public ClientYamlTestExecutionContext( - ClientYamlTestCandidate clientYamlTestCandidate, - ClientYamlTestClient clientYamlTestClient, - boolean randomizeContentType, - final Set nodesVersions, - final TestFeatureService testFeatureService, - final Set osList + final Set osSet ) { this( clientYamlTestCandidate, @@ -100,56 +73,26 @@ public class ClientYamlTestExecutionContext { randomizeContentType, nodesVersions, testFeatureService, - osList, + osSet, (ignoreApi, ignorePath) -> true ); } - @Deprecated - static Version getEsVersion(Set nodesVersions) { - return nodesVersions.stream() - .map(ESRestTestCase::parseLegacyVersion) - .flatMap(Optional::stream) - .min(VersionId::compareTo) - .orElse(Version.CURRENT); - } - - @Deprecated public ClientYamlTestExecutionContext( ClientYamlTestCandidate clientYamlTestCandidate, ClientYamlTestClient clientYamlTestClient, boolean randomizeContentType, - final Version esVersion, - final Predicate clusterFeaturesPredicate, - final String os - ) { - this( - clientYamlTestCandidate, - clientYamlTestClient, - randomizeContentType, - esVersion, - clusterFeaturesPredicate, - os, - (ignoreApi, ignorePath) -> true - ); - } - - @Deprecated - public ClientYamlTestExecutionContext( - ClientYamlTestCandidate clientYamlTestCandidate, - ClientYamlTestClient clientYamlTestClient, - boolean randomizeContentType, - final Version esVersion, - final Predicate clusterFeaturesPredicate, - final String os, + final Set nodesVersions, + final TestFeatureService testFeatureService, + final Set osSet, BiPredicate pathPredicate ) { this.clientYamlTestClient = clientYamlTestClient; this.clientYamlTestCandidate = clientYamlTestCandidate; this.randomizeContentType = randomizeContentType; - this.esVersion = esVersion; - this.clusterFeaturesPredicate = clusterFeaturesPredicate; - this.os = os; + this.nodesVersions = nodesVersions; + this.testFeatureService = testFeatureService; + this.osSet = osSet; this.pathPredicate = pathPredicate; } @@ -304,14 +247,14 @@ public class ClientYamlTestExecutionContext { } /** - * @return the version of the oldest node in the cluster + * @return the distinct node versions running in the cluster */ - public Version esVersion() { - return esVersion; + public Set nodesVersions() { + return nodesVersions; } public String os() { - return os; + return osSet.iterator().next(); } public ClientYamlTestCandidate getClientYamlTestCandidate() { @@ -319,6 +262,6 @@ public class ClientYamlTestExecutionContext { } public boolean clusterHasFeature(String featureId) { - return clusterFeaturesPredicate.test(featureId); + return testFeatureService.clusterHasFeature(featureId); } } diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java index 34555c44ce2d..ad05a81cc257 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java @@ -26,6 +26,7 @@ import org.elasticsearch.client.WarningsHandler; import org.elasticsearch.client.sniff.ElasticsearchNodesSniffer; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.VersionId; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.core.IOUtils; @@ -61,10 +62,8 @@ import java.util.Optional; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; -import java.util.function.Predicate; import java.util.stream.Collectors; -import static org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext.getEsVersion; import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; /** @@ -144,6 +143,8 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase { final Set nodesVersions = getCachedNodesVersions(); final String os = readOsFromNodesInfo(adminClient()); + logger.info("initializing client, node versions [{}], hosts {}, os [{}]", nodesVersions, hosts, os); + var semanticNodeVersions = nodesVersions.stream() .map(ESRestTestCase::parseLegacyVersion) .flatMap(Optional::stream) @@ -199,32 +200,15 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase { ClientYamlTestClient clientYamlTestClient, final Set nodesVersions, final TestFeatureService testFeatureService, - final Set osList - ) { - return createRestTestExecutionContext( - clientYamlTestCandidate, - clientYamlTestClient, - getEsVersion(nodesVersions), - testFeatureService::clusterHasFeature, - osList.iterator().next() - ); - } - - @Deprecated - protected ClientYamlTestExecutionContext createRestTestExecutionContext( - ClientYamlTestCandidate clientYamlTestCandidate, - ClientYamlTestClient clientYamlTestClient, - final Version esVersion, - final Predicate clusterFeaturesPredicate, - final String os + final Set osSet ) { return new ClientYamlTestExecutionContext( clientYamlTestCandidate, clientYamlTestClient, randomizeContentType(), - esVersion, - clusterFeaturesPredicate, - os + nodesVersions, + testFeatureService, + osSet ); } @@ -463,20 +447,31 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase { ); } + // Try to extract the minimum node version. Assume CURRENT if nodes have non-semantic versions + // TODO: after https://github.com/elastic/elasticsearch/pull/103404 is merged, we can push this logic into SkipVersionContext. + // This way will have version parsing only when we actually have to skip on a version, we can remove the default and throw an + // IllegalArgumentException instead (attempting to skip on version where version is not semantic) + var oldestNodeVersion = restTestExecutionContext.nodesVersions() + .stream() + .map(ESRestTestCase::parseLegacyVersion) + .flatMap(Optional::stream) + .min(VersionId::compareTo) + .orElse(Version.CURRENT); + // skip test if the whole suite (yaml file) is disabled assumeFalse( testCandidate.getSetupSection().getSkipSection().getSkipMessage(testCandidate.getSuitePath()), - testCandidate.getSetupSection().getSkipSection().skip(restTestExecutionContext.esVersion()) + testCandidate.getSetupSection().getSkipSection().skip(oldestNodeVersion) ); // skip test if the whole suite (yaml file) is disabled assumeFalse( testCandidate.getTeardownSection().getSkipSection().getSkipMessage(testCandidate.getSuitePath()), - testCandidate.getTeardownSection().getSkipSection().skip(restTestExecutionContext.esVersion()) + testCandidate.getTeardownSection().getSkipSection().skip(oldestNodeVersion) ); // skip test if test section is disabled assumeFalse( testCandidate.getTestSection().getSkipSection().getSkipMessage(testCandidate.getTestPath()), - testCandidate.getTestSection().getSkipSection().skip(restTestExecutionContext.esVersion()) + testCandidate.getTestSection().getSkipSection().skip(oldestNodeVersion) ); // skip test if os is excluded assumeFalse( diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java index 08631c148a7e..f57d90e911ea 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java @@ -17,10 +17,12 @@ import org.elasticsearch.client.Node; import org.elasticsearch.client.NodeSelector; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.VersionId; import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.core.Tuple; import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.rest.action.admin.indices.RestPutIndexTemplateAction; +import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.test.rest.RestTestLegacyFeatures; import org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext; import org.elasticsearch.test.rest.yaml.ClientYamlTestResponse; @@ -39,6 +41,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.function.Predicate; @@ -376,8 +379,14 @@ public class DoSection implements ExecutableSection { // This is really difficult to express just with features, so I will break it down into 2 parts: version check for v7, // and feature check for v8. This way the version check can be removed once we move to v9 @UpdateForV9 - var fixedInV7 = executionContext.esVersion().major == Version.V_7_17_0.major - && executionContext.esVersion().onOrAfter(Version.V_7_17_2); + var fixedInV7 = executionContext.nodesVersions() + .stream() + .map(ESRestTestCase::parseLegacyVersion) + .flatMap(Optional::stream) + .min(VersionId::compareTo) + .map(v -> v.major == Version.V_7_17_0.major && v.onOrAfter(Version.V_7_17_2)) + .orElse(false); + var fixedProductionHeader = fixedInV7 || executionContext.clusterHasFeature(RestTestLegacyFeatures.REST_ELASTIC_PRODUCT_HEADER_PRESENT.id()); if (fixedProductionHeader) { diff --git a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContextTests.java b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContextTests.java index 1c3515cf02f9..6e8397c816b3 100644 --- a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContextTests.java +++ b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContextTests.java @@ -9,31 +9,38 @@ package org.elasticsearch.test.rest.yaml; import org.apache.http.HttpEntity; -import org.elasticsearch.Version; import org.elasticsearch.client.NodeSelector; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.VersionUtils; +import org.elasticsearch.test.rest.TestFeatureService; import java.io.IOException; import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import static org.hamcrest.Matchers.is; public class ClientYamlTestExecutionContextTests extends ESTestCase { + private static class MockTestFeatureService implements TestFeatureService { + @Override + public boolean clusterHasFeature(String featureId) { + return true; + } + } + public void testHeadersSupportStashedValueReplacement() throws IOException { final AtomicReference> headersRef = new AtomicReference<>(); - final Version version = VersionUtils.randomVersion(random()); + final String version = randomAlphaOfLength(10); final ClientYamlTestExecutionContext context = new ClientYamlTestExecutionContext( null, null, randomBoolean(), - version, - feature -> true, - "os" + Set.of(version), + new MockTestFeatureService(), + Set.of("os") ) { @Override ClientYamlTestResponse callApiInternal( @@ -64,14 +71,14 @@ public class ClientYamlTestExecutionContextTests extends ESTestCase { } public void testStashHeadersOnException() throws IOException { - final Version version = VersionUtils.randomVersion(random()); + final String version = randomAlphaOfLength(10); final ClientYamlTestExecutionContext context = new ClientYamlTestExecutionContext( null, null, randomBoolean(), - version, - feature -> true, - "os" + Set.of(version), + new MockTestFeatureService(), + Set.of("os") ) { @Override ClientYamlTestResponse callApiInternal( diff --git a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java index 501f83bb02e1..0cb9a3e29e63 100644 --- a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java +++ b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java @@ -16,7 +16,6 @@ import org.elasticsearch.client.NodeSelector; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.logging.HeaderWarning; import org.elasticsearch.core.Strings; -import org.elasticsearch.test.VersionUtils; import org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext; import org.elasticsearch.test.rest.yaml.ClientYamlTestResponse; import org.elasticsearch.xcontent.XContentLocation; @@ -31,6 +30,7 @@ import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.regex.Pattern; import static java.util.Collections.emptyList; @@ -610,7 +610,7 @@ public class DoSectionTests extends AbstractClientYamlTestFragmentParserTestCase doSection.getApiCallSection().getNodeSelector() ) ).thenReturn(mockResponse); - when(context.esVersion()).thenReturn(VersionUtils.randomVersion(random())); + when(context.nodesVersions()).thenReturn(Set.of(randomAlphaOfLength(10))); when(mockResponse.getHeaders("X-elastic-product")).thenReturn(List.of("Elasticsearch")); doSection.execute(context); verify(context).callApi(