Cross cluster search integration tests #2178
Conversation
|
|
||
| static final String GENRE_ROCK = "rock"; | ||
| static final String GENRE_JAZZ = "jazz"; | ||
| static final String GENRE_BLUES = "blues"; | ||
|
|
||
| static final String QUERY_TITLE_NEXT_SONG = FIELD_TITLE + ":" + "\"" + TITLE_NEXT_SONG + "\""; | ||
| static final String QUERY_TITLE_POISON = FIELD_TITLE + ":" + TITLE_POISON; | ||
| static final String QUERY_TITLE_MAGNUM_OPUS = FIELD_TITLE + ":" + TITLE_MAGNUM_OPUS; | ||
|
|
||
| static final Object[][] SONGS = { |
There was a problem hiding this comment.
This is complex and a small error could blow up many tests for an unknown reason, what do you think about introducing a type representing a song scheme to ensure consistent construction?
There was a problem hiding this comment.
You are right. This data structure is getting more and more complex. I will create a DTO class which contains fields like artist, title, etc. and will serialize the class to JSON with a Jackson library.
There was a problem hiding this comment.
Finally, I manage to do this without Jackson.
src/integrationTest/java/org/opensearch/test/framework/cluster/ClusterManager.java
Outdated
Show resolved
Hide resolved
| @@ -26,14 +26,21 @@ public final class SearchRequestFactory { | |||
| private SearchRequestFactory() { | |||
|
|
|||
| } | |||
| public static SearchRequest queryStringQueryRequest(String indexName, String queryString) { | |||
| public static SearchRequest queryStringQueryRequest(String indexName, String queryString, Boolean ccsMinimizeRoundtrips) { | |||
There was a problem hiding this comment.
The SearchRequest is mutable, these additional overloads seem like they are going to be more maintenance work as additional scenarios are added over the long run. What would you think about creating a 'builder' function like minimizeRoundTrips(...) that can be used inline to add these parameter?
public static SearchRequest minimizeRoundTrips(SearchRequest sr) {
sr.setCcsMinimizeRoundtrips(true);
return sr;
}There was a problem hiding this comment.
done
src/integrationTest/java/org/opensearch/security/CrossClusterSearchTests.java
Show resolved
Hide resolved
src/integrationTest/java/org/opensearch/security/CrossClusterSearchTests.java
Show resolved
Hide resolved
lukasz-soszynski-eliatra
force-pushed
the
cross-cluster-search-tests
branch
from
5d4f915
to
ad420f3
Compare
Signed-off-by: Lukasz Soszynski <lukasz.soszynski@eliatra.com>
lukasz-soszynski-eliatra
force-pushed
the
cross-cluster-search-tests
branch
from
ad420f3
to
90ba4f2
Compare
There was a problem hiding this comment.
Great work @lukasz-soszynski-eliatra ! I have request a small change.
| @@ -49,7 +49,7 @@ | |||
| mismatchDescription.appendText("Source document is null, is fetch source option set to true?"); | |||
| return false; | |||
| } | |||
| if(!source.containsKey(fieldName)) { | |||
| if(source.containsKey(fieldName) == false) { | |||
There was a problem hiding this comment.
nit: This change should not be needed
There was a problem hiding this comment.
Please correct me if I am wrong, but I thought that some documents define the convention to use expressions like source.containsKey(fieldName) == false instead of !source.containsKey(fieldName)
There was a problem hiding this comment.
interesting...I'm not aware about such documents.. If you have one handy could you please link it here
There was a problem hiding this comment.
There is an implicit rule that negative boolean expressions should use the form foo == false instead of !foo for better readability of the code. While this isn't strictly enforced, if might get called out in PR reviews as something to change.
Please see https://github.com/opensearch-project/OpenSearch/blob/main/DEVELOPER_GUIDE.md .
But this file is placed in the main OpenSearch repository and not in the security plugin repository. Therefore I am not sure if the rule is applicable.
src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java
Show resolved
Hide resolved
|
Bwc tests are broken on main. Tracking issue here: #2221 |
Description
[Describe what this change achieves]
Integration tests related to the cross-cluster search feature created
Issues Resolved
[List any issues this PR will resolve]
Is this a backport? If so, please add backport PR # and/or commits #
Testing
[Please provide details of testing done: unit testing, integration testing and manual testing]
Check List
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.