elastic / elasticsearch

Free and Open, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
68.65k stars 24.38k forks source link

Testing documentation examples that require time to process their results #51572

Open lcawl opened 4 years ago

lcawl commented 4 years ago

Currently the examples in the Elasticsearch documentation are tested using the functionality described here: https://github.com/elastic/elasticsearch/tree/master/docs#snippet-testing

Unfortunately, some of the examples contain APIs that take time to generate results (e.g. machine learning analytics, forecasts, etc). As a result, the code tests fail too soon or hit race conditions depending on the speed of the machine. As a result, we have disabled testing for many examples, particularly in the machine learning documentation.

Ideally, we should run tests against a system that has had time to set up and process data so that we get meaningful and consistent results. Alternatively, we should add the ability to wait for a condition before testing the results.

NOTE: Some APIs have wait_for_xxx parameters (e.g. https://www.elastic.co/guide/en/elasticsearch/reference/7.5/docs-update.html#update-api-desc), but certainly not all of them.

elasticmachine commented 4 years ago

Pinging @elastic/es-docs (>docs)

elasticmachine commented 4 years ago

Pinging @elastic/es-core-infra (:Core/Infra/Build)

mark-vieira commented 4 years ago

Shout out to the @elastic/machine-learning folks. How do we handle this for the ML YAML tests? Certainly we have some mechanism in our test infrastructure to wait for asynchronous processing to finish.

davidkyle commented 4 years ago

Consider a transform, the test would create and start a transform to do some work on data indexed during the test setup. The transform does its work and when it is finished we will assert on the output and the transforms final state. There is no wait_for_completion parameter to starting the transform so we have to poll to find out when the transform is complete and as you know there is no polling in YAML tests so the answer is we use Rest Tests instead of YAML tests and call assertBusy to wait for asynchronous processing to finish.

If we has assertBusy and the ability to retry a call then we could wait for a condition that signals the processing is finished as we do in the rest tests.

  - do: 
    transform.start:
      id : foo

# call this repeatedly until the timeout is reached or status == finished
  - assertBusy:
    timeout: 10s
  - do:
    transforms.getstatus:
      status: finished
droberts195 commented 4 years ago

@mark-vieira what @davidkyle says is one aspect of how we could make more interesting ML stories work in the docs tests. But before implementing that it's important to consider the wider picture.

Just over two years ago #27699 proposed the same thing and was rejected primarily on the grounds that any functionality added to the YAML test framework has to be implemented by all the different client test frameworks. So if we add assertBusy to the syntax then as well as Java it also has to be implemented in Python, Perl, Rust, Go, PHP, etc...

The other argument against implementing it at that time was that the YAML tests are supposed to just test the API interactions and not be bigger tests of whether non-trivial functionality works. Those are supposed to be in Java integration tests. So the direct answer to:

How do we handle this for the ML YAML tests?

is "at the moment we don't - we do it in Java integration tests and or ML QA team tests instead".

Next consider:

so that we get meaningful and consistent results

It's nicer for the ML docs if the examples we give do not contain all zeros in responses but are more realistic examples of something that someone who was using ML in a useful production scenario would see. But this means a non-trivial and meaningful data set needs to be used. Our Java integration tests don't solve this problem - they create silly data that exercises one code path but doesn't not reflect a real-world data set.

For the real-world data sets we have a separate test framework created by the ML QA team. But these are pretty big. One problem is that if you only have 100 documents you probably don't need ML. So any test data set that will produce interesting and realistic responses needs to be bigger than what you might embed into a docs test file. That in turn also means that you wouldn't want to run ML on it multiple times during the docs tests - you'd want to run it once and keep the results around so that various docs snippets can be tested against those same results.

I think that where this is all leading is that there's a major difference between the YAML tests that are used both with the Elasticsearch tests and also within the clients tests and with the docs tests.

Maybe one aspect of the solution could be implementing assertBusy in the YAML framework but only allowing it to be used in docs tests, not client API tests. Maybe another part of the solution could be to have a realistic yet non-confidential data set somewhere that the docs tests download and use with ML. And another part could be to allow one ML job to be immune from the usual test setup/teardown so that it can be run once rather than adding considerable test setup time to many docs files.

But these are quite far-reaching decisions. I don't think there's an easy answer to this problem. (If there was then we would have just done it years ago.)

mark-vieira commented 4 years ago

Thanks for the detailed response @droberts195!

elasticsearchmachine commented 1 year ago

Pinging @elastic/es-docs (Team:Docs)

elasticsearchmachine commented 1 year ago

Pinging @elastic/es-delivery (Team:Delivery)