From 482cf3b6768e70759abe2cf64f4e88b338247aba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenzo=20Dematt=C3=A9?= Date: Tue, 24 Oct 2023 11:24:32 +0200 Subject: [PATCH] Reapply "Making yaml tests version selector parser compatible with versions returned by Build" (#100953) * Compatible version parsing in YAML tests * Propagate exception in case of non-semantic version where one is expected * Removed remove of SNAPSHOT (no longer needed) --- .../test/rest/yaml/section/DoSection.java | 39 +++++++++++++++---- .../rest/yaml/section/DoSectionTests.java | 35 +++++++++++++++-- 2 files changed, 62 insertions(+), 12 deletions(-) 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 0220c0931bca1..6e9107152c6f7 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 @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.Build; import org.elasticsearch.Version; import org.elasticsearch.client.HasAttributeNodeSelector; import org.elasticsearch.client.Node; @@ -38,6 +39,7 @@ import java.util.Objects; import java.util.Set; import java.util.TreeMap; +import java.util.function.Predicate; import java.util.regex.Pattern; import static java.util.Collections.emptyList; @@ -626,24 +628,45 @@ public String toString() { return result; } + private static boolean matchWithRange(String nodeVersionString, List acceptedVersionRanges, XContentLocation location) { + try { + Version version = Version.fromString(nodeVersionString); + return acceptedVersionRanges.stream().anyMatch(v -> v.contains(version)); + } catch (IllegalArgumentException e) { + throw new XContentParseException( + location, + "[version] range node selector expects a semantic version format (x.y.z), but found " + nodeVersionString, + e + ); + } + } + private static NodeSelector parseVersionSelector(XContentParser parser) throws IOException { if (false == parser.currentToken().isValue()) { throw new XContentParseException(parser.getTokenLocation(), "expected [version] to be a value"); } - List skipVersionRanges = parser.text().equals("current") - ? List.of(new VersionRange(Version.CURRENT, Version.CURRENT)) - : SkipSection.parseVersionRanges(parser.text()); + + final Predicate nodeMatcher; + final String versionSelectorString; + if (parser.text().equals("current")) { + nodeMatcher = nodeVersion -> Build.current().version().equals(nodeVersion); + versionSelectorString = "version is " + Build.current().version() + " (current)"; + } else { + var acceptedVersionRange = SkipSection.parseVersionRanges(parser.text()); + nodeMatcher = nodeVersion -> matchWithRange(nodeVersion, acceptedVersionRange, parser.getTokenLocation()); + versionSelectorString = "version ranges " + acceptedVersionRange; + } + return new NodeSelector() { @Override public void select(Iterable nodes) { for (Iterator itr = nodes.iterator(); itr.hasNext();) { Node node = itr.next(); - if (node.getVersion() == null) { + String versionString = node.getVersion(); + if (versionString == null) { throw new IllegalStateException("expected [version] metadata to be set but got " + node); } - Version version = Version.fromString(node.getVersion()); - boolean skip = skipVersionRanges.stream().anyMatch(v -> v.contains(version)); - if (false == skip) { + if (nodeMatcher.test(versionString) == false) { itr.remove(); } } @@ -651,7 +674,7 @@ public void select(Iterable nodes) { @Override public String toString() { - return "version ranges " + skipVersionRanges; + return versionSelectorString; } }; } 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 88c5fdfdb1e78..501f83bb02e1f 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 @@ -9,6 +9,7 @@ package org.elasticsearch.test.rest.yaml.section; import org.apache.http.HttpHost; +import org.elasticsearch.Build; import org.elasticsearch.Version; import org.elasticsearch.client.Node; import org.elasticsearch.client.NodeSelector; @@ -19,6 +20,7 @@ import org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext; import org.elasticsearch.test.rest.yaml.ClientYamlTestResponse; import org.elasticsearch.xcontent.XContentLocation; +import org.elasticsearch.xcontent.XContentParseException; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.yaml.YamlXContent; import org.hamcrest.MatcherAssert; @@ -36,6 +38,7 @@ import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; @@ -576,7 +579,7 @@ public void testParseDoSectionAllowedWarnings() throws Exception { assertThat(e.getMessage(), equalTo("the warning [foo] was both allowed and expected")); } - public void testNodeSelectorByVersion() throws IOException { + public void testNodeSelectorByVersionRange() throws IOException { parser = createParser(YamlXContent.yamlXContent, """ node_selector: version: 5.2.0-6.0.0 @@ -626,6 +629,28 @@ public void testNodeSelectorByVersion() throws IOException { } } + public void testNodeSelectorByVersionRangeFailsWithNonSemanticVersion() throws IOException { + parser = createParser(YamlXContent.yamlXContent, """ + node_selector: + version: 5.2.0-6.0.0 + indices.get_field_mapping: + index: test_index"""); + + DoSection doSection = DoSection.parse(parser); + assertNotSame(NodeSelector.ANY, doSection.getApiCallSection().getNodeSelector()); + Node nonSemantic = nodeWithVersion("abddef"); + List nodes = new ArrayList<>(); + + var exception = expectThrows( + XContentParseException.class, + () -> doSection.getApiCallSection().getNodeSelector().select(List.of(nonSemantic)) + ); + assertThat( + exception.getMessage(), + endsWith("[version] range node selector expects a semantic version format (x.y.z), but found abddef") + ); + } + public void testNodeSelectorCurrentVersion() throws IOException { parser = createParser(YamlXContent.yamlXContent, """ node_selector: @@ -638,14 +663,16 @@ public void testNodeSelectorCurrentVersion() throws IOException { Node v170 = nodeWithVersion("1.7.0"); Node v521 = nodeWithVersion("5.2.1"); Node v550 = nodeWithVersion("5.5.0"); - Node current = nodeWithVersion(Version.CURRENT.toString()); + Node oldCurrent = nodeWithVersion(Version.CURRENT.toString()); + Node newCurrent = nodeWithVersion(Build.current().version()); List nodes = new ArrayList<>(); nodes.add(v170); nodes.add(v521); nodes.add(v550); - nodes.add(current); + nodes.add(oldCurrent); + nodes.add(newCurrent); doSection.getApiCallSection().getNodeSelector().select(nodes); - assertEquals(List.of(current), nodes); + assertEquals(List.of(oldCurrent, newCurrent), nodes); } private static Node nodeWithVersion(String version) {