Skip to content

Commit d57b034

Browse files
ldematteldematte
authored
Introduce new overloads for ClientYamlTestExecutionContext and deprecate old ones (#103626)
Smaller steps to introduce a signature change without breaking submodule update in serverless
1 parent 2a7866f commit d57b034

File tree

4 files changed

+112
-63
lines changed

4 files changed

+112
-63
lines changed

‎test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ public void initClient() throws IOException {
310310
.collect(Collectors.toSet());
311311
assert semanticNodeVersions.isEmpty() == false || serverless;
312312

313-
testFeatureService = createTestFeatureService(adminClient, semanticNodeVersions);
313+
testFeatureService = createTestFeatureService(getClusterStateFeatures(adminClient), semanticNodeVersions);
314314
}
315315

316316
assert testFeatureService != null;
@@ -321,8 +321,10 @@ public void initClient() throws IOException {
321321
assert nodesVersions != null;
322322
}
323323

324-
protected static TestFeatureService createTestFeatureService(RestClient adminClient, Set<Version> semanticNodeVersions)
325-
throws IOException {
324+
protected static TestFeatureService createTestFeatureService(
325+
Map<String, Set<String>> clusterStateFeatures,
326+
Set<Version> semanticNodeVersions
327+
) {
326328
// Historical features information is unavailable when using legacy test plugins
327329
boolean hasHistoricalFeaturesInformation = System.getProperty("tests.features.metadata.path") != null;
328330
var providers = hasHistoricalFeaturesInformation
@@ -333,7 +335,7 @@ protected static TestFeatureService createTestFeatureService(RestClient adminCli
333335
hasHistoricalFeaturesInformation,
334336
providers,
335337
semanticNodeVersions,
336-
ClusterFeatures.calculateAllNodeFeatures(getClusterStateFeatures(adminClient).values())
338+
ClusterFeatures.calculateAllNodeFeatures(clusterStateFeatures.values())
337339
);
338340
}
339341

@@ -2094,7 +2096,7 @@ public void ensurePeerRecoveryRetentionLeasesRenewedAndSynced(String index) thro
20942096
}, 60, TimeUnit.SECONDS);
20952097
}
20962098

2097-
private static Map<String, Set<String>> getClusterStateFeatures(RestClient adminClient) throws IOException {
2099+
protected static Map<String, Set<String>> getClusterStateFeatures(RestClient adminClient) throws IOException {
20982100
final Request request = new Request("GET", "_cluster/state");
20992101
request.addParameter("filter_path", "nodes_features");
21002102

@@ -2187,7 +2189,7 @@ protected static TransportVersion getTransportVersionWithFallback(
21872189
return fallbackSupplier.get();
21882190
}
21892191

2190-
protected static Optional<Version> parseLegacyVersion(String version) {
2192+
public static Optional<Version> parseLegacyVersion(String version) {
21912193
var semanticVersionMatcher = SEMANTIC_VERSION_PATTERN.matcher(version);
21922194
if (semanticVersionMatcher.matches()) {
21932195
return Optional.of(Version.fromString(semanticVersionMatcher.group(1)));

‎test/framework/src/main/java/org/elasticsearch/test/rest/TestFeatureService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ private static boolean hasHistoricalFeature(NavigableMap<Version, Set<String>> h
5656
return features != null && features.getValue().contains(featureId);
5757
}
5858

59-
boolean clusterHasFeature(String featureId) {
59+
public boolean clusterHasFeature(String featureId) {
6060
if (clusterStateFeatures.contains(featureId)) {
6161
return true;
6262
}

‎test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,11 @@
1717
import org.apache.lucene.util.BytesRef;
1818
import org.elasticsearch.Version;
1919
import org.elasticsearch.client.NodeSelector;
20+
import org.elasticsearch.common.VersionId;
2021
import org.elasticsearch.common.bytes.BytesReference;
22+
import org.elasticsearch.test.rest.ESRestTestCase;
2123
import org.elasticsearch.test.rest.Stash;
24+
import org.elasticsearch.test.rest.TestFeatureService;
2225
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi;
2326
import org.elasticsearch.xcontent.XContentBuilder;
2427
import org.elasticsearch.xcontent.XContentFactory;
@@ -30,6 +33,8 @@
3033
import java.util.HashMap;
3134
import java.util.List;
3235
import java.util.Map;
36+
import java.util.Optional;
37+
import java.util.Set;
3338
import java.util.function.BiPredicate;
3439
import java.util.function.Predicate;
3540

@@ -59,6 +64,57 @@ public class ClientYamlTestExecutionContext {
5964
private final boolean randomizeContentType;
6065
private final BiPredicate<ClientYamlSuiteRestApi, ClientYamlSuiteRestApi.Path> pathPredicate;
6166

67+
// TODO: this will become the ctor once work is done on serverless side and https://github.com/elastic/elasticsearch/pull/103311/
68+
// has been merged
69+
public ClientYamlTestExecutionContext(
70+
ClientYamlTestCandidate clientYamlTestCandidate,
71+
ClientYamlTestClient clientYamlTestClient,
72+
boolean randomizeContentType,
73+
final Set<String> nodesVersions,
74+
final TestFeatureService testFeatureService,
75+
final Set<String> osList,
76+
BiPredicate<ClientYamlSuiteRestApi, ClientYamlSuiteRestApi.Path> pathPredicate
77+
) {
78+
this(
79+
clientYamlTestCandidate,
80+
clientYamlTestClient,
81+
randomizeContentType,
82+
getEsVersion(nodesVersions),
83+
testFeatureService::clusterHasFeature,
84+
osList.iterator().next(),
85+
pathPredicate
86+
);
87+
}
88+
89+
public ClientYamlTestExecutionContext(
90+
ClientYamlTestCandidate clientYamlTestCandidate,
91+
ClientYamlTestClient clientYamlTestClient,
92+
boolean randomizeContentType,
93+
final Set<String> nodesVersions,
94+
final TestFeatureService testFeatureService,
95+
final Set<String> osList
96+
) {
97+
this(
98+
clientYamlTestCandidate,
99+
clientYamlTestClient,
100+
randomizeContentType,
101+
nodesVersions,
102+
testFeatureService,
103+
osList,
104+
(ignoreApi, ignorePath) -> true
105+
);
106+
}
107+
108+
@Deprecated
109+
static Version getEsVersion(Set<String> nodesVersions) {
110+
return nodesVersions.stream()
111+
.map(ESRestTestCase::parseLegacyVersion)
112+
.flatMap(Optional::stream)
113+
.min(VersionId::compareTo)
114+
.orElse(Version.CURRENT);
115+
}
116+
117+
@Deprecated
62118
public ClientYamlTestExecutionContext(
63119
ClientYamlTestCandidate clientYamlTestCandidate,
64120
ClientYamlTestClient clientYamlTestClient,
@@ -78,6 +134,7 @@ public ClientYamlTestExecutionContext(
78134
);
79135
}
80136

137+
@Deprecated
81138
public ClientYamlTestExecutionContext(
82139
ClientYamlTestCandidate clientYamlTestCandidate,
83140
ClientYamlTestClient clientYamlTestClient,

‎test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java‎

Lines changed: 46 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@
2929
import org.elasticsearch.common.settings.Settings;
3030
import org.elasticsearch.common.xcontent.support.XContentMapValues;
3131
import org.elasticsearch.core.IOUtils;
32-
import org.elasticsearch.core.Tuple;
3332
import org.elasticsearch.test.ClasspathUtils;
3433
import org.elasticsearch.test.rest.ESRestTestCase;
34+
import org.elasticsearch.test.rest.TestFeatureService;
3535
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi;
3636
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestSpec;
3737
import org.elasticsearch.test.rest.yaml.section.ClientYamlTestSection;
@@ -62,7 +62,9 @@
6262
import java.util.SortedSet;
6363
import java.util.TreeSet;
6464
import java.util.function.Predicate;
65+
import java.util.stream.Collectors;
6566

67+
import static org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext.getEsVersion;
6668
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
6769

6870
/**
@@ -139,33 +141,35 @@ public void initAndResetContext() throws Exception {
139141
validateSpec(restSpec);
140142
restSpecification = restSpec;
141143
final List<HttpHost> hosts = getClusterHosts();
142-
Tuple<Version, Version> versionVersionTuple = readVersionsFromCatNodes(adminClient());
143-
final Version esVersion = versionVersionTuple.v1();
144-
final Version masterVersion = versionVersionTuple.v2();
144+
final Set<String> nodesVersions = getCachedNodesVersions();
145145
final String os = readOsFromNodesInfo(adminClient());
146146

147-
logger.info(
148-
"initializing client, minimum es version [{}], master version, [{}], hosts {}, os [{}]",
149-
esVersion,
150-
masterVersion,
151-
hosts,
152-
os
147+
var semanticNodeVersions = nodesVersions.stream()
148+
.map(ESRestTestCase::parseLegacyVersion)
149+
.flatMap(Optional::stream)
150+
.collect(Collectors.toSet());
151+
final TestFeatureService testFeatureService = createTestFeatureService(
152+
getClusterStateFeatures(adminClient()),
153+
semanticNodeVersions
153154
);
155+
156+
logger.info("initializing client, node versions [{}], hosts {}, os [{}]", nodesVersions, hosts, os);
157+
154158
clientYamlTestClient = initClientYamlTestClient(restSpec, client(), hosts);
155159
restTestExecutionContext = createRestTestExecutionContext(
156160
testCandidate,
157161
clientYamlTestClient,
158-
esVersion,
159-
ESRestTestCase::clusterHasFeature,
160-
os
162+
nodesVersions,
163+
testFeatureService,
164+
Set.of(os)
161165
);
162166
adminExecutionContext = new ClientYamlTestExecutionContext(
163167
testCandidate,
164168
clientYamlTestClient,
165169
false,
166-
esVersion,
167-
ESRestTestCase::clusterHasFeature,
168-
os
170+
nodesVersions,
171+
testFeatureService,
172+
Set.of(os)
169173
);
170174
final String[] blacklist = resolvePathsProperty(REST_TESTS_BLACKLIST, null);
171175
blacklistPathMatchers = new ArrayList<>();
@@ -190,6 +194,23 @@ public void initAndResetContext() throws Exception {
190194
/**
191195
* Create the test execution context. Can be overwritten in sub-implementations of the test if the context needs to be modified.
192196
*/
197+
protected ClientYamlTestExecutionContext createRestTestExecutionContext(
198+
ClientYamlTestCandidate clientYamlTestCandidate,
199+
ClientYamlTestClient clientYamlTestClient,
200+
final Set<String> nodesVersions,
201+
final TestFeatureService testFeatureService,
202+
final Set<String> osList
203+
) {
204+
return createRestTestExecutionContext(
205+
clientYamlTestCandidate,
206+
clientYamlTestClient,
207+
getEsVersion(nodesVersions),
208+
testFeatureService::clusterHasFeature,
209+
osList.iterator().next()
210+
);
211+
}
212+
213+
@Deprecated
193214
protected ClientYamlTestExecutionContext createRestTestExecutionContext(
194215
ClientYamlTestCandidate clientYamlTestCandidate,
195216
ClientYamlTestClient clientYamlTestClient,
@@ -319,13 +340,15 @@ static Map<String, Set<Path>> loadSuites(String... paths) throws Exception {
319340
for (String strPath : paths) {
320341
Path path = root.resolve(strPath);
321342
if (Files.isDirectory(path)) {
322-
Files.walk(path).forEach(file -> {
323-
if (file.toString().endsWith(".yml")) {
324-
addSuite(root, file, files);
325-
} else if (file.toString().endsWith(".yaml")) {
326-
throw new IllegalArgumentException("yaml files are no longer supported: " + file);
327-
}
328-
});
343+
try (var filesStream = Files.walk(path)) {
344+
filesStream.forEach(file -> {
345+
if (file.toString().endsWith(".yml")) {
346+
addSuite(root, file, files);
347+
} else if (file.toString().endsWith(".yaml")) {
348+
throw new IllegalArgumentException("yaml files are no longer supported: " + file);
349+
}
350+
});
351+
}
329352
} else {
330353
path = root.resolve(strPath + ".yml");
331354
assert Files.exists(path) : "Path " + path + " does not exist in YAML test root";
@@ -402,35 +425,6 @@ private static void validateSpec(ClientYamlSuiteRestSpec restSpec) {
402425
}
403426
}
404427

405-
Tuple<Version, Version> readVersionsFromCatNodes(RestClient restClient) throws IOException {
406-
// we simply go to the _cat/nodes API and parse all versions in the cluster
407-
final Request request = new Request("GET", "/_cat/nodes");
408-
request.addParameter("h", "version,master");
409-
request.setOptions(getCatNodesVersionMasterRequestOptions());
410-
Response response = restClient.performRequest(request);
411-
ClientYamlTestResponse restTestResponse = new ClientYamlTestResponse(response);
412-
String nodesCatResponse = restTestResponse.getBodyAsString();
413-
String[] split = nodesCatResponse.split("\n");
414-
Version version = null;
415-
Version masterVersion = null;
416-
for (String perNode : split) {
417-
final String[] versionAndMaster = perNode.split("\\s+");
418-
assert versionAndMaster.length == 2 : "invalid line: " + perNode + " length: " + versionAndMaster.length;
419-
final Version currentVersion = Version.fromString(versionAndMaster[0]);
420-
final boolean master = versionAndMaster[1].trim().equals("*");
421-
if (master) {
422-
assert masterVersion == null;
423-
masterVersion = currentVersion;
424-
}
425-
if (version == null) {
426-
version = currentVersion;
427-
} else if (version.onOrAfter(currentVersion)) {
428-
version = currentVersion;
429-
}
430-
}
431-
return new Tuple<>(version, masterVersion);
432-
}
433-
434428
static String readOsFromNodesInfo(RestClient restClient) throws IOException {
435429
final Request request = new Request("GET", "/_nodes/os");
436430
Response response = restClient.performRequest(request);
@@ -459,10 +453,6 @@ static String readOsFromNodesInfo(RestClient restClient) throws IOException {
459453
return osPrettyNames.last();
460454
}
461455

462-
protected RequestOptions getCatNodesVersionMasterRequestOptions() {
463-
return RequestOptions.DEFAULT;
464-
}
465-
466456
public void test() throws IOException {
467457
// skip test if it matches one of the blacklist globs
468458
for (BlacklistedPathPatternMatcher blacklistedPathMatcher : blacklistPathMatchers) {

0 commit comments

Comments
 (0)