Skip to content

Commit

Permalink
Support path filtering in YAML test suite (elastic#98430)
Browse files Browse the repository at this point in the history
Some Rest Specs contain multiple paths, where some paths are
preferred and the others exist for historical reasons.

This commit makes 2 changes:

1. Tracks when a path is deprecated in the Rest Spec, so that metadata
   can be accessed in the Java model of the API
2. Adds filter (predicate) to the execution context to determine which
   paths should be considered as candidates within the test suite

Tests that extends from `ESClientYamlSuiteTestCase` can override
`createRestTestExecutionContext` and pass in a predicate that skips
deprecated paths (or any other criteria), provided that it does not
skip _all_ paths in an API.
  • Loading branch information
tvernum committed Aug 15, 2023
1 parent 7bfc52b commit 50706f0
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.elasticsearch.test.cluster.FeatureFlag;
import org.elasticsearch.test.cluster.local.LocalClusterConfigProvider;
import org.elasticsearch.test.rest.ObjectPath;
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi;
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestSpec;
import org.elasticsearch.test.rest.yaml.section.ClientYamlTestSection;
import org.elasticsearch.test.rest.yaml.section.DoSection;
Expand All @@ -48,6 +49,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.BiPredicate;

import static java.util.Collections.unmodifiableList;

Expand Down Expand Up @@ -344,7 +346,8 @@ public ClientYamlTestResponse callApi(
Map<String, String> params,
HttpEntity entity,
Map<String, String> headers,
NodeSelector nodeSelector
NodeSelector nodeSelector,
BiPredicate<ClientYamlSuiteRestApi, ClientYamlSuiteRestApi.Path> pathPredicate
) throws IOException {
// on request, we need to replace index specifications by prefixing the remote cluster
if (shouldReplaceIndexWithRemote(apiName)) {
Expand All @@ -365,7 +368,7 @@ public ClientYamlTestResponse callApi(
}
params.put(parameterName, String.join(",", expandedIndices));
}
return super.callApi(apiName, params, entity, headers, nodeSelector);
return super.callApi(apiName, params, entity, headers, nodeSelector, pathPredicate);
}

private boolean shouldReplaceIndexWithRemote(String apiName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
import org.elasticsearch.client.RestClient;
import org.elasticsearch.client.RestClientBuilder;
import org.elasticsearch.common.CheckedSupplier;
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi;
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestSpec;

import java.io.IOException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.BiPredicate;

/**
* Used to execute REST requests according to the docs snippets that need to be tests. Wraps a
Expand All @@ -51,7 +53,8 @@ public ClientYamlTestResponse callApi(
Map<String, String> params,
HttpEntity entity,
Map<String, String> headers,
NodeSelector nodeSelector
NodeSelector nodeSelector,
BiPredicate<ClientYamlSuiteRestApi, ClientYamlSuiteRestApi.Path> pathPredicate
) throws IOException {

if ("raw".equals(apiName)) {
Expand All @@ -73,6 +76,6 @@ public ClientYamlTestResponse callApi(
throw new ClientYamlTestResponseException(e);
}
}
return super.callApi(apiName, params, entity, headers, nodeSelector);
return super.callApi(apiName, params, entity, headers, nodeSelector, pathPredicate);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.elasticsearch.client.RestClientBuilder;
import org.elasticsearch.client.WarningsHandler;
import org.elasticsearch.common.CheckedSupplier;
import org.elasticsearch.common.Strings;
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi;
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestSpec;

Expand All @@ -45,6 +46,7 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.function.BiPredicate;
import java.util.stream.Collectors;

import static com.carrotsearch.randomizedtesting.RandomizedTest.frequently;
Expand Down Expand Up @@ -107,7 +109,8 @@ public ClientYamlTestResponse callApi(
Map<String, String> params,
HttpEntity entity,
Map<String, String> headers,
NodeSelector nodeSelector
NodeSelector nodeSelector,
BiPredicate<ClientYamlSuiteRestApi, ClientYamlSuiteRestApi.Path> pathPredicate
) throws IOException {

ClientYamlSuiteRestApi restApi = restApi(apiName);
Expand All @@ -120,8 +123,20 @@ public ClientYamlTestResponse callApi(
.collect(Collectors.toSet());

List<ClientYamlSuiteRestApi.Path> bestPaths = restApi.getBestMatchingPaths(params.keySet());
List<ClientYamlSuiteRestApi.Path> filteredPaths = bestPaths.stream()
.filter(path -> pathPredicate.test(restApi, path))
.collect(Collectors.toUnmodifiableList());
if (filteredPaths.isEmpty()) {
throw new IllegalStateException(
Strings.format(
"All possible paths [%s] for API [%s] have been skipped",
Strings.collectionToCommaDelimitedString(bestPaths),
apiName
)
);
}
// the rest path to use is randomized out of the matching ones (if more than one)
ClientYamlSuiteRestApi.Path path = RandomizedTest.randomFrom(bestPaths);
ClientYamlSuiteRestApi.Path path = RandomizedTest.randomFrom(filteredPaths);

// divide params between ones that go within query string and ones that go within path
Map<String, String> pathParts = new HashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.elasticsearch.client.NodeSelector;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.test.rest.Stash;
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentFactory;
import org.elasticsearch.xcontent.XContentType;
Expand All @@ -29,6 +30,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.BiPredicate;

/**
* Execution context passed across the REST tests.
Expand All @@ -49,15 +51,26 @@ public class ClientYamlTestExecutionContext {
private ClientYamlTestResponse response;

private final boolean randomizeContentType;
private final BiPredicate<ClientYamlSuiteRestApi, ClientYamlSuiteRestApi.Path> pathPredicate;

public ClientYamlTestExecutionContext(
ClientYamlTestCandidate clientYamlTestCandidate,
ClientYamlTestClient clientYamlTestClient,
boolean randomizeContentType
) {
this(clientYamlTestCandidate, clientYamlTestClient, randomizeContentType, (ignoreApi, ignorePath) -> true);
}

public ClientYamlTestExecutionContext(
ClientYamlTestCandidate clientYamlTestCandidate,
ClientYamlTestClient clientYamlTestClient,
boolean randomizeContentType,
BiPredicate<ClientYamlSuiteRestApi, ClientYamlSuiteRestApi.Path> pathPredicate
) {
this.clientYamlTestClient = clientYamlTestClient;
this.clientYamlTestCandidate = clientYamlTestCandidate;
this.randomizeContentType = randomizeContentType;
this.pathPredicate = pathPredicate;
}

/**
Expand Down Expand Up @@ -183,7 +196,7 @@ ClientYamlTestResponse callApiInternal(
Map<String, String> headers,
NodeSelector nodeSelector
) throws IOException {
return clientYamlTestClient(apiName).callApi(apiName, params, entity, headers, nodeSelector);
return clientYamlTestClient(apiName).callApi(apiName, params, entity, headers, nodeSelector, pathPredicate);
}

protected ClientYamlTestClient clientYamlTestClient(String apiName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public String getLocation() {
return location;
}

void addPath(String path, String[] methods, Set<String> parts) {
void addPath(String path, String[] methods, Set<String> parts, boolean deprecated) {
Objects.requireNonNull(path, name + " API: path must not be null");
Objects.requireNonNull(methods, name + " API: methods must not be null");
if (methods.length == 0) {
Expand All @@ -81,7 +81,7 @@ void addPath(String path, String[] methods, Set<String> parts) {
throw new IllegalArgumentException(name + " API: part [" + part + "] not contained in path [" + path + "]");
}
}
boolean add = this.paths.add(new Path(path, methods, parts));
boolean add = this.paths.add(new Path(path, methods, parts, deprecated));
if (add == false) {
throw new IllegalArgumentException(name + " API: found duplicate path [" + path + "]");
}
Expand Down Expand Up @@ -194,7 +194,7 @@ public List<ClientYamlSuiteRestApi.Path> getBestMatchingPaths(Set<String> pathPa
return pathsByRelevance;
}

public record Path(String path, String[] methods, Set<String> parts) {
public record Path(String path, String[] methods, Set<String> parts, boolean deprecated) {

@Override
public boolean equals(Object o) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ public ClientYamlSuiteRestApi parse(String location, XContentParser parser) thro
String path = null;
Set<String> methods = new HashSet<>();
Set<String> pathParts = new HashSet<>();
boolean deprecated = false;
while (parser.nextToken() != XContentParser.Token.END_OBJECT) {
if ("path".equals(parser.currentName())) {
parser.nextToken();
Expand Down Expand Up @@ -160,6 +161,7 @@ public ClientYamlSuiteRestApi parse(String location, XContentParser parser) thro
apiName + " API: expected [deprecated] field in rest api definition to hold an object"
);
}
deprecated = true;
parser.skipChildren();
} else {
throw new ParsingException(
Expand All @@ -173,7 +175,7 @@ public ClientYamlSuiteRestApi parse(String location, XContentParser parser) thro
);
}
}
restApi.addPath(path, methods.toArray(new String[0]), pathParts);
restApi.addPath(path, methods.toArray(new String[0]), pathParts, deprecated);
}
} else {
throw new ParsingException(
Expand Down

0 comments on commit 50706f0

Please sign in to comment.