Skip to content

Commit

Permalink
Refactor: move version, os, features from ClientYamlTestClient to Cli…
Browse files Browse the repository at this point in the history 
…entYamlTestExecutionContext (elastic#103560)

Just moving stuff around, no change in behaviour. Moving these fields showed how we are not treating correctly in derived classes where multiple clusters are tested (ex: CCR), but this is for another time.

Co-authored-by: Moritz Mack <[email protected]>
  • Loading branch information
@ldematte @mosche
ldematte and mosche authored Dec 20, 2023
1 parent 62ddafb commit 1900a99
Show file tree
Hide file tree
Showing 9 changed files with 133 additions and 135 deletions.
17 changes: 2 additions & 15 deletions docs/src/yamlRestTest/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import org.apache.http.util.EntityUtils;
import org.apache.lucene.tests.util.TimeUnits;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.Version;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.RestClient;
import org.elasticsearch.common.settings.SecureString;
Expand Down Expand Up @@ -48,7 +47,6 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.function.Predicate;

import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
Expand Down Expand Up @@ -98,20 +96,9 @@ protected boolean randomizeContentType() {
protected ClientYamlTestClient initClientYamlTestClient(
final ClientYamlSuiteRestSpec restSpec,
final RestClient restClient,
final List<HttpHost> hosts,
final Version esVersion,
final Predicate<String> clusterFeaturesPredicate,
final String os
final List<HttpHost> hosts
) {
return new ClientYamlDocsTestClient(
restSpec,
restClient,
hosts,
esVersion,
clusterFeaturesPredicate,
os,
this::getClientBuilderWithSniffedHosts
);
return new ClientYamlDocsTestClient(restSpec, restClient, hosts, this::getClientBuilderWithSniffedHosts);
}

@Before
Expand Down
36 changes: 16 additions & 20 deletions ...rest/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/CcsCommonYamlTestSuiteIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.elasticsearch.common.CheckedSupplier;
import org.elasticsearch.common.Strings;
import org.elasticsearch.core.IOUtils;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.test.cluster.ElasticsearchCluster;
import org.elasticsearch.test.cluster.FeatureFlag;
import org.elasticsearch.test.cluster.local.LocalClusterConfigProvider;
Expand Down Expand Up @@ -159,19 +158,7 @@ public void initSearchClient() throws IOException {
searchClient = buildClient(restClientSettings(), clusterHosts.toArray(new HttpHost[clusterHosts.size()]));
adminSearchClient = buildClient(restAdminSettings(), clusterHosts.toArray(new HttpHost[clusterHosts.size()]));

Tuple<Version, Version> versionVersionTuple = readVersionsFromCatNodes(adminSearchClient);
final Version esVersion = versionVersionTuple.v1();
final String os = readOsFromNodesInfo(adminSearchClient);

searchYamlTestClient = new TestCandidateAwareClient(
getRestSpec(),
searchClient,
hosts,
esVersion,
ESRestTestCase::clusterHasFeature,
os,
this::getClientBuilderWithSniffedHosts
);
searchYamlTestClient = new TestCandidateAwareClient(getRestSpec(), searchClient, hosts, this::getClientBuilderWithSniffedHosts);

// check that we have an established CCS connection
Request request = new Request("GET", "_remote/info");
Expand Down Expand Up @@ -298,10 +285,22 @@ public static Iterable<Object[]> parameters() throws Exception {
@Override
protected ClientYamlTestExecutionContext createRestTestExecutionContext(
ClientYamlTestCandidate clientYamlTestCandidate,
ClientYamlTestClient clientYamlTestClient
ClientYamlTestClient clientYamlTestClient,
final Version esVersion,
final Predicate<String> clusterFeaturesPredicate,
final String os
) {
// depending on the API called, we either return the client running against the "write" or the "search" cluster here
return new ClientYamlTestExecutionContext(clientYamlTestCandidate, clientYamlTestClient, randomizeContentType()) {

// 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;
Expand All @@ -328,12 +327,9 @@ static class TestCandidateAwareClient extends ClientYamlTestClient {
ClientYamlSuiteRestSpec restSpec,
RestClient restClient,
List<HttpHost> hosts,
Version esVersion,
Predicate<String> clusterFeaturesPredicate,
String os,
CheckedSupplier<RestClientBuilder, IOException> clientBuilderWithSniffedNodes
) {
super(restSpec, restClient, hosts, esVersion, clusterFeaturesPredicate, os, clientBuilderWithSniffedNodes);
super(restSpec, restClient, hosts, clientBuilderWithSniffedNodes);
}

public void setTestCandidate(ClientYamlTestCandidate testCandidate) {
Expand Down
32 changes: 16 additions & 16 deletions ...t/src/yamlRestTest/java/org/elasticsearch/test/rest/yaml/RcsCcsCommonYamlTestSuiteIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.core.IOUtils;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.test.cluster.ElasticsearchCluster;
import org.elasticsearch.test.cluster.FeatureFlag;
import org.elasticsearch.test.cluster.local.LocalClusterConfigProvider;
Expand All @@ -46,6 +45,7 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Predicate;

import static java.util.Collections.unmodifiableList;
import static org.elasticsearch.test.rest.yaml.CcsCommonYamlTestSuiteIT.CCS_APIS;
Expand Down Expand Up @@ -221,19 +221,7 @@ public void initSearchClient() throws IOException {
clusterHosts.toArray(new HttpHost[clusterHosts.size()])
);

Tuple<Version, Version> versionVersionTuple = readVersionsFromCatNodes(adminSearchClient);
final Version esVersion = versionVersionTuple.v1();
final String os = readOsFromNodesInfo(adminSearchClient);

searchYamlTestClient = new TestCandidateAwareClient(
getRestSpec(),
searchClient,
hosts,
esVersion,
ESRestTestCase::clusterHasFeature,
os,
this::getClientBuilderWithSniffedHosts
);
searchYamlTestClient = new TestCandidateAwareClient(getRestSpec(), searchClient, hosts, this::getClientBuilderWithSniffedHosts);

configureRemoteCluster();
// check that we have an established CCS connection
Expand Down Expand Up @@ -282,10 +270,22 @@ public static Iterable<Object[]> parameters() throws Exception {
@Override
protected ClientYamlTestExecutionContext createRestTestExecutionContext(
ClientYamlTestCandidate clientYamlTestCandidate,
ClientYamlTestClient clientYamlTestClient
ClientYamlTestClient clientYamlTestClient,
final Version esVersion,
final Predicate<String> clusterFeaturesPredicate,
final String os
) {
// depending on the API called, we either return the client running against the "write" or the "search" cluster here
return new ClientYamlTestExecutionContext(clientYamlTestCandidate, clientYamlTestClient, randomizeContentType()) {

// 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;
Expand Down
41 changes: 24 additions & 17 deletions ...ster-search/src/test/java/org/elasticsearch/search/MultiClusterSearchYamlTestSuiteIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,15 @@

import org.apache.lucene.tests.util.TimeUnits;
import org.elasticsearch.Version;
import org.elasticsearch.test.rest.ESRestTestCase;
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;

@TimeoutSuite(millis = 5 * TimeUnits.MINUTE) // to account for slow as hell VMs
public class MultiClusterSearchYamlTestSuiteIT extends ESClientYamlSuiteTestCase {

Expand All @@ -33,27 +36,31 @@ public static void determineRemoteClusterMinimumVersion() {
}
}

@Override
protected ClientYamlTestExecutionContext createRestTestExecutionContext(
ClientYamlTestCandidate clientYamlTestCandidate,
ClientYamlTestClient clientYamlTestClient
ClientYamlTestClient clientYamlTestClient,
final Version esVersion,
final Predicate<String> clusterFeaturesPredicate,
final String os
) {
return new ClientYamlTestExecutionContext(clientYamlTestCandidate, clientYamlTestClient, randomizeContentType()) {
/*
* 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.
*/
final Version commonEsVersion = remoteEsVersion != null && remoteEsVersion.before(esVersion) ? remoteEsVersion : esVersion;

// TODO: same for os and features

/**
* 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.
*/
@Override
public Version esVersion() {
Version clientEsVersion = clientYamlTestClient.getEsVersion();
if (remoteEsVersion == null) {
return clientEsVersion;
} else {
return remoteEsVersion.before(clientEsVersion) ? remoteEsVersion : clientEsVersion;
}
}
};
return new ClientYamlTestExecutionContext(
clientYamlTestCandidate,
clientYamlTestClient,
randomizeContentType(),
commonEsVersion,
ESRestTestCase::clusterHasFeature,
os
);
}

@Override
Expand Down
7 changes: 1 addition & 6 deletions ...-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlDocsTestClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import org.apache.http.HttpEntity;
import org.apache.http.HttpHost;
import org.elasticsearch.Version;
import org.elasticsearch.client.NodeSelector;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
Expand All @@ -27,7 +26,6 @@
import java.util.Map;
import java.util.Objects;
import java.util.function.BiPredicate;
import java.util.function.Predicate;

/**
* Used to execute REST requests according to the docs snippets that need to be tests. Wraps a
Expand All @@ -40,12 +38,9 @@ public ClientYamlDocsTestClient(
final ClientYamlSuiteRestSpec restSpec,
final RestClient restClient,
final List<HttpHost> hosts,
final Version esVersion,
final Predicate<String> clusterFeaturesPredicate,
final String os,
final CheckedSupplier<RestClientBuilder, IOException> clientBuilderWithSniffedNodes
) {
super(restSpec, restClient, hosts, esVersion, clusterFeaturesPredicate, os, clientBuilderWithSniffedNodes);
super(restSpec, restClient, hosts, clientBuilderWithSniffedNodes);
}

@Override
Expand Down
26 changes: 0 additions & 26 deletions ...yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.apache.http.util.EntityUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.Version;
import org.elasticsearch.client.NodeSelector;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.RequestOptions;
Expand All @@ -47,7 +46,6 @@
import java.util.Map.Entry;
import java.util.Set;
import java.util.function.BiPredicate;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import static com.carrotsearch.randomizedtesting.RandomizedTest.frequently;
Expand All @@ -64,44 +62,20 @@ public class ClientYamlTestClient implements Closeable {

private final ClientYamlSuiteRestSpec restSpec;
private final Map<NodeSelector, RestClient> restClients = new HashMap<>();
private final Version esVersion;
private final String os;
private final CheckedSupplier<RestClientBuilder, IOException> clientBuilderWithSniffedNodes;
private final Predicate<String> clusterFeaturesPredicate;

ClientYamlTestClient(
final ClientYamlSuiteRestSpec restSpec,
final RestClient restClient,
final List<HttpHost> hosts,
final Version esVersion,
final Predicate<String> clusterFeaturesPredicate,
final String os,
final CheckedSupplier<RestClientBuilder, IOException> clientBuilderWithSniffedNodes
) {
this.clusterFeaturesPredicate = clusterFeaturesPredicate;
assert hosts.size() > 0;
this.restSpec = restSpec;
this.restClients.put(NodeSelector.ANY, restClient);
this.esVersion = esVersion;
this.os = os;
this.clientBuilderWithSniffedNodes = clientBuilderWithSniffedNodes;
}

/**
* @return the version of the oldest node in the cluster
*/
public Version getEsVersion() {
return esVersion;
}

public boolean clusterHasFeature(String featureId) {
return clusterFeaturesPredicate.test(featureId);
}

public String getOs() {
return os;
}

/**
* Calls an api with the provided parameters and body
*/
Expand Down
Loading

0 comments on commit 1900a99

Please sign in to comment.