-
Notifications
You must be signed in to change notification settings - Fork 62
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
Update TermsQuery
and TermsSetQuery and add tests
#546
Conversation
Changes AnalysisCommit SHA: 7cb604a API ChangesSummary
ReportThe full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/10688023181/artifacts/1887360554 API Coverage
|
e60e61b
to
d1d45b1
Compare
d1d45b1
to
30f1846
Compare
Spec Test Coverage Analysis
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
I think we should rename and organize the .yaml test filenames into folders consistently with the keywords used.
tests/default/_core/search/terms_query_field.yaml
is aquery
withterms
, sotests/default/_core/search/query/terms.yaml
terms_query_multiple
is a variation of the first one, so you should combine themterms_set_query
is a query withterms_set
, so it becomesquery/terms_set.yaml
Generally we're not testing OpenSearch, just the schema. No need to test response bodies.
Finally, we also plan to use these tests to auto-generate documentation, so consistency becomes important (too many docs are all over the place when it comes to kinds of examples) - try to simplify these tests as much as possible. Try only using our canoical movies
and games
examples used elsewhere instead of introducing new types of things (classes, students).
I have some more nitpicky comments, see which ones make sense to you.
payload: | ||
name: John Doe | ||
student_id: '333' | ||
status: [201] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of this can be replaced with a much shorter version with _bulk
. Look for it in other tests.
afd79fb
to
4ac4c58
Compare
Thanks @dblock I have updated the PR addressing your comments, please take a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny nit of ,<space>
.
4ac4c58
to
762db01
Compare
Signed-off-by: Prudhvi Godithi <[email protected]>
762db01
to
7cb604a
Compare
Updated @dblock, please check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏
If you're looking for more work @prudhvigodithi, #548 looks like it's a bit more involved, might also need doc fixes. |
Sure @dblock. |
Description
Terms
.TermsQuery
andTermsSetQuery
to use theTerms
schema.TermsQuery
andTermsSetQuery
.Issues Resolved
Part of #540
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.