From 0641e1807605f125e20dc5c532872b23b0257775 Mon Sep 17 00:00:00 2001 From: David Kyle Date: Tue, 19 Jun 2018 11:34:05 +0100 Subject: [PATCH 1/3] Only assert no shard failures for doc write operations --- .../doc/RestTestsFromSnippetsTask.groovy | 27 +++++++++++++------ .../doc/RestTestsFromSnippetsTaskTest.groovy | 11 +++++--- 2 files changed, 27 insertions(+), 11 deletions(-) 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 adacc1863c595..be1f2b6008eb2 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy @@ -27,7 +27,7 @@ import org.gradle.api.tasks.OutputDirectory import java.nio.file.Files import java.nio.file.Path -import java.util.regex.Matcher +import java.util.regex.Pattern /** * Generates REST tests for each snippet marked // TEST. @@ -39,6 +39,12 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { */ private static final List BAD_LANGUAGES = ['json', 'javascript'] + /** + * Doc write operations start with an index name that cannot + * start with -, _ or + and must be lower case. + */ + private static final Pattern DOCS_WRITE_OP_PATTERN = Pattern.compile("^[^_\\-\\+][a-z_-]+/"); + @Input Map setups = new HashMap() @@ -100,6 +106,13 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { return snippet.language == 'js' || snippet.curl } + /** + * Is the URL path a doc write request? + */ + static isDocWriteRequest(String path) { + return DOCS_WRITE_OP_PATTERN.matcher(path).find(); + } + /** * Converts Kibana's block quoted strings into standard JSON. These * {@code """} delimited strings can be embedded in CONSOLE and can @@ -308,14 +321,12 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { /* Catch any shard failures. These only cause a non-200 response if * no shard succeeds. But we need to fail the tests on all of these * because they mean invalid syntax or broken queries or something - * else that we don't want to teach people to do. The REST test - * framework doesn't allow us to has assertions in the setup - * section so we have to skip it there. We also have to skip _cat - * actions because they don't return json so we can't is_false - * them. That is ok because they don't have this - * partial-success-is-success thing. + * else that we don't want to teach people to do. Shard failures + * can occur in document CRUD operations. The REST test + * framework doesn't allow us to have assertions in the setup + * section so we have to skip it there. */ - if (false == inSetup && false == path.startsWith('_cat')) { + if (false == inSetup && isDocWriteRequest(path)) { current.println(" - is_false: _shards.failures") } } diff --git a/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy b/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy index d0a7a2825e6f2..5c3e9a20089a3 100644 --- a/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy +++ b/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy @@ -19,9 +19,7 @@ package org.elasticsearch.gradle.doc -import org.elasticsearch.gradle.doc.SnippetsTask.Snippet -import org.gradle.api.InvalidUserDataException - +import static org.elasticsearch.gradle.doc.RestTestsFromSnippetsTask.isDocWriteRequest import static org.elasticsearch.gradle.doc.RestTestsFromSnippetsTask.replaceBlockQuote class RestTestFromSnippetsTaskTest extends GroovyTestCase { @@ -47,4 +45,11 @@ class RestTestFromSnippetsTaskTest extends GroovyTestCase { assertEquals("\"foo\": \"bort\\n baz\"", replaceBlockQuote("\"foo\": \"\"\"bort\n baz\"\"\"")); } + + void testIsDocWriteRequest() { + assertTrue(isDocWriteRequest("doc-index/doc_id")); + assertTrue(isDocWriteRequest("doc_index/doc_type/doc_id")); + assertFalse(isDocWriteRequest("doc_index")) + assertFalse(isDocWriteRequest("_xpack/ml/datafeeds/datafeed-id/_preview")); + } } From b629d7eed09b6799a038e46985bc96d317ac29ad Mon Sep 17 00:00:00 2001 From: David Kyle Date: Tue, 19 Jun 2018 15:28:20 +0100 Subject: [PATCH 2/3] Update regex to exclude API calls on the index --- .../gradle/doc/RestTestsFromSnippetsTask.groovy | 6 ++++-- .../gradle/doc/RestTestsFromSnippetsTaskTest.groovy | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) 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 be1f2b6008eb2..c85cda5e7e2a8 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy @@ -41,9 +41,11 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { /** * Doc write operations start with an index name that cannot - * start with -, _ or + and must be lower case. + * start with -, _ or + and must be lower case and are followed + * by the doc type or doc Id. If the 2nd part of the path + * (after the '/') starts with a '_' then it is an API call. */ - private static final Pattern DOCS_WRITE_OP_PATTERN = Pattern.compile("^[^_\\-\\+][a-z_-]+/"); + private static final Pattern DOCS_WRITE_OP_PATTERN = Pattern.compile("^[^_\\-\\+][a-z_-]+/[^_]"); @Input Map setups = new HashMap() diff --git a/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy b/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy index 5c3e9a20089a3..6b61a1aaf23cf 100644 --- a/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy +++ b/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy @@ -49,7 +49,7 @@ class RestTestFromSnippetsTaskTest extends GroovyTestCase { void testIsDocWriteRequest() { assertTrue(isDocWriteRequest("doc-index/doc_id")); assertTrue(isDocWriteRequest("doc_index/doc_type/doc_id")); - assertFalse(isDocWriteRequest("doc_index")) + assertFalse(isDocWriteRequest("doc_index/_search")) assertFalse(isDocWriteRequest("_xpack/ml/datafeeds/datafeed-id/_preview")); } } From 539f057c53e3bf3ed363ab6e51b8771db50c674f Mon Sep 17 00:00:00 2001 From: David Kyle Date: Tue, 19 Jun 2018 17:07:50 +0100 Subject: [PATCH 3/3] Revert to filtering _cat and _xpack/ml/datafeed APIs --- .../doc/RestTestsFromSnippetsTask.groovy | 24 +++++++------------ .../doc/RestTestsFromSnippetsTaskTest.groovy | 9 ++++--- 2 files changed, 12 insertions(+), 21 deletions(-) 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 c85cda5e7e2a8..3c056a5528b5e 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy @@ -27,7 +27,6 @@ import org.gradle.api.tasks.OutputDirectory import java.nio.file.Files import java.nio.file.Path -import java.util.regex.Pattern /** * Generates REST tests for each snippet marked // TEST. @@ -39,14 +38,6 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { */ private static final List BAD_LANGUAGES = ['json', 'javascript'] - /** - * Doc write operations start with an index name that cannot - * start with -, _ or + and must be lower case and are followed - * by the doc type or doc Id. If the 2nd part of the path - * (after the '/') starts with a '_' then it is an API call. - */ - private static final Pattern DOCS_WRITE_OP_PATTERN = Pattern.compile("^[^_\\-\\+][a-z_-]+/[^_]"); - @Input Map setups = new HashMap() @@ -109,10 +100,11 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { } /** - * Is the URL path a doc write request? + * Certain requests should not have the shard failure check because the + * format of the response is incompatible i.e. it is not a JSON object. */ - static isDocWriteRequest(String path) { - return DOCS_WRITE_OP_PATTERN.matcher(path).find(); + static shouldAddShardFailureCheck(String path) { + return path.startsWith('_cat') == false && path.startsWith('_xpack/ml/datafeeds/') == false } /** @@ -323,12 +315,12 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { /* Catch any shard failures. These only cause a non-200 response if * no shard succeeds. But we need to fail the tests on all of these * because they mean invalid syntax or broken queries or something - * else that we don't want to teach people to do. Shard failures - * can occur in document CRUD operations. The REST test + * else that we don't want to teach people to do. The REST test * framework doesn't allow us to have assertions in the setup - * section so we have to skip it there. + * section so we have to skip it there. We also omit the assertion + * from APIs that don't return a JSON object */ - if (false == inSetup && isDocWriteRequest(path)) { + if (false == inSetup && shouldAddShardFailureCheck(path)) { current.println(" - is_false: _shards.failures") } } diff --git a/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy b/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy index 6b61a1aaf23cf..b986319492001 100644 --- a/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy +++ b/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy @@ -19,7 +19,7 @@ package org.elasticsearch.gradle.doc -import static org.elasticsearch.gradle.doc.RestTestsFromSnippetsTask.isDocWriteRequest +import static org.elasticsearch.gradle.doc.RestTestsFromSnippetsTask.shouldAddShardFailureCheck import static org.elasticsearch.gradle.doc.RestTestsFromSnippetsTask.replaceBlockQuote class RestTestFromSnippetsTaskTest extends GroovyTestCase { @@ -47,9 +47,8 @@ class RestTestFromSnippetsTaskTest extends GroovyTestCase { } void testIsDocWriteRequest() { - assertTrue(isDocWriteRequest("doc-index/doc_id")); - assertTrue(isDocWriteRequest("doc_index/doc_type/doc_id")); - assertFalse(isDocWriteRequest("doc_index/_search")) - assertFalse(isDocWriteRequest("_xpack/ml/datafeeds/datafeed-id/_preview")); + assertTrue(shouldAddShardFailureCheck("doc-index/_search")); + assertFalse(shouldAddShardFailureCheck("_cat")) + assertFalse(shouldAddShardFailureCheck("_xpack/ml/datafeeds/datafeed-id/_preview")); } }