Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing rest specs for testing features' code snippets #48663

Open
lcawl opened this issue Oct 29, 2019 · 8 comments
Open

Missing rest specs for testing features' code snippets #48663

lcawl opened this issue Oct 29, 2019 · 8 comments
Labels
>bug :Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team v9.0.0

Comments

@lcawl
Copy link
Contributor

lcawl commented Oct 29, 2019

The docs/build.gradle that we use for code snippet testing doesn't seem to parse all the X-Pack API specs. For example, in #44018 we received:

org.elasticsearch.smoketest.DocsClientYamlTestSuiteIT > test {yaml=reference/ml/apis/delete-dfanalytics/line_38} FAILED
08:43:32 java.lang.RuntimeException: Failure at [reference/ml/apis/delete-dfanalytics:33]: rest api [ml.put_data_frame_analytics] doesn't exist in the rest spec

We have worked around this for rollups and for a handful of ML APIs by using "raw" methods in the build.gradle file. For example: #48259

Instead of implementing these work-arounds, however, it would be preferable to have all the rest specs recognized by the documentation tests.

@lcawl lcawl added the :Delivery/Build Build or test infrastructure label Oct 29, 2019
@elasticmachine
Copy link
Collaborator

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

@lcawl
Copy link
Contributor Author

lcawl commented Oct 30, 2019

Per @droberts195:

We have cleanup code both for the X-Pack REST tests [1] and the (Apache 2 license) HLRC tests [2].

The docs tests in the x-pack/docs directory already use the X-Pack ML REST test cleaner, because XDocsClientYamlTestSuiteIT extends XPackRestIT [3] and that uses the MlRestTestStateCleaner.

The only ML cleanup for the main docs directory tests is to wait for pending tasks [4]. So ironically the x-pack/docs tests are doing ML cleanup after each test (don’t worry - it should be very fast when there are no ML jobs) whereas the tests of the directory where the ML documentation actually lives are not.

One way we could solve both the cleanup problem and the spec problem would be to move all the ML docs into the x-pack/docs directory...The other way would be to either duplicate the HLRC ML cleanup class into the docs test code or put it somewhere where both sets of tests can use it...

[1] https://github.com/elastic/elasticsearch/blob/master/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/integration/MlRestTestStateCleaner.java#L19
[2] https://github.com/elastic/elasticsearch/blob/master/client/rest-high-level/src/test/java/org/elasticsearch/client/MlTestStateCleaner.java#L42
[3] https://github.com/elastic/elasticsearch/blob/master/x-pack/docs/src/test/java/org/elasticsearch/smoketest/XDocsClientYamlTestSuiteIT.java#L32
[4] https://github.com/elastic/elasticsearch/blob/master/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java#L103

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (:Docs)

@rjernst rjernst added Team:Core/Infra Meta label for core/infra team Team:Docs Meta label for docs team labels May 4, 2020
@mark-vieira mark-vieira added Team:Delivery Meta label for Delivery team and removed Team:Core/Infra Meta label for core/infra team labels Nov 11, 2020
@mark-vieira
Copy link
Contributor

@lcawl Does this continue to be a problem? I think with the change to remove the OSS distribution this simplifies things a bit WRT to testing as we no longer will have the notion of "OSS" docs.

@jakelandis on a related point, I wonder to what degree we can simplify the distinction between "oss" and "x-pack" for REST API specs in generally, given that the default distribution is now the only one that will exist 🤔

@lcawl
Copy link
Contributor Author

lcawl commented Feb 8, 2021

I haven't tried removing them lately, but I still see "raw" methods in the build.gradle, such as

@jakelandis
Copy link
Contributor

I wonder to what degree we can simplify the distinction between "oss" and "x-pack" for REST API specs in generally, given that the default distribution is now the only one that will exist

@mark-vieira - It would be great to simply move the x-pack specs (x-pack/plugin/src/test/resources/rest-api-spec/api/**) to same directory as the non-x-pack specs (rest-api-spec/src/main/resources/rest-api-spec/api). To allow things to keep working it should be as easy find all the configurations like this:

restResources {
  restApi {
    includeCore '_common', 'cluster', 'nodes', 'indices', 'index', 'info'
    includeXpack 'ccr'
  }
}

and change it to

restResources {
  restApi {
    include '_common', 'cluster', 'nodes', 'indices', 'index', 'info', `ccr`
  }
}

There are probably a couple edge cases...but that would be the primary change for ES. The restResources are essentially 2 buckets of files, and this would allow us to simply remove one of the buckets (the test buckets would remain unchanged). This would likely break the client's team since they rely on the the directory structure. Additionally the artifact that we publish would now include the x-pack specs, so not sure if there are licensing concerns there.

We also support project specific API's (specs), but I think there is only 1 project (lang-painless) that actually does that.

Instead of implementing these work-arounds, however, it would be preferable to have all the rest specs recognized by the documentation tests.

@lcawl - I think you should be able to change the configuration from:

restResources {
  restApi {
    includeCore '*'
  }
}

to

restResources {
  restApi {
    includeCore '*'
    includeXpack '*'
  }
}

To bring in all the x-pack specs into docs/build.gradle. This would only put the specs on the classpath, but it doesn't influence the test setup/cleanup.

@mark-vieira
Copy link
Contributor

@jakelandis that's pretty much what I was thinking.

@jrodewig jrodewig added >docs General docs changes and removed :Docs labels Sep 16, 2021
@arteam arteam added v8.1.0 and removed v8.0.0 labels Jan 12, 2022
@mark-vieira mark-vieira added v8.2.0 and removed v8.1.0 labels Feb 2, 2022
@lockewritesdocs lockewritesdocs removed >docs General docs changes Team:Docs Meta label for docs team labels Apr 27, 2022
@elasticsearchmachine
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team v9.0.0
Projects
None yet
Development

No branches or pull requests