From ddc531e729d403247d6d53512a7b004ebceb6799 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 5 May 2016 16:46:40 -0400 Subject: [PATCH] Build a plugin for testing docs This makes it much easier to apply to other projects. Fixes to doc tests infrastructure: * Fix comparing lists. Was totally broken. * Fix order of actual vs expected parameters. * Allow multiple `// TESTRESPONSE` lines with substitutions to join into one big list of subtitutions. This makes lets the docs look tidier. * Exclude build from snippet scanning * Allow subclasses of ESRestTestCase access to the admin execution context --- .../gradle/doc/DocsTestPlugin.groovy | 65 +++++++++++++++++ .../RestTestsFromSnippetsTask.groovy | 4 +- .../gradle/{ => doc}/SnippetsTask.groovy | 8 ++- .../elasticsearch.docs-test.properties | 20 ++++++ docs/build.gradle | 70 ++++++------------- .../test/rest/ESRestTestCase.java | 4 ++ .../rest/section/ResponseBodyAssertion.java | 13 ++-- 7 files changed, 125 insertions(+), 59 deletions(-) create mode 100644 buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/DocsTestPlugin.groovy rename buildSrc/src/main/groovy/org/elasticsearch/gradle/{ => doc}/RestTestsFromSnippetsTask.groovy (98%) rename buildSrc/src/main/groovy/org/elasticsearch/gradle/{ => doc}/SnippetsTask.groovy (97%) create mode 100644 buildSrc/src/main/resources/META-INF/gradle-plugins/elasticsearch.docs-test.properties diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/DocsTestPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/DocsTestPlugin.groovy new file mode 100644 index 00000000000..3c3ad15a45b --- /dev/null +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/DocsTestPlugin.groovy @@ -0,0 +1,65 @@ +/* + * 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.gradle.doc + +import org.elasticsearch.gradle.test.RestTestPlugin +import org.gradle.api.Project +import org.gradle.api.Task + +/** + * Sets up tests for documentation. + */ +public class DocsTestPlugin extends RestTestPlugin { + + @Override + public void apply(Project project) { + super.apply(project) + Task listSnippets = project.tasks.create('listSnippets', SnippetsTask) + listSnippets.group 'Docs' + listSnippets.description 'List each snippet' + listSnippets.perSnippet { println(it.toString()) } + + Task listConsoleCandidates = project.tasks.create( + 'listConsoleCandidates', SnippetsTask) + listConsoleCandidates.group 'Docs' + listConsoleCandidates.description + 'List snippets that probably should be marked // CONSOLE' + listConsoleCandidates.perSnippet { + if ( + it.autoSense // Already marked, nothing to do + || it.testResponse // It is a response + ) { + return + } + List languages = [ + // These languages should almost always be marked autosense + 'js', 'json', + // These are often curl commands that should be converted but + // are probably false positives + 'sh', 'shell', + ] + if (false == languages.contains(it.language)) { + return + } + println(it.toString()) + } + + project.tasks.create('buildRestTests', RestTestsFromSnippetsTask) + } +} diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/RestTestsFromSnippetsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy similarity index 98% rename from buildSrc/src/main/groovy/org/elasticsearch/gradle/RestTestsFromSnippetsTask.groovy rename to buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy index 1ce8adf1ba4..2696f9ce661 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/RestTestsFromSnippetsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy @@ -17,9 +17,9 @@ * under the License. */ -package org.elasticsearch.gradle +package org.elasticsearch.gradle.doc -import org.elasticsearch.gradle.SnippetsTask.Snippet +import org.elasticsearch.gradle.doc.SnippetsTask.Snippet import org.gradle.api.InvalidUserDataException import org.gradle.api.tasks.Input import org.gradle.api.tasks.OutputDirectory diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/SnippetsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/SnippetsTask.groovy similarity index 97% rename from buildSrc/src/main/groovy/org/elasticsearch/gradle/SnippetsTask.groovy rename to buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/SnippetsTask.groovy index a2e6502690f..954505e65f2 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/SnippetsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/SnippetsTask.groovy @@ -17,7 +17,7 @@ * under the License. */ -package org.elasticsearch.gradle +package org.elasticsearch.gradle.doc import org.gradle.api.DefaultTask import org.gradle.api.InvalidUserDataException @@ -55,6 +55,8 @@ public class SnippetsTask extends DefaultTask { ConfigurableFileTree docs = project.fileTree(project.projectDir) { // No snippets in the build file exclude 'build.gradle' + // That is where the snippets go, not where they come from! + exclude 'build' } @TaskAction @@ -166,7 +168,9 @@ public class SnippetsTask extends DefaultTask { } snippet.testResponse = true if (matcher.group(2) != null) { - substitutions = [] + if (substitutions == null) { + substitutions = [] + } String loc = "$file:$lineNumber" parse(loc, matcher.group(2), /$SUBSTITUTION ?/) { substitutions.add([it.group(1), it.group(2)]) diff --git a/buildSrc/src/main/resources/META-INF/gradle-plugins/elasticsearch.docs-test.properties b/buildSrc/src/main/resources/META-INF/gradle-plugins/elasticsearch.docs-test.properties new file mode 100644 index 00000000000..fb264ff4fd0 --- /dev/null +++ b/buildSrc/src/main/resources/META-INF/gradle-plugins/elasticsearch.docs-test.properties @@ -0,0 +1,20 @@ +# +# 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. +# + +implementation-class=org.elasticsearch.gradle.doc.DocsTestPlugin diff --git a/docs/build.gradle b/docs/build.gradle index 3858ec1f9fb..f80ef1e1fd0 100644 --- a/docs/build.gradle +++ b/docs/build.gradle @@ -17,50 +17,27 @@ * under the License. */ -import org.elasticsearch.gradle.SnippetsTask -import org.elasticsearch.gradle.SnippetsTask.Snippet -import org.elasticsearch.gradle.RestTestsFromSnippetsTask +apply plugin: 'elasticsearch.docs-test' -apply plugin: 'elasticsearch.rest-test' - -task listSnippets(type: SnippetsTask) { - group 'Docs' - description 'List each snippet' - perSnippet { println(it) } -} - -task listAutoSenseCandidates(type: SnippetsTask) { - group 'Docs' - description 'List snippets that probably should be marked // CONSOLE' - perSnippet { - if ( - it.autoSense // Already marked, nothing to do - || it.testResponse // Only commands are autosense - ) { - return - } - List languages = [ - 'js', 'json', // These languages should almost always be marked autosense - 'sh', 'shell', // These are often curl commands that should be converted - ] - if (false == languages.contains(it.language)) { - return - } - println(it) +integTest { + cluster { + setting 'script.inline', 'true' } } -task buildRestTests(type: RestTestsFromSnippetsTask) { - docs = fileTree(project.projectDir) { - // No snippets in here! - exclude 'build.gradle' - // Remove plugins because they aren't installed during this test. Yet? - exclude 'plugins' - // This file simply doesn't pass yet. We should figure out how to fix it. - exclude 'reference/modules/snapshots.asciidoc' - } - Closure setupTwitter = { String name, int count -> - setups[name] = ''' +buildRestTests.docs = fileTree(projectDir) { + // No snippets in here! + exclude 'build.gradle' + // That is where the snippets go, not where they come from! + exclude 'build' + // Remove plugins because they aren't installed during this test. Yet? + exclude 'plugins' + // This file simply doesn't pass yet. We should figure out how to fix it. + exclude 'reference/modules/snapshots.asciidoc' +} + +Closure setupTwitter = { String name, int count -> + buildRestTests.setups[name] = ''' - do: bulk: index: twitter @@ -68,17 +45,10 @@ task buildRestTests(type: RestTestsFromSnippetsTask) { refresh: true body: |''' for (int i = 0; i < count; i++) { - setups[name] += """ + buildRestTests.setups[name] += """ {"index":{}} {"msg": "some message with the number $i", "date": $i}""" } } - setupTwitter('twitter', 5) - setupTwitter('big_twitter', 120) -} - -integTest { - cluster { - setting 'script.inline', 'true' - } -} +setupTwitter('twitter', 5) +setupTwitter('big_twitter', 120) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java index fbc518b136d..ab2c1d2afbe 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java @@ -249,6 +249,10 @@ public abstract class ESRestTestCase extends ESTestCase { adminExecutionContext = new RestTestExecutionContext(restSpec); } + protected RestTestExecutionContext getAdminExecutionContext() { + return adminExecutionContext; + } + private static void validateSpec(RestSpec restSpec) { boolean validateSpec = RandomizedTest.systemPropertyAsBoolean(REST_TESTS_VALIDATE_SPEC, true); if (validateSpec) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/section/ResponseBodyAssertion.java b/test/framework/src/main/java/org/elasticsearch/test/rest/section/ResponseBodyAssertion.java index 4a878a77323..3ead65a2111 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/section/ResponseBodyAssertion.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/section/ResponseBodyAssertion.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.TreeMap; import org.elasticsearch.common.Nullable; @@ -69,7 +70,7 @@ public class ResponseBodyAssertion extends Assertion { actual = new TreeMap<>(actual); expected = new TreeMap<>(expected); for (Map.Entry expectedEntry : expected.entrySet()) { - compare(expectedEntry.getKey(), expectedEntry.getValue(), actual.remove(expectedEntry.getKey())); + compare(expectedEntry.getKey(), actual.remove(expectedEntry.getKey()), expectedEntry.getValue()); } for (Map.Entry unmatchedEntry : actual.entrySet()) { field(unmatchedEntry.getKey(), "unexpected but found [" + unmatchedEntry.getValue() + "]"); @@ -80,6 +81,7 @@ public class ResponseBodyAssertion extends Assertion { int i = 0; while (i < actual.size() && i < expected.size()) { compare(i, actual.get(i), expected.get(i)); + i++; } if (actual.size() == expected.size()) { return; @@ -92,7 +94,7 @@ public class ResponseBodyAssertion extends Assertion { message.append("received [").append(actual.size() - i).append("] more entries than expected\n"); } - private void compare(Object field, Object expected, @Nullable Object actual) { + private void compare(Object field, @Nullable Object actual, Object expected) { if (expected instanceof Map) { if (actual == null) { field(field, "expected map but not found"); @@ -108,10 +110,11 @@ public class ResponseBodyAssertion extends Assertion { Map actualMap = (Map) actual; if (expectedMap.isEmpty() && actualMap.isEmpty()) { field(field, "same [empty map]"); + return; } field(field, null); indent += 1; - compareMaps(expectedMap, actualMap); + compareMaps(actualMap, expectedMap); indent -= 1; return; } @@ -134,7 +137,7 @@ public class ResponseBodyAssertion extends Assertion { } field(field, null); indent += 1; - compareLists(expectedList, actualList); + compareLists(actualList, expectedList); indent -= 1; return; } @@ -142,7 +145,7 @@ public class ResponseBodyAssertion extends Assertion { field(field, "expected [" + expected + "] but not found"); return; } - if (expected.equals(actual)) { + if (Objects.equals(expected, actual)) { field(field, "same [" + expected + "]"); return; }