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

Rest Tests - Support for matching objects in lists #34693

Merged
merged 2 commits into from
Nov 1, 2018

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Oct 22, 2018

Makes it possible to have a key that points to a list and assert that a
certain object is present in the list. All keys have to be present and
values have to match. The objects in the source list may have additional
fields.

example:

  contains:  { 'nodes.$master.plugins': { name: ingest-attachment }  }

Backported from #30874 (e8b8d11)

Co-authored-by: Alpar Torok torokalpar@gmail.com

Makes it possible to have a key that points to a list and assert that a
certain object is present in the list. All keys have to be present and
values have to match. The objects in the source list may have additional
fields.

example:
```
  contains:  { 'nodes.$master.plugins': { name: ingest-attachment }  }
```

Backported from elastic#30874 (e8b8d11)

Co-authored-by: Alpar Torok <torokalpar@gmail.com>
@tvernum tvernum added :Delivery/Build Build or test infrastructure v6.5.0 labels Oct 22, 2018
@javanna
Copy link
Member

javanna commented Oct 22, 2018

where do we use this new assertion? I think that the clients team need to be aware as well, or we may end up writing tests that only work when run by the java runner. Otherwise there's a way to declare a feature requirement in a test, so that it is skipped when run by a runner that does not support such feature(s).

@tvernum
Copy link
Contributor Author

tvernum commented Oct 23, 2018

Apologies, this got raised during GitHub's issues yesterday & I couldn't add explanation comments or reviewers.

This was added to master in #30874.
It was originally used in a number of "10_basic.yml" tests to check that environments were correctly configured (plugin installed, etc), and I recent used it in #33928 because it was available, and I didn't realise it was new (I just figured I'd never noticed it before).

The main benefit it brings is being able to test output where there is an array with unpredictable order.
It can also be used to check arrays that might contain additional elements that the test doesn't care about (and therefore shouldn't be dependent on).

To be honest, I wasn't aware that we even had other runners for the rest tests, this is one of those areas where it can be hard to uncover the history and contraints.

@javanna
Copy link
Member

javanna commented Oct 23, 2018

Thanks for the explanation @tvernum . I think that the original change in master didn't get the proper visibility as it was part of a much bigger PR with a title that did not suggest adding a new assertion to the yaml tests. Every language client runs the yaml tests, and each language has its own runner. When adding new assertion, the clients team needs to be notified and we use a skip feature section to signal that updated tests rely on some feature that not all runners support. This gives time for the other runners to adapt to the changes, without breaking their builds. I am not sure if this has already been noticed by the clients team, but in any case we should add skip sections to the modified tests in master. cc @atorok ping me for review there ;)

@alpar-t
Copy link
Contributor

alpar-t commented Oct 23, 2018

Thanks for bringing this up @javanna I was also unaware of the implications when I made that change.

@tvernum thanks for the back-port LGTM.

@javanna
Copy link
Member

javanna commented Oct 31, 2018

@tvernum are there issues with backporting this? There's a couple of changes that are pending backport and waiting for this one to be backported ;)

@tvernum
Copy link
Contributor Author

tvernum commented Nov 1, 2018

Sorry @javanna I got distracted after the original concerns about a new feature, and didn't manage to get back to it.

@tvernum tvernum merged commit 487b540 into elastic:6.x Nov 1, 2018
@javanna
Copy link
Member

javanna commented Nov 1, 2018

no worries, thank you ;)

@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure >non-issue Team:Delivery Meta label for Delivery team v6.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants