Convert all the docs to // CONSOLE

[nik9000]

Describe the feature:
#18075 will merge soon, testing all the // CONSOLE snippets in our docs. But lots of our docs aren't annotated with it! We should annotate more snippets so we test more of the docs.

Edit: // CONSOLE has replaced // AUTOSENSE. The sense application is named console in 5.0.0.

[nik9000]

Running gradle listConsoleCandidates will spit out a list of snippets that maybe should be converted to // AUTOSENSE. I imagine it has a very very high false positive rate. If someone is so included, they can run it and convert what they can.

If we can get the false positive rate down to 0 then we can fail the build if this finds any snippets. if.

[geekpete]

Hi nik9000, do you have any easy guide on how to assist with converting?

[nik9000]

The simplest thing to convert to // AUTOSENSE is curl commands because they are already complete commands.

1. Remove the curl -X part. Sense doesn't need it.
2. Remove the localhost:9200/ part. Sense has the port and host configured.
3. Remove the -d'{ and put the opening { on the next line. Sense likes it there and the -d' is implied.
4. Remove the closing '.
5. If the source tag at the top of the snippet is [source,sh] or something switch it to [source,js].

Now that should just work with // AUTOSENSE. You can read docs/README.md for any of the extra test markup. Stuff like // TESTRESPONSE for matching the response and // TEST[continued] for continuing on from the last snippet.

You can test your changes by running gradle :docs:check. The first time you do it will take a while, just like any java build. The list of things it has to do before it can start and skips if they are already done is immense. From downloading dependencies to compiling Elasticsearch to transforming the .asciidoc files into tests.

There are places where we didn't write the whole API call but I think those are easier once you know the sense syntax.

[clintongormley]

* What went wrong:
Execution failed for task ':docs:buildRestTests'.
> Path shouldn't start with a '/': reference/search.asciidoc[20:27](js)// AUTOSENSE
  POST /twitter/tweet?routing=kimchy
  {
    "user": "kimchy",
    "postDate": "2009-11-15T14:12:12",
    "message": "trying out Elasticsearch"
  }

It's perfectly fine for the path to start with a /. In fact, I prefer it because it looks like a real path

[clintongormley]

Remove the curl -X part. Sense doesn't need it.
Remove the localhost:9200/ part. Sense has the port and host configured.
Remove the -d'{ and put the opening { on the next line. Sense likes it there and the -d' is implied.
Remove the closing '.
If the source tag at the top of the snippet is [source,sh] or something switch it to [source,js].

Or paste it into Sense, which should convert it to Sense syntax. Then press Ctrl-I or Cmd-I to reformat.

[nik9000]

It's perfectly fine for the path to start with a /. In fact, I prefer it because it looks like a real path

I didn't like their being two ways to do it and it looked like more than half of the docs did it sans-/ so I picked that way. No one seemed to complain but maybe no one noticed?

If you really want to have the inconsistency back I can support that too.

Or paste it into Sense, which should convert it to Sense syntax. Then press Ctrl-I or Cmd-I to reformat.

The great irony of this is that I never use sense. I tried this morning and, of course, it didn't like that I was running on Elasticsearch's master branch and I hadn't built it from source. I don't have the patience nor the skill with javascript to run that from master too, I think.

[clintongormley]

If you really want to have the inconsistency back I can support that too.

If anything, I'd prefer the other way. But I'm OK with the inconsistency

[nik9000]

@clintongormley I created #18184 for it.

[MaineC]

@nik9000 How deep do we want those tests to be? TESTRESPONSE seems to be present only in a couple of documents.

Concrete example: For the query dsl docs - should there be more than just the AUTOSENSE annotation to include them in the test run so parse failures are caught?

Another questions: Should we/ do we barf in case there's deprecated stuff in those snippets?

[nik9000]

@nik9000 How deep do we want those tests to be? TESTRESPONSE seems to be present only in a couple of documents.

I don't know. Just trying to run the query at all is a huge improvement over what we have. As a first pass I'd just // AUTOSENSEify all of them.

If we want to go deeper we can use the // TEST[setup:name_of_data_set] markup to reference a dataset defined in docs/build.gradle and have that run before the snippet. Then you can make a // TESTRESPONSE.

In my first pass I was just trying to convert what we had, inventing syntax along the way. I just tried to pick up all the // AUTOSENSE stuff and do sensible things with the snippets that looked like responses. I don't know what the final state should be.

Another questions: Should we/ do we barf in case there's deprecated stuff in those snippets?

Yes we should. No we don't.

I don't know that Elasticsearch has a parameter to reject deprecated stuff. If not we should invent a parameter and make a big deal out of it because it seems really useful. And all the snippets should send that parameter by default. I think some snippets will talk about deprecated syntax and we can make // TEST[deprecated] or something to allow the snippet to send deprecated syntax.

[MaineC]

I don't know. Just trying to run the query at all is a huge improvement over what we have. As a
first pass I'd just // AUTOSENSEify all of them.

That's what I'm currently doing for the query-dsl docs (went through them about half a year ago already when switching the parsing code to ParseField, the more complicated json snippets ended up as *fromJSON tests in the *QueryBuilder Java tests back then, so I should recognize most of the issues I run into while going along)

I don't know that Elasticsearch has a parameter to reject deprecated stuff.

For parsing queries through ParseField there is an option called "index.query.parse.strict" that will fail to parse deprecated json parameters with an exception.

[nik9000]

For parsing queries through ParseField there is an option called "index.query.parse.strict" that will fail to parse deprecated json parameters with an exception.

Cool! Is that at the index level? I'd love something at the request level for this though I haven't looked at how hard it'd be to implement. And it'd be great if it were global including stuff like detecting deprecated mappings and stuff. It is the kind of thing we could have a lively discussion with @clintongormley about, I think.

[MaineC]

#17512 is related to this setting.

[nik9000]

Lol, I should remember my own issues.

[nik9000]

@clintongormley, you mentioned to me that you planned to rewrite some areas of the docs soon and would get to this as part of the rewrite. Like #18356. Anyway, do you have a list of places you plan to rewrite or a list of places you know you won't rewrite?

[clintongormley]

Anyway, do you have a list of places you plan to rewrite or a list of places you know you won't rewrite?

Mappings I won't rewrite (or only a little). Nor cluster and index settings. Analysis is in progress. That's about it - other docs I'll redo as I have the time

[MaineC]

Am I the only one having trouble to run gradle :docs:check even on master at the moment?

Running into a

==> Test Info: seed=FA5BECA4868DAF99; jvm=1; suite=1
Suite: org.elasticsearch.smoketest.SmokeTestDocsIT
ERROR   0.01s | SmokeTestDocsIT.initializationError <<< FAILURES!
   > Throwable #1: java.lang.IllegalArgumentException: Negative position
   >    at sun.nio.ch.FileChannelImpl.read(FileChannelImpl.java:717)
   >    at org.elasticsearch.test.rest.parser.RestTestSuiteParser.parse(RestTestSuiteParser.java:54)
   >    at org.elasticsearch.test.rest.ESRestTestCase.collectTestCandidates(ESRestTestCase.java:171)
   >    at org.elasticsearch.test.rest.ESRestTestCase.createParameters(ESRestTestCase.java:148)
   >    at org.elasticsearch.smoketest.SmokeTestDocsIT.parameters(SmokeTestDocsIT.java:40)
   >    at java.lang.reflect.Constructor.newInstance(Constructor.java:422)
Completed [1/1] in 0.01s, 1 test, 1 error <<< FAILURES!

[nik9000]

Clint had that a while back and needed to gradle docs:clean though I
don't know what caused it.
On May 17, 2016 6:50 AM, "Isabel Drost-Fromm" notifications@github.com
wrote:

Am I the only one having trouble to run gradle :docs:check even on master
at the moment?

Running into a

==> Test Info: seed=FA5BECA4868DAF99; jvm=1; suite=1
Suite: org.elasticsearch.smoketest.SmokeTestDocsIT
ERROR 0.01s | SmokeTestDocsIT.initializationError <<< FAILURES!

Throwable #1: java.lang.IllegalArgumentException: Negative position
at sun.nio.ch.FileChannelImpl.read(FileChannelImpl.java:717)
at org.elasticsearch.test.rest.parser.RestTestSuiteParser.parse(RestTestSuiteParser.java:54)
at org.elasticsearch.test.rest.ESRestTestCase.collectTestCandidates(ESRestTestCase.java:171)
at org.elasticsearch.test.rest.ESRestTestCase.createParameters(ESRestTestCase.java:148)
at org.elasticsearch.smoketest.SmokeTestDocsIT.parameters(SmokeTestDocsIT.java:40)
at java.lang.reflect.Constructor.newInstance(Constructor.java:422)
Completed [1/1] in 0.01s, 1 test, 1 error <<< FAILURES!

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#18160 (comment)

[MaineC]

That did the trick for me. Thanks.

Another question: Is it possible to limit test execution to just one doc file? That might speed up finding typos a bit when converting doc files with several snippets which look easy enough to not require trying each one of them separately.

[nik9000]

@tlrx Finished this in 643eb28. Thanks everyone doing all this work! 🍰

[geekpete]

Amazing effort! Awesome.

…

On 22 Oct 2017 10:30 a.m., "Nik Everett" ***@***.***> wrote: @tlrx <https://github.com/tlrx> Finished this in 643eb28 <643eb28>. Thanks everyone doing all this work! 🍰 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#18160 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB-ZO7XdtGmbsYsqs2M3NInv6-vGKEgXks5suoyzgaJpZM4IYP5M> .