[TEST] Added ability to skip REST test suite/sections based on their required features

javanna · javanna · commit 0cce4e3491b4 · 2014-01-31T17:14:06.000+01:00
As we have different runners for the REST tests we need a mechanism that allows us to add features to any of them without breaking all others builds.
The idea is to name a feature and temporarily use skip sections that mention the required new features, so that runners that don't support it will skip the test.

Added support for `features` field in skip section.
Added `Features` class that contains a static list of the features supported by the runner. If a feature mentioned in a skip section is not listed here, the test will be skipped.
diff --git a/rest-api-spec/test/README.asciidoc b/rest-api-spec/test/README.asciidoc
@@ -84,6 +84,23 @@ to a string with sufficient digits to allow string comparison, eg
 Snapshot versions and versions of the form `1.0.0.Beta1` can be treated
 as the rounded down version, eg `1.0.0`.
 
+The skip section can also be used to list new features that need to be
+supported in order to run a test. This way the up-to-date runners will
+run the test, while the ones that don't support the feature yet can
+temporarily skip it, and avoid having lots of test failures in the meantime.
+
+....
+    "Parent":
+     - skip:
+          features:    regex
+
+     - do:
+       ... test definitions ...
+....
+
+The `features` field can either be a string or an array of strings.
+The skip section requires to specify either a `version` or a `features` list.
+
 Required operators:
 -------------------
 
diff --git a/src/test/java/org/elasticsearch/test/rest/junit/RestTestSuiteRunner.java b/src/test/java/org/elasticsearch/test/rest/junit/RestTestSuiteRunner.java
@@ -40,6 +40,7 @@
 import org.elasticsearch.test.rest.section.RestTestSuite;
 import org.elasticsearch.test.rest.section.TestSection;
 import org.elasticsearch.test.rest.spec.RestSpec;
+import org.elasticsearch.test.rest.support.Features;
 import org.elasticsearch.test.rest.support.FileUtils;
 import org.junit.runner.Description;
 import org.junit.runner.notification.Failure;
@@ -264,7 +265,7 @@ protected List<RestTestCandidate> collectTestCandidates(Description rootDescript
                 apiDescription.addChild(testSuiteDescription);
 
                 if (restTestSuite.getTestSections().size() == 0) {
-                    assert restTestSuite.getSetupSection().getSkipSection().skipVersion(restTestExecutionContext.esVersion());
+                    assert restTestSuite.getSetupSection().getSkipSection().skip(restTestExecutionContext.esVersion());
                     testCandidates.add(RestTestCandidate.empty(restTestSuite, testSuiteDescription));
                     continue;
                 }
@@ -274,7 +275,7 @@ protected List<RestTestCandidate> collectTestCandidates(Description rootDescript
                 for (TestSection testSection : restTestSuite.getTestSections()) {
 
                     //no need to generate seed if we are going to skip the test section
-                    if (testSection.getSkipSection().skipVersion(restTestExecutionContext.esVersion())) {
+                    if (testSection.getSkipSection().skip(restTestExecutionContext.esVersion())) {
                         Description testSectionDescription = createTestSectionIterationDescription(restTestSuite, testSection, null);
                         testSuiteDescription.addChild(testSectionDescription);
                         testCandidates.add(new RestTestCandidate(restTestSuite, testSuiteDescription, testSection, testSectionDescription, -1));
@@ -448,23 +449,37 @@ public void run() {
     protected void runChild(RestTestCandidate testCandidate, RunNotifier notifier) {
 
         //if the while suite needs to be skipped, no test sections were loaded, only an empty one that we need to mark as ignored
-        if (testCandidate.getSetupSection().getSkipSection().skipVersion(restTestExecutionContext.esVersion())) {
-            logger.info("skipped test suite [{}]\nreason: {}\nskip versions: {} (current version: {})",
-                    testCandidate.getSuiteDescription(), testCandidate.getSetupSection().getSkipSection().getReason(),
-                    testCandidate.getSetupSection().getSkipSection().getVersion(), restTestExecutionContext.esVersion());
-
+        if (testCandidate.getSetupSection().getSkipSection().skip(restTestExecutionContext.esVersion())) {
+            if (logger.isInfoEnabled()) {
+                if (testCandidate.getSetupSection().getSkipSection().isVersionCheck()) {
+                    logger.info("skipped test suite [{}]\nreason: {}\nskip versions: {} (current version: {})",
+                            testCandidate.getSuiteDescription(), testCandidate.getSetupSection().getSkipSection().getReason(),
+                            testCandidate.getSetupSection().getSkipSection().getVersion(), restTestExecutionContext.esVersion());
+                } else {
+                    logger.info("skipped test suite [{}]\nreason: feature not supported\nrequired features: {} (supported features: {})",
+                            testCandidate.getSuiteDescription(), testCandidate.getSetupSection().getSkipSection().getFeatures(), Features.getSupported());
+                }
+            }
             notifier.fireTestIgnored(testCandidate.describeSuite());
             return;
         }
 
         //from now on no more empty test candidates are expected
         assert testCandidate.getTestSection() != null;
 
-        if (testCandidate.getTestSection().getSkipSection().skipVersion(restTestExecutionContext.esVersion())) {
-            logger.info("skipped test [{}/{}]\nreason: {}\nskip versions: {} (current version: {})",
-                    testCandidate.getSuiteDescription(), testCandidate.getTestSection().getName(),
-                    testCandidate.getTestSection().getSkipSection().getReason(),
-                    testCandidate.getTestSection().getSkipSection().getVersion(), restTestExecutionContext.esVersion());
+        if (testCandidate.getTestSection().getSkipSection().skip(restTestExecutionContext.esVersion())) {
+            if (logger.isInfoEnabled()) {
+                if (testCandidate.getTestSection().getSkipSection().isVersionCheck()) {
+                    logger.info("skipped test [{}/{}]\nreason: {}\nskip versions: {} (current version: {})",
+                            testCandidate.getSuiteDescription(), testCandidate.getTestSection().getName(),
+                            testCandidate.getTestSection().getSkipSection().getReason(),
+                            testCandidate.getTestSection().getSkipSection().getVersion(), restTestExecutionContext.esVersion());
+                } else {
+                    logger.info("skipped test [{}/{}]\nreason: feature not supported\nrequired features: {} (supported features: {})",
+                            testCandidate.getSuiteDescription(), testCandidate.getTestSection().getName(),
+                            testCandidate.getTestSection().getSkipSection().getFeatures(), Features.getSupported());
+                }
+            }
 
             notifier.fireTestIgnored(testCandidate.describeTest());
             return;
diff --git a/src/test/java/org/elasticsearch/test/rest/parser/RestTestSectionParser.java b/src/test/java/org/elasticsearch/test/rest/parser/RestTestSectionParser.java
@@ -39,7 +39,7 @@ public TestSection parse(RestTestSuiteParseContext parseContext) throws IOExcept
         parser.nextToken();
         testSection.setSkipSection(parseContext.parseSkipSection());
 
-        boolean skip = testSection.getSkipSection().skipVersion(parseContext.getCurrentVersion());
+        boolean skip = testSection.getSkipSection().skip(parseContext.getCurrentVersion());
 
         while ( parser.currentToken() != XContentParser.Token.END_ARRAY) {
             if (skip) {
diff --git a/src/test/java/org/elasticsearch/test/rest/parser/RestTestSuiteParser.java b/src/test/java/org/elasticsearch/test/rest/parser/RestTestSuiteParser.java
@@ -67,7 +67,7 @@ public RestTestSuite parse(RestTestSuiteParseContext parseContext) throws IOExce
 
         restTestSuite.setSetupSection(parseContext.parseSetupSection());
 
-        boolean skip = restTestSuite.getSetupSection().getSkipSection().skipVersion(parseContext.getCurrentVersion());
+        boolean skip = restTestSuite.getSetupSection().getSkipSection().skip(parseContext.getCurrentVersion());
 
         while(true) {
             //the "---" section separator is not understood by the yaml parser. null is returned, same as when the parser is closed
diff --git a/src/test/java/org/elasticsearch/test/rest/parser/SetupSectionParser.java b/src/test/java/org/elasticsearch/test/rest/parser/SetupSectionParser.java
@@ -36,7 +36,7 @@ public SetupSection parse(RestTestSuiteParseContext parseContext) throws IOExcep
         SetupSection setupSection = new SetupSection();
         setupSection.setSkipSection(parseContext.parseSkipSection());
 
-        boolean skip = setupSection.getSkipSection().skipVersion(parseContext.getCurrentVersion());
+        boolean skip = setupSection.getSkipSection().skip(parseContext.getCurrentVersion());
 
         while (parser.currentToken() != XContentParser.Token.END_ARRAY) {
             if (skip) {
diff --git a/src/test/java/org/elasticsearch/test/rest/parser/SkipSectionParser.java b/src/test/java/org/elasticsearch/test/rest/parser/SkipSectionParser.java
@@ -18,11 +18,13 @@
  */
 package org.elasticsearch.test.rest.parser;
 
+import com.google.common.collect.Lists;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.test.rest.section.SkipSection;
 
 import java.io.IOException;
+import java.util.List;
 
 /**
  * Parser for skip sections
@@ -38,6 +40,7 @@ public SkipSection parse(RestTestSuiteParseContext parseContext) throws IOExcept
         XContentParser.Token token;
         String version = null;
         String reason = null;
+        List<String> features = Lists.newArrayList();
 
         while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
             if (token == XContentParser.Token.FIELD_NAME) {
@@ -47,22 +50,34 @@ public SkipSection parse(RestTestSuiteParseContext parseContext) throws IOExcept
                     version = parser.text();
                 } else if ("reason".equals(currentFieldName)) {
                     reason = parser.text();
-                } else {
+                } else if ("features".equals(currentFieldName)) {
+                    features.add(parser.text());
+                }
+                else {
                     throw new RestTestParseException("field " + currentFieldName + " not supported within skip section");
                 }
+            } else if (token == XContentParser.Token.START_ARRAY) {
+                if ("features".equals(currentFieldName)) {
+                    while(parser.nextToken() != XContentParser.Token.END_ARRAY) {
+                        features.add(parser.text());
+                    }
+                }
             }
         }
 
         parser.nextToken();
 
-        if (!Strings.hasLength(version)) {
-            throw new RestTestParseException("version is mandatory within skip section");
+        if (!Strings.hasLength(version) && features.isEmpty()) {
+            throw new RestTestParseException("version or features is mandatory within skip section");
+        }
+        if (Strings.hasLength(version) && !features.isEmpty()) {
+            throw new RestTestParseException("version or features are mutually exclusive");
         }
-        if (!Strings.hasLength(reason)) {
-            throw new RestTestParseException("reason is mandatory within skip section");
+        if (Strings.hasLength(version) && !Strings.hasLength(reason)) {
+            throw new RestTestParseException("reason is mandatory within skip version section");
         }
 
-        return new SkipSection(version, reason);
+        return new SkipSection(version, features, reason);
 
     }
 }
diff --git a/src/test/java/org/elasticsearch/test/rest/section/SkipSection.java b/src/test/java/org/elasticsearch/test/rest/section/SkipSection.java
@@ -18,37 +18,63 @@
  */
 package org.elasticsearch.test.rest.section;
 
+import com.google.common.collect.Lists;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.test.rest.support.Features;
 import org.elasticsearch.test.rest.support.VersionUtils;
 
+import java.util.List;
+
 /**
  * Represents a skip section that tells whether a specific test section or suite needs to be skipped
- * based on the elasticsearch version the tests are running against.
+ * based on:
+ * - the elasticsearch version the tests are running against
+ * - a specific test feature required that might not be implemented yet by the runner
  */
 public class SkipSection {
 
-    public static final SkipSection EMPTY = new SkipSection("", "");
+    public static final SkipSection EMPTY = new SkipSection("", Lists.<String>newArrayList(), "");
 
     private final String version;
+    private final List<String> features;
     private final String reason;
 
-    public SkipSection(String version, String reason) {
+    public SkipSection(String version, List<String> features, String reason) {
         this.version = version;
+        this.features = features;
         this.reason = reason;
     }
 
     public String getVersion() {
         return version;
     }
 
+    public List<String> getFeatures() {
+        return features;
+    }
+
     public String getReason() {
         return reason;
     }
 
-    public boolean skipVersion(String currentVersion) {
+    public boolean skip(String currentVersion) {
         if (isEmpty()) {
             return false;
         }
-        return VersionUtils.skipCurrentVersion(version, currentVersion);
+
+        if (version != null) {
+            return VersionUtils.skipCurrentVersion(version, currentVersion);
+        }
+
+        if (features != null && !this.features.isEmpty()) {
+            return !Features.areAllSupported(this.features);
+        }
+
+        throw new IllegalArgumentException("version or feature should be not null in a non empty skip section");
+    }
+
+    public boolean isVersionCheck() {
+        return Strings.hasLength(version);
     }
 
     public boolean isEmpty() {
diff --git a/src/test/java/org/elasticsearch/test/rest/support/Features.java b/src/test/java/org/elasticsearch/test/rest/support/Features.java
@@ -0,0 +1,57 @@
+/*
+ * 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.support;
+
+import com.google.common.collect.Lists;
+
+import java.util.List;
+
+/**
+ * Allows to register additional features supported by the tests runner.
+ * This way any runner can add extra features and use proper skip sections to avoid
+ * breaking others runners till they have implemented the new feature as well.
+ */
+public final class Features {
+
+    private static final List<String> SUPPORTED = Lists.newArrayList();
+
+    private Features() {
+
+    }
+
+    /**
+     * Tells whether all the features provided as argument are supported
+     */
+    public static boolean areAllSupported(List<String> features) {
+        for (String feature : features) {
+            if (!SUPPORTED.contains(feature)) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    /**
+     * Returns all the supported features
+     */
+    public static List<String> getSupported() {
+        return SUPPORTED;
+    }
+}
diff --git a/src/test/java/org/elasticsearch/test/rest/test/SkipSectionParserTests.java b/src/test/java/org/elasticsearch/test/rest/test/SkipSectionParserTests.java
