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

Tests for indices.put_template use illegal action #25694

Closed
honzakral opened this issue Jul 12, 2017 · 7 comments
Closed

Tests for indices.put_template use illegal action #25694

honzakral opened this issue Jul 12, 2017 · 7 comments
Assignees
Labels
>test Issues or PRs that are addressing/adding tests

Comments

@honzakral
Copy link
Contributor

https://github.com/elastic/elasticsearch/blob/master/rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_template/10_basic.yml#L221-L225 uses a directive raw which is not documented in the test specifications (https://github.com/elastic/elasticsearch/blob/master/rest-api-spec/src/main/resources/rest-api-spec/test/README.asciidoc) and it is not marked with a skip.feature to ensure forwards compatibility of all the clients.

My suggestion is to remove the test since it is anyway something that seems to belong more in a Java-based test - typically the REST tests serve to check the declared API (not raw requests) and that checks the required parameters in the client side so this test would never be possible.

This affects all versions >= 5.5.0

@honzakral honzakral added the >test Issues or PRs that are addressing/adding tests label Jul 12, 2017
@rjernst
Copy link
Member

rjernst commented Jul 12, 2017

The entire put_template api is deprecated in 5.6 (#25437) and will be removed for 6.0. Is it alright to just have the test go away in master, or is it necessary for the client tests to work with 5.x (can they just blacklist that test?)?

@honzakral
Copy link
Contributor Author

In that case the easiest would be to annotate that test with:

     - skip:
          features:    raw

so that the clients, that haven't implemented the raw feature will automatically skip it.

Thanks!

@javanna
Copy link
Member

javanna commented Jul 13, 2017

Seems like we have these raw requests in some other tests. They come from 5463294 . Raw is something that should only be used for docs tests, I wouldn't introduce it in yaml tests. I think these tests shouldn't be yaml tests, we should migrate them to java tests that extend ESRestTestCase instead.

@nik9000
Copy link
Member

nik9000 commented Jul 13, 2017

I imagine we can only enable raw for the docs which would prevent this from happening again.

@javanna
Copy link
Member

javanna commented Jul 13, 2017

@nik9000 that would be nice, I think we have to get used to a different way of writing these tests. We can use java now, we don't have to use yaml all the time :)

@karmi
Copy link
Contributor

karmi commented Jul 19, 2017

is it necessary for the client tests to work with 5.x

Yes, we need to run the tests on the 5.x branch as well, so either let's rewrite the test to not use the raw action, or annotate it with skip correctly, as @honzakral suggests.

@javanna
Copy link
Member

javanna commented Jul 19, 2017

I plan on rewriting these tests in java, I just need to get to this... next week hopefully?

@javanna javanna self-assigned this Aug 3, 2017
javanna added a commit to javanna/elasticsearch that referenced this issue Aug 7, 2017
Raw requests are supported only by the java yaml test runner and were introduced to test docs snippets. Some yaml tests ended up using them (see elastic#23497) which causes failures for other language clients. This commit migrates those yaml tests to Java tests that send requests through the Java low-level REST client, and also moves the ability to send raw requests to a special client that's only available when testing docs snippets.

Closes elastic#25694
javanna added a commit that referenced this issue Aug 7, 2017
Raw requests are supported only by the java yaml test runner and were introduced to test docs snippets. Some yaml tests ended up using them (see #23497) which causes failures for other language clients. This commit migrates those yaml tests to Java tests that send requests through the Java low-level REST client, and also moves the ability to send raw requests to a special client that's only available when testing docs snippets.

Closes #25694
javanna added a commit that referenced this issue Aug 7, 2017
Raw requests are supported only by the java yaml test runner and were introduced to test docs snippets. Some yaml tests ended up using them (see #23497) which causes failures for other language clients. This commit migrates those yaml tests to Java tests that send requests through the Java low-level REST client, and also moves the ability to send raw requests to a special client that's only available when testing docs snippets.

Closes #25694
javanna added a commit that referenced this issue Aug 7, 2017
Raw requests are supported only by the java yaml test runner and were introduced to test docs snippets. Some yaml tests ended up using them (see #23497) which causes failures for other language clients. This commit migrates those yaml tests to Java tests that send requests through the Java low-level REST client, and also moves the ability to send raw requests to a special client that's only available when testing docs snippets.

Closes #25694
javanna added a commit that referenced this issue Aug 7, 2017
Raw requests are supported only by the java yaml test runner and were introduced to test docs snippets. Some yaml tests ended up using them (see #23497) which causes failures for other language clients. This commit migrates those yaml tests to Java tests that send requests through the Java low-level REST client, and also moves the ability to send raw requests to a special client that's only available when testing docs snippets.

Closes #25694
javanna added a commit that referenced this issue Aug 7, 2017
Raw requests are supported only by the java yaml test runner and were introduced to test docs snippets. Some yaml tests ended up using them (see #23497) which causes failures for other language clients. This commit migrates those yaml tests to Java tests that send requests through the Java low-level REST client, and also moves the ability to send raw requests to a special client that's only available when testing docs snippets.

Closes #25694
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>test Issues or PRs that are addressing/adding tests
Projects
None yet
Development

No branches or pull requests

5 participants