From 156393be0e6fef7694a7d1ba4e8f7c84b3bd2ab5 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 9 Sep 2016 11:10:13 -0400 Subject: [PATCH] Fail build if new doc snippets aren't `// CONSOLE` This tracks the snippets that probably should be converted to `// CONSOLE` or `// TESTRESPONSE` and fails the build if the list of files with such snippets doesn't match the list in `docs/build.gradle`. Setting the file looks like ``` /* List of files that have snippets that probably should be converted to * `// CONSOLE` and `// TESTRESPONSE` but have yet to be converted. Try and * only remove entries from this list. When it is empty we'll remove it * entirely and have a party! There will be cake and everything.... */ buildRestTests.expectedUnconvertedCandidates = [ 'plugins/discovery-azure-classic.asciidoc', ... 'reference/search/suggesters/completion-suggest.asciidoc', ] ``` This list is in `build.gradle` because we expect it to be fairly temporary. In a few months we'll have converted all of the docs and won't ned it any more. From now on if you add now docs that contain a snippet that shows an interaction with elasticsearch you have three choices: 1. Stick `// CONSOLE` on the interactions and `// TESTRESPONSE` on the responses. The build (specifically (`gradle docs:check`) will test that these interactions "work". If there isn't a `// TESTRESPONSE` snippet then "work" just means "Elasticsearch responds with a 200-level response code and no `WARNING` headers. This is way better than nothing. 2. Add `// NOTCONSOLE` if the snippet isn't actually interacting with Elasticsearch. This should only be required for stuff like javascript source code or `curl` against an external service like AWS or GCE. The snippet will not get "OPEN IN CONSOLE" or "COPY AS CURL" buttons or be tested. 3. Add `// TEST[skip:reason]` under the snippet. This will just skip the snippet in the test phase. This should really be reserved for snippets where we can't test them because they require an external service that we don't have at testing time. Please, please, please, please don't add more things to the list. After all, it sais there'll be cake when we remove it entirely! Relates to #18160 --- .../gradle/doc/DocsTestPlugin.groovy | 12 +- .../doc/RestTestsFromSnippetsTask.groovy | 72 ++++++++ docs/build.gradle | 165 ++++++++++++++++++ docs/reference/search.asciidoc | 14 +- 4 files changed, 248 insertions(+), 15 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/DocsTestPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/DocsTestPlugin.groovy index 0fefecc144646..11bdbd1952561 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/DocsTestPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/DocsTestPlugin.groovy @@ -53,17 +53,7 @@ public class DocsTestPlugin extends RestTestPlugin { 'List snippets that probably should be marked // CONSOLE' listConsoleCandidates.defaultSubstitutions = defaultSubstitutions listConsoleCandidates.perSnippet { - if ( - it.console != null // Already marked, nothing to do - || it.testResponse // It is a response - ) { - return - } - if ( // js almost always should be `// CONSOLE` - it.language == 'js' || - // snippets containing `curl` *probably* should - // be `// CONSOLE` - it.curl) { + if (RestTestsFromSnippetsTask.isConsoleCandidate(it)) { println(it.toString()) } } 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 dc4e6f5f70af4..a01d1d5839fe2 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTask.groovy @@ -41,6 +41,16 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { @Input Map setups = new HashMap() + /** + * A list of files that contain snippets that *probably* should be + * converted to `// CONSOLE` but have yet to be converted. If a file is in + * this list and doesn't contain unconverted snippets this task will fail. + * If there are unconverted snippets not in this list then this task will + * fail. All files are paths relative to the docs dir. + */ + @Input + List expectedUnconvertedCandidates = [] + /** * Root directory of the tests being generated. To make rest tests happy * we generate them in a testRoot() which is contained in this directory. @@ -56,6 +66,7 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { TestBuilder builder = new TestBuilder() doFirst { outputRoot().delete() } perSnippet builder.&handleSnippet + doLast builder.&checkUnconverted doLast builder.&finishLastTest } @@ -67,6 +78,27 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { return new File(testRoot, '/rest-api-spec/test') } + /** + * Is this snippet a candidate for conversion to `// CONSOLE`? + */ + static isConsoleCandidate(Snippet snippet) { + /* Snippets that are responses or already marked as `// CONSOLE` or + * `// NOTCONSOLE` are not candidates. */ + if (snippet.console != null || snippet.testResponse) { + return false + } + /* js snippets almost always should be marked with `// CONSOLE`. js + * snippets that shouldn't be marked `// CONSOLE`, like examples for + * js client, should always be marked with `// NOTCONSOLE`. + * + * `sh` snippets that contain `curl` almost always should be marked + * with `// CONSOLE`. In the exceptionally rare cases where they are + * not communicating with Elasticsearch, like the xamples in the ec2 + * and gce discovery plugins, the snippets should be marked + * `// NOTCONSOLE`. */ + return snippet.language == 'js' || snippet.curl + } + private class TestBuilder { private static final String SYNTAX = { String method = /(?GET|PUT|POST|HEAD|OPTIONS|DELETE)/ @@ -88,11 +120,21 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { */ PrintWriter current + /** + * Files containing all snippets that *probably* should be converted + * to `// CONSOLE` but have yet to be converted. All files are paths + * relative to the docs dir. + */ + Set unconvertedCandidates = new HashSet<>() + /** * Called each time a snippet is encountered. Tracks the snippets and * calls buildTest to actually build the test. */ void handleSnippet(Snippet snippet) { + if (RestTestsFromSnippetsTask.isConsoleCandidate(snippet)) { + unconvertedCandidates.add(snippet.path.toString()) + } if (BAD_LANGUAGES.contains(snippet.language)) { throw new InvalidUserDataException( "$snippet: Use `js` instead of `${snippet.language}`.") @@ -250,5 +292,35 @@ public class RestTestsFromSnippetsTask extends SnippetsTask { current = null } } + + void checkUnconverted() { + List listedButNotFound = [] + for (String listed : expectedUnconvertedCandidates) { + if (false == unconvertedCandidates.remove(listed)) { + listedButNotFound.add(listed) + } + } + String message = "" + if (false == listedButNotFound.isEmpty()) { + Collections.sort(listedButNotFound) + listedButNotFound = listedButNotFound.collect {' ' + it} + message += "Expected unconverted snippets but none found in:\n" + message += listedButNotFound.join("\n") + } + if (false == unconvertedCandidates.isEmpty()) { + List foundButNotListed = + new ArrayList<>(unconvertedCandidates) + Collections.sort(foundButNotListed) + foundButNotListed = foundButNotListed.collect {' ' + it} + if (false == "".equals(message)) { + message += "\n" + } + message += "Unexpected unconverted snippets:\n" + message += foundButNotListed.join("\n") + } + if (false == "".equals(message)) { + throw new InvalidUserDataException(message); + } + } } } diff --git a/docs/build.gradle b/docs/build.gradle index 6e2b02b026329..95d851ba2bd2a 100644 --- a/docs/build.gradle +++ b/docs/build.gradle @@ -19,6 +19,171 @@ apply plugin: 'elasticsearch.docs-test' +/* List of files that have snippets that probably should be converted to + * `// CONSOLE` and `// TESTRESPONSE` but have yet to be converted. Try and + * only remove entries from this list. When it is empty we'll remove it + * entirely and have a party! There will be cake and everything.... */ +buildRestTests.expectedUnconvertedCandidates = [ + 'plugins/discovery-azure-classic.asciidoc', + 'reference/aggregations.asciidoc', + 'reference/aggregations/bucket/children-aggregation.asciidoc', + 'reference/aggregations/bucket/datehistogram-aggregation.asciidoc', + 'reference/aggregations/bucket/daterange-aggregation.asciidoc', + 'reference/aggregations/bucket/diversified-sampler-aggregation.asciidoc', + 'reference/aggregations/bucket/filter-aggregation.asciidoc', + 'reference/aggregations/bucket/filters-aggregation.asciidoc', + 'reference/aggregations/bucket/geodistance-aggregation.asciidoc', + 'reference/aggregations/bucket/geohashgrid-aggregation.asciidoc', + 'reference/aggregations/bucket/global-aggregation.asciidoc', + 'reference/aggregations/bucket/histogram-aggregation.asciidoc', + 'reference/aggregations/bucket/iprange-aggregation.asciidoc', + 'reference/aggregations/bucket/missing-aggregation.asciidoc', + 'reference/aggregations/bucket/nested-aggregation.asciidoc', + 'reference/aggregations/bucket/range-aggregation.asciidoc', + 'reference/aggregations/bucket/reverse-nested-aggregation.asciidoc', + 'reference/aggregations/bucket/sampler-aggregation.asciidoc', + 'reference/aggregations/bucket/significantterms-aggregation.asciidoc', + 'reference/aggregations/bucket/terms-aggregation.asciidoc', + 'reference/aggregations/matrix/stats-aggregation.asciidoc', + 'reference/aggregations/metrics/avg-aggregation.asciidoc', + 'reference/aggregations/metrics/cardinality-aggregation.asciidoc', + 'reference/aggregations/metrics/extendedstats-aggregation.asciidoc', + 'reference/aggregations/metrics/geobounds-aggregation.asciidoc', + 'reference/aggregations/metrics/geocentroid-aggregation.asciidoc', + 'reference/aggregations/metrics/max-aggregation.asciidoc', + 'reference/aggregations/metrics/min-aggregation.asciidoc', + 'reference/aggregations/metrics/percentile-aggregation.asciidoc', + 'reference/aggregations/metrics/percentile-rank-aggregation.asciidoc', + 'reference/aggregations/metrics/scripted-metric-aggregation.asciidoc', + 'reference/aggregations/metrics/stats-aggregation.asciidoc', + 'reference/aggregations/metrics/sum-aggregation.asciidoc', + 'reference/aggregations/metrics/tophits-aggregation.asciidoc', + 'reference/aggregations/metrics/valuecount-aggregation.asciidoc', + 'reference/aggregations/pipeline.asciidoc', + 'reference/aggregations/pipeline/avg-bucket-aggregation.asciidoc', + 'reference/aggregations/pipeline/bucket-script-aggregation.asciidoc', + 'reference/aggregations/pipeline/bucket-selector-aggregation.asciidoc', + 'reference/aggregations/pipeline/cumulative-sum-aggregation.asciidoc', + 'reference/aggregations/pipeline/derivative-aggregation.asciidoc', + 'reference/aggregations/pipeline/extended-stats-bucket-aggregation.asciidoc', + 'reference/aggregations/pipeline/max-bucket-aggregation.asciidoc', + 'reference/aggregations/pipeline/min-bucket-aggregation.asciidoc', + 'reference/aggregations/pipeline/movavg-aggregation.asciidoc', + 'reference/aggregations/pipeline/percentiles-bucket-aggregation.asciidoc', + 'reference/aggregations/pipeline/serial-diff-aggregation.asciidoc', + 'reference/aggregations/pipeline/stats-bucket-aggregation.asciidoc', + 'reference/aggregations/pipeline/sum-bucket-aggregation.asciidoc', + 'reference/analysis/analyzers/lang-analyzer.asciidoc', + 'reference/analysis/analyzers/pattern-analyzer.asciidoc', + 'reference/analysis/charfilters/htmlstrip-charfilter.asciidoc', + 'reference/analysis/charfilters/pattern-replace-charfilter.asciidoc', + 'reference/analysis/tokenfilters/asciifolding-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/cjk-bigram-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/common-grams-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/compound-word-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/elision-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/hunspell-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/keep-types-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/keep-words-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/keyword-marker-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/keyword-repeat-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/limit-token-count-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/lowercase-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/pattern-capture-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/snowball-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/stemmer-override-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/stemmer-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/stop-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/synonym-tokenfilter.asciidoc', + 'reference/analysis/tokenfilters/word-delimiter-tokenfilter.asciidoc', + 'reference/api-conventions.asciidoc', + 'reference/cat.asciidoc', + 'reference/cat/alias.asciidoc', + 'reference/cat/allocation.asciidoc', + 'reference/cat/count.asciidoc', + 'reference/cat/fielddata.asciidoc', + 'reference/cat/health.asciidoc', + 'reference/cat/indices.asciidoc', + 'reference/cat/master.asciidoc', + 'reference/cat/nodeattrs.asciidoc', + 'reference/cat/nodes.asciidoc', + 'reference/cat/pending_tasks.asciidoc', + 'reference/cat/plugins.asciidoc', + 'reference/cat/recovery.asciidoc', + 'reference/cat/repositories.asciidoc', + 'reference/cat/segments.asciidoc', + 'reference/cat/shards.asciidoc', + 'reference/cat/snapshots.asciidoc', + 'reference/cat/thread_pool.asciidoc', + 'reference/cluster.asciidoc', + 'reference/cluster/allocation-explain.asciidoc', + 'reference/cluster/nodes-info.asciidoc', + 'reference/cluster/nodes-stats.asciidoc', + 'reference/cluster/pending.asciidoc', + 'reference/cluster/reroute.asciidoc', + 'reference/cluster/state.asciidoc', + 'reference/cluster/stats.asciidoc', + 'reference/cluster/tasks.asciidoc', + 'reference/cluster/update-settings.asciidoc', + 'reference/docs/bulk.asciidoc', + 'reference/docs/delete-by-query.asciidoc', + 'reference/docs/delete.asciidoc', + 'reference/docs/get.asciidoc', + 'reference/docs/index_.asciidoc', + 'reference/docs/multi-get.asciidoc', + 'reference/docs/multi-termvectors.asciidoc', + 'reference/docs/reindex.asciidoc', + 'reference/docs/termvectors.asciidoc', + 'reference/docs/update-by-query.asciidoc', + 'reference/docs/update.asciidoc', + 'reference/getting-started.asciidoc', + 'reference/index-modules/similarity.asciidoc', + 'reference/index-modules/store.asciidoc', + 'reference/index-modules/translog.asciidoc', + 'reference/indices/analyze.asciidoc', + 'reference/indices/flush.asciidoc', + 'reference/indices/get-field-mapping.asciidoc', + 'reference/indices/get-settings.asciidoc', + 'reference/indices/put-mapping.asciidoc', + 'reference/indices/recovery.asciidoc', + 'reference/indices/segments.asciidoc', + 'reference/indices/shadow-replicas.asciidoc', + 'reference/indices/shard-stores.asciidoc', + 'reference/indices/update-settings.asciidoc', + 'reference/indices/upgrade.asciidoc', + 'reference/ingest/ingest-node.asciidoc', + 'reference/mapping/dynamic/templates.asciidoc', + 'reference/mapping/fields/all-field.asciidoc', + 'reference/mapping/params/analyzer.asciidoc', + 'reference/mapping/types/binary.asciidoc', + 'reference/mapping/types/geo-point.asciidoc', + 'reference/mapping/types/geo-shape.asciidoc', + 'reference/mapping/types/ip.asciidoc', + 'reference/mapping/types/nested.asciidoc', + 'reference/mapping/types/object.asciidoc', + 'reference/mapping/types/percolator.asciidoc', + 'reference/modules/cluster/misc.asciidoc', + 'reference/modules/indices/request_cache.asciidoc', + 'reference/modules/scripting/native.asciidoc', + 'reference/modules/scripting/security.asciidoc', + 'reference/modules/scripting/using.asciidoc', + 'reference/modules/transport.asciidoc', + 'reference/query-dsl/exists-query.asciidoc', + 'reference/query-dsl/function-score-query.asciidoc', + 'reference/query-dsl/geo-shape-query.asciidoc', + 'reference/query-dsl/terms-query.asciidoc', + 'reference/redirects.asciidoc', + 'reference/search/field-stats.asciidoc', + 'reference/search/multi-search.asciidoc', + 'reference/search/profile.asciidoc', + 'reference/search/request/highlighting.asciidoc', + 'reference/search/request/inner-hits.asciidoc', + 'reference/search/request/rescore.asciidoc', + 'reference/search/request/scroll.asciidoc', + 'reference/search/search-template.asciidoc', + 'reference/search/suggesters/completion-suggest.asciidoc', +] + integTest { cluster { setting 'script.inline', 'true' diff --git a/docs/reference/search.asciidoc b/docs/reference/search.asciidoc index a4de20ee21392..61d807cc21237 100644 --- a/docs/reference/search.asciidoc +++ b/docs/reference/search.asciidoc @@ -18,13 +18,14 @@ when indexing tweets, the routing value can be the user name: [source,js] -------------------------------------------------- -$ curl -XPOST 'http://localhost:9200/twitter/tweet?routing=kimchy' -d '{ +POST /twitter/tweet?routing=kimchy +{ "user" : "kimchy", "postDate" : "2009-11-15T14:12:12", "message" : "trying out Elasticsearch" } -' -------------------------------------------------- +// CONSOLE In such a case, if we want to search only on the tweets for a specific user, we can specify it as the routing, resulting in the search hitting @@ -32,7 +33,8 @@ only the relevant shard: [source,js] -------------------------------------------------- -$ curl -XGET 'http://localhost:9200/twitter/tweet/_search?routing=kimchy' -d '{ +POST /twitter/tweet/_search?routing=kimchy +{ "query": { "bool" : { "must" : { @@ -46,8 +48,9 @@ $ curl -XGET 'http://localhost:9200/twitter/tweet/_search?routing=kimchy' -d '{ } } } -' -------------------------------------------------- +// CONSOLE +// TEST[continued] The routing parameter can be multi valued represented as a comma separated string. This will result in hitting the relevant shards where @@ -65,6 +68,7 @@ the request with two different groups: [source,js] -------------------------------------------------- +POST /_search { "query" : { "match_all" : {} @@ -72,6 +76,8 @@ the request with two different groups: "stats" : ["group1", "group2"] } -------------------------------------------------- +// CONSOLE +// TEST[setup:twitter] [float] [[global-search-timeout]]