From 81ff2d0f0dd6fdf35001e04a1660e2a7b3c695b1 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Tue, 10 Dec 2019 16:43:41 +0100 Subject: [PATCH] Allow skipping ranges of versions backport(#50014) (#50028) Multiple version ranges are allowed to be used in section skip in yml tests. This is useful when a bugfix was backported to latest versions and all previous releases contain a wire breaking bug. examples: 6.1.0 - 6.3.0, 6.6.0 - 6.7.9, 7.0 - - 7.2, 8.0.0 - backport #50014 --- .../search/180_locale_dependent_mapping.yml | 6 +- .../test/rest/yaml/section/DoSection.java | 7 ++- .../test/rest/yaml/section/SkipSection.java | 58 ++++++++++--------- .../test/rest/yaml/section/VersionRange.java | 49 ++++++++++++++++ .../rest/yaml/section/SkipSectionTests.java | 20 +++++++ 5 files changed, 107 insertions(+), 33 deletions(-) create mode 100644 test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/VersionRange.java diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/180_locale_dependent_mapping.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/180_locale_dependent_mapping.yml index e65b6d4d5ed..9987c1eb437 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/180_locale_dependent_mapping.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/180_locale_dependent_mapping.yml @@ -1,8 +1,10 @@ --- "Test Index and Search locale dependent mappings / dates": + - skip: - version: " - 6.8.4" - reason: JDK9 only supports this with a special sysproperty added in 6.2.0 and the fix for avoiding multiple 8 prefix is in 6.8.5 + version: " - 6.8.4, 7.0.0 - 7.4.99" + reason: JDK9 only supports this with a special sysproperty added in 6.2.0 and java.time 8prefix fix is in 6.8.5, 7.5 and master + - do: indices.create: index: test_index diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java index b74811abbbb..ccd61dd1d38 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java @@ -435,7 +435,7 @@ public class DoSection implements ExecutableSection { if (false == parser.currentToken().isValue()) { throw new XContentParseException(parser.getTokenLocation(), "expected [version] to be a value"); } - Version[] range = SkipSection.parseVersionRange(parser.text()); + List skipVersionRanges = SkipSection.parseVersionRanges(parser.text()); return new NodeSelector() { @Override public void select(Iterable nodes) { @@ -446,7 +446,8 @@ public class DoSection implements ExecutableSection { + node); } Version version = Version.fromString(node.getVersion()); - if (false == (version.onOrAfter(range[0]) && version.onOrBefore(range[1]))) { + boolean skip = skipVersionRanges.stream().anyMatch(v -> v.contains(version)); + if (false == skip) { itr.remove(); } } @@ -454,7 +455,7 @@ public class DoSection implements ExecutableSection { @Override public String toString() { - return "version between [" + range[0] + "] and [" + range[1] + "]"; + return "version ranges "+skipVersionRanges; } }; } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/SkipSection.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/SkipSection.java index e487f8e74da..81eb4708920 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/SkipSection.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/SkipSection.java @@ -27,6 +27,7 @@ import org.elasticsearch.test.rest.yaml.Features; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; /** @@ -98,33 +99,30 @@ public class SkipSection { public static final SkipSection EMPTY = new SkipSection(); - private final Version lowerVersion; - private final Version upperVersion; + private final List versionRanges; private final List features; private final String reason; private SkipSection() { - this.lowerVersion = null; - this.upperVersion = null; + this.versionRanges = new ArrayList<>(); this.features = new ArrayList<>(); this.reason = null; } public SkipSection(String versionRange, List features, String reason) { assert features != null; - Version[] versions = parseVersionRange(versionRange); - this.lowerVersion = versions[0]; - this.upperVersion = versions[1]; + this.versionRanges = parseVersionRanges(versionRange); + assert versionRanges.isEmpty() == false; this.features = features; this.reason = reason; } public Version getLowerVersion() { - return lowerVersion; + return versionRanges.get(0).getLower(); } public Version getUpperVersion() { - return upperVersion; + return versionRanges.get(versionRanges.size() - 1).getUpper(); } public List getFeatures() { @@ -139,10 +137,8 @@ public class SkipSection { if (isEmpty()) { return false; } - boolean skip = lowerVersion != null && upperVersion != null && currentVersion.onOrAfter(lowerVersion) - && currentVersion.onOrBefore(upperVersion); - skip |= Features.areAllSupported(features) == false; - return skip; + boolean skip = versionRanges.stream().anyMatch(range -> range.contains(currentVersion)); + return skip || Features.areAllSupported(features) == false; } public boolean isVersionCheck() { @@ -153,24 +149,30 @@ public class SkipSection { return EMPTY.equals(this); } - static Version[] parseVersionRange(String versionRange) { - if (versionRange == null) { - return new Version[] { null, null }; + static List parseVersionRanges(String rawRanges) { + if (rawRanges == null) { + return Collections.singletonList(new VersionRange(null, null)); } - if (versionRange.trim().equals("all")) { - return new Version[]{VersionUtils.getFirstVersion(), Version.CURRENT}; - } - String[] skipVersions = versionRange.split("-"); - if (skipVersions.length > 2) { - throw new IllegalArgumentException("version range malformed: " + versionRange); + if (rawRanges.trim().equals("all")) { + return Collections.singletonList(new VersionRange(VersionUtils.getFirstVersion(), Version.CURRENT)); } + String[] ranges = rawRanges.split(","); + List versionRanges = new ArrayList<>(); + for (String rawRange : ranges) { + String[] skipVersions = rawRange.split("-", -1); + if (skipVersions.length > 2) { + throw new IllegalArgumentException("version range malformed: " + rawRanges); + } - String lower = skipVersions[0].trim(); - String upper = skipVersions[1].trim(); - return new Version[] { - lower.isEmpty() ? VersionUtils.getFirstVersion() : Version.fromString(lower), - upper.isEmpty() ? Version.CURRENT : Version.fromString(upper) - }; + String lower = skipVersions[0].trim(); + String upper = skipVersions[1].trim(); + VersionRange versionRange = new VersionRange( + lower.isEmpty() ? VersionUtils.getFirstVersion() : Version.fromString(lower), + upper.isEmpty() ? Version.CURRENT : Version.fromString(upper) + ); + versionRanges.add(versionRange); + } + return versionRanges; } public String getSkipMessage(String description) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/VersionRange.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/VersionRange.java new file mode 100644 index 00000000000..f1b1df2a1a1 --- /dev/null +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/VersionRange.java @@ -0,0 +1,49 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.test.rest.yaml.section; + +import org.elasticsearch.Version; + +public class VersionRange { + private final Version lower; + private final Version upper; + + public VersionRange(Version lower, Version upper) { + this.lower = lower; + this.upper = upper; + } + + public Version getLower() { + return lower; + } + + public Version getUpper() { + return upper; + } + + public boolean contains(Version currentVersion) { + return lower != null && upper != null && currentVersion.onOrAfter(lower) + && currentVersion.onOrBefore(upper); + } + + @Override + public String toString() { + return "[" + lower + " - " + upper + "]"; + } +} diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/SkipSectionTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/SkipSectionTests.java index e5e466a82cc..9c8644dfee6 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/SkipSectionTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/SkipSectionTests.java @@ -33,6 +33,26 @@ import static org.hamcrest.Matchers.nullValue; public class SkipSectionTests extends AbstractClientYamlTestFragmentParserTestCase { + public void testSkipMultiRange() { + SkipSection section = new SkipSection("6.0.0 - 6.1.0, 7.1.0 - 7.5.0", + Collections.emptyList() , "foobar"); + + assertFalse(section.skip(Version.CURRENT)); + assertFalse(section.skip(Version.fromString("6.2.0"))); + assertFalse(section.skip(Version.fromString("7.0.0"))); + assertFalse(section.skip(Version.fromString("7.6.0"))); + + assertTrue(section.skip(Version.fromString("6.0.0"))); + assertTrue(section.skip(Version.fromString("6.1.0"))); + assertTrue(section.skip(Version.fromString("7.1.0"))); + assertTrue(section.skip(Version.fromString("7.5.0"))); + + section = new SkipSection("- 7.1.0, 7.2.0 - 7.5.0", + Collections.emptyList() , "foobar"); + assertTrue(section.skip(Version.fromString("7.0.0"))); + assertTrue(section.skip(Version.fromString("7.3.0"))); + } + public void testSkip() { SkipSection section = new SkipSection("6.0.0 - 6.1.0", randomBoolean() ? Collections.emptyList() : Collections.singletonList("warnings"), "foobar");