From 08ea8f2dbc935312bff459b0a488fa9d892991ff Mon Sep 17 00:00:00 2001
From: Nik Everett <nik9000@gmail.com>
Date: Sat, 24 Jun 2017 22:02:57 -0400
Subject: [PATCH] Throw useful error on bad docs snippets

You can continue a test started in a previous snippet by marking the
next snippet with `// TEST[continued]`. The trouble is, if you mark the
first snippet in a file with `// TEST[continued]` you'd get difficult
to predict behavior because you'd continue the test started in another
file. This will usually create a test that fails the build but it isn't
easy to track down what you did wrong. This commit catches this
scenario up front and fails the build with a useful error message.

Similarly, if you put `// TEST[continued]` directly after a
`// TESTSETUP` section then the docs tests will fail to run but the
error message did not point you to the `// TEST[continued]` snippet.
This commit catches this scenario up front as well and fails the build
with a useful error message.
---
 .../doc/RestTestsFromSnippetsTask.groovy      | 20 ++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy
index f126839a8d48a..0395b31786f63 100644
--- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy
+++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy
@@ -127,6 +127,11 @@ public class RestTestsFromSnippetsTask extends SnippetsTask {
          */
         Set<String> unconvertedCandidates = new HashSet<>()
 
+        /**
+         * The last non-TESTRESPONSE snippet.
+         */
+        Snippet previousTest
+
         /**
          * Called each time a snippet is encountered. Tracks the snippets and
          * calls buildTest to actually build the test.
@@ -142,6 +147,7 @@ public class RestTestsFromSnippetsTask extends SnippetsTask {
             }
             if (snippet.testSetup) {
                 setup(snippet)
+                previousTest = snippet
                 return
             }
             if (snippet.testResponse) {
@@ -150,6 +156,7 @@ public class RestTestsFromSnippetsTask extends SnippetsTask {
             }
             if (snippet.test || snippet.console) {
                 test(snippet)
+                previousTest = snippet
                 return
             }
             // Must be an unmarked snippet....
@@ -158,7 +165,18 @@ public class RestTestsFromSnippetsTask extends SnippetsTask {
         private void test(Snippet test) {
             setupCurrent(test)
 
-            if (false == test.continued) {
+            if (test.continued) {
+                /* Catch some difficult to debug errors with // TEST[continued]
+                 * and throw a helpful error message. */
+                if (previousTest == null || previousTest.path != test.path) {
+                    throw new InvalidUserDataException("// TEST[continued] " +
+                        "cannot be on first snippet in a file: $test")
+                }
+                if (previousTest != null && previousTest.testSetup) {
+                    throw new InvalidUserDataException("// TEST[continued] " +
+                        "cannot immediately follow // TESTSETUP: $test")
+                }
+            } else {
                 current.println('---')
                 current.println("\"line_$test.start\":")
                 /* The Elasticsearch test runner doesn't support the warnings