Sfoglia il codice sorgente

Docs: Allow skipping response assertions (#34240)

We generate tests from our documentation, including assertions about the
responses returned by a particular API. But sometimes we *can't* assert
that the response is correct because of some defficiency in our tooling.
Previously we marked the response `// NOTCONSOLE` to skip it, but this
is kind of odd because `// NOTCONSOLE` is really to mark snippets that
are json but aren't requests or responses. This introduces a new
construct to skip response assertions:
```
// TESTRESPONSE[skip:reason we skipped this]
```
Nik Everett 7 anni fa
parent
commit
dc2cf28fde

+ 11 - 9
buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy

@@ -226,10 +226,10 @@ public class RestTestsFromSnippetsTask extends SnippetsTask {
             } else {
                 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. */
+                /* The Elasticsearch test runner doesn't support quite a few
+                 * constructs unless we output this skip. We don't know if
+                 * we're going to use these constructs, but we might so we
+                 * output the skip just in case. */
                 current.println("  - skip:")
                 current.println("      features: ")
                 current.println("        - default_shards")
@@ -250,13 +250,13 @@ public class RestTestsFromSnippetsTask extends SnippetsTask {
                     }
                 }
             }
-            if (test.skipTest) {
+            if (test.skip) {
                 if (test.continued) {
                     throw new InvalidUserDataException("Continued snippets "
                         + "can't be skipped")
                 }
                 current.println("        - always_skip")
-                current.println("      reason: $test.skipTest")
+                current.println("      reason: $test.skip")
             }
             if (test.setup != null) {
                 // Insert a setup defined outside of the docs
@@ -274,9 +274,11 @@ public class RestTestsFromSnippetsTask extends SnippetsTask {
         }
 
         private void response(Snippet response) {
-            current.println("  - match: ")
-            current.println("      \$body: ")
-            response.contents.eachLine { current.println("        $it") }
+            if (null == response.skip) {
+                current.println("  - match: ")
+                current.println("      \$body: ")
+                response.contents.eachLine { current.println("        $it") }
+            }
         }
 
         void emitDo(String method, String pathAndQuery, String body,

+ 14 - 6
buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/SnippetsTask.groovy

@@ -122,7 +122,9 @@ public class SnippetsTask extends DefaultTask {
                             + "contain `curl`.")
                     }
                 }
-                if (snippet.testResponse && snippet.language == 'js') {
+                if (snippet.testResponse
+                        && 'js' == snippet.language
+                        && null == snippet.skip) {
                     String quoted = snippet.contents
                         // quote values starting with $
                         .replaceAll(/([:,])\s*(\$[^ ,\n}]+)/, '$1 "$2"')
@@ -216,7 +218,7 @@ public class SnippetsTask extends DefaultTask {
                                 return
                             }
                             if (it.group(4) != null) {
-                                snippet.skipTest = it.group(4)
+                                snippet.skip = it.group(4)
                                 return
                             }
                             if (it.group(5) != null) {
@@ -249,7 +251,7 @@ public class SnippetsTask extends DefaultTask {
                             substitutions = []
                         }
                         String loc = "$file:$lineNumber"
-                        parse(loc, matcher.group(2), /(?:$SUBSTITUTION|$CAT) ?/) {
+                        parse(loc, matcher.group(2), /(?:$SUBSTITUTION|$CAT|$SKIP) ?/) {
                             if (it.group(1) != null) {
                                 // TESTRESPONSE[s/adsf/jkl/]
                                 substitutions.add([it.group(1), it.group(2)])
@@ -259,6 +261,9 @@ public class SnippetsTask extends DefaultTask {
                                 substitutions.add(['\n$', '\\\\s*/'])
                                 substitutions.add(['( +)', '$1\\\\s+'])
                                 substitutions.add(['\n', '\\\\s*\n '])
+                            } else if (it.group(4) != null) {
+                                // TESTRESPONSE[skip:reason]
+                                snippet.skip = it.group(4)
                             }
                         }
                     }
@@ -312,7 +317,7 @@ public class SnippetsTask extends DefaultTask {
         boolean test = false
         boolean testResponse = false
         boolean testSetup = false
-        String skipTest = null
+        String skip = null
         boolean continued = false
         String language = null
         String catchPart = null
@@ -337,8 +342,8 @@ public class SnippetsTask extends DefaultTask {
                 if (catchPart) {
                     result += "[catch: $catchPart]"
                 }
-                if (skipTest) {
-                    result += "[skip=$skipTest]"
+                if (skip) {
+                    result += "[skip=$skip]"
                 }
                 if (continued) {
                     result += '[continued]'
@@ -352,6 +357,9 @@ public class SnippetsTask extends DefaultTask {
             }
             if (testResponse) {
                 result += '// TESTRESPONSE'
+                if (skip) {
+                    result += "[skip=$skip]"
+                }
             }
             if (testSetup) {
                 result += '// TESTSETUP'

+ 6 - 0
docs/README.asciidoc

@@ -63,6 +63,8 @@ for its modifiers:
   * `// TESTRESPONSE[_cat]`: Add substitutions for testing `_cat` responses. Use
   this after all other substitutions so it doesn't make other substitutions
   difficult.
+  * `// TESTRESPONSE[skip:reason]`: Skip the assertions specified by this
+  response.
 * `// TESTSETUP`: Marks this snippet as the "setup" for all other snippets in
   this file. This is a somewhat natural way of structuring documentation. You
   say "this is the data we use to explain this feature" then you add the
@@ -73,6 +75,10 @@ for its modifiers:
   right in the documentation file. In general, we should prefer `// TESTSETUP`
   over `// TEST[setup:name]` because it makes it more clear what steps have to
   be taken before the examples will work.
+* `// NOTCONSOLE`: Marks this snippet as neither `// CONSOLE` nor
+  `// TESTRESPONSE`, excluding it from the list of unconverted snippets. We
+  should only use this for snippets that *are* JSON but are *not* responses or
+  requests.
 
 In addition to the standard CONSOLE syntax these snippets can contain blocks
 of yaml surrounded by markers like this:

+ 1 - 1
docs/reference/aggregations/bucket/significanttext-aggregation.asciidoc

@@ -89,7 +89,7 @@ Response:
     }
 }
 --------------------------------------------------
-// NOTCONSOLE
+// TESTRESPONSE[skip:historically skipped]
 
 The results show that "h5n1" is one of several terms strongly associated with bird flu.
 It only occurs 5 times in our index as a whole (see the `bg_count`) and yet 4 of these