Browse Source

Don't allow yaml tests with `warnings` that don't skip `warnings` (#21989)

If you write a yaml test with a `warnings` section in a `do` block
that doesn't also have a corresponding `skip` section for `warnings`
then client test runners that don't support `warnings` will fail.
This causes the elasticsearch build to fail so we catch these errors
earlier.

Related to #21811
Nik Everett 8 years ago
parent
commit
e9bb8d8b38

+ 12 - 2
buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy

@@ -161,10 +161,20 @@ public class RestTestsFromSnippetsTask extends SnippetsTask {
             if (false == test.continued) {
                 current.println('---')
                 current.println("\"line_$test.start\":")
+                /* The Elasticsearch test runner doesn't support the warnings
+                 * construct unless you output this skip. Since we don't know
+                 * if this snippet will use the warnings construct we emit this
+                 * warning every time. */
+                current.println("  - skip:")
+                current.println("      features: ")
+                current.println("        - warnings")
             }
             if (test.skipTest) {
-                current.println("  - skip:")
-                current.println("      features: always_skip")
+                if (test.continued) {
+                    throw new InvalidUserDataException("Continued snippets "
+                        + "can't be skipped")
+                }
+                current.println("        - always_skip")
                 current.println("      reason: $test.skipTest")
             }
             if (test.setup != null) {

+ 9 - 0
test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/ClientYamlTestSection.java

@@ -51,6 +51,15 @@ public class ClientYamlTestSection implements Comparable<ClientYamlTestSection>
     }
 
     public void addExecutableSection(ExecutableSection executableSection) {
+        if (executableSection instanceof DoSection) {
+            DoSection doSection = (DoSection) executableSection;
+            if (false == doSection.getExpectedWarningHeaders().isEmpty()
+                    && false == skipSection.getFeatures().contains("warnings")) {
+                throw new IllegalArgumentException("Attempted to add a [do] with a [warnings] section without a corresponding [skip] so "
+                        + "runners that do not support the [warnings] section can skip the test at line ["
+                        + doSection.getLocation().lineNumber + "]");
+            }
+        }
         this.executableSections.add(executableSection);
     }
 

+ 56 - 0
test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/ClientYamlTestSectionTests.java

@@ -0,0 +1,56 @@
+/*
+ * 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.common.xcontent.XContentLocation;
+import org.elasticsearch.test.ESTestCase;
+
+import static java.util.Collections.singletonList;
+
+public class ClientYamlTestSectionTests extends ESTestCase {
+    public void testAddingDoWithoutWarningWithoutSkip() {
+        int lineNumber = between(1, 10000);
+        ClientYamlTestSection section = new ClientYamlTestSection("test");
+        section.setSkipSection(SkipSection.EMPTY);
+        DoSection doSection = new DoSection(new XContentLocation(lineNumber, 0));
+        section.addExecutableSection(doSection);
+    }
+
+    public void testAddingDoWithWarningWithSkip() {
+        int lineNumber = between(1, 10000);
+        ClientYamlTestSection section = new ClientYamlTestSection("test");
+        section.setSkipSection(new SkipSection(null, singletonList("warnings"), null));
+        DoSection doSection = new DoSection(new XContentLocation(lineNumber, 0));
+        doSection.setExpectedWarningHeaders(singletonList("foo"));
+        section.addExecutableSection(doSection);
+    }
+
+    public void testAddingDoWithWarningWithSkipButNotWarnings() {
+        int lineNumber = between(1, 10000);
+        ClientYamlTestSection section = new ClientYamlTestSection("test");
+        section.setSkipSection(new SkipSection(null, singletonList("yaml"), null));
+        DoSection doSection = new DoSection(new XContentLocation(lineNumber, 0));
+        doSection.setExpectedWarningHeaders(singletonList("foo"));
+        Exception e = expectThrows(IllegalArgumentException.class, () -> section.addExecutableSection(doSection));
+        assertEquals("Attempted to add a [do] with a [warnings] section without a corresponding [skip] so runners that do not support the"
+                + " [warnings] section can skip the test at line [" + lineNumber + "]", e.getMessage());
+    }
+
+}