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- added helper methods to ESRestTestCase for checking warnings #36443

Merged

Conversation

markharwood
Copy link
Contributor

@markharwood markharwood commented Dec 10, 2018

Helper method for checking mixed and current-version-only clusters in Java REST tests that use LLRC or HLRC apis.

Closes #36251

@markharwood markharwood added >test Issues or PRs that are addressing/adding tests WIP :Clients/Java Low Level REST Client Minimal dependencies Java Client for Elasticsearch v7.0.0 :Core/Features/Java High Level REST Client labels Dec 10, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a great idea! I left a bunch of small stuff but I think it'll be super nice.

@markharwood markharwood force-pushed the fix/36251VersionedWarningsHandler branch 2 times, most recently from c00410a to ba91f58 Compare December 11, 2018 12:10
Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -177,6 +181,69 @@ public void initClient() throws IOException {
assert hasXPack != null;
assert nodeVersions != null;
}

// Helper class to check warnings in REST responses with sensitivity to versions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use Javadoc here? I'm just so used to it in this context that anything else is like a cognitive speed bump.

@nik9000
Copy link
Member

nik9000 commented Dec 11, 2018

Thanks for doing this @markharwood!


}

public static RequestOptions expectVersionSpecificWarnings(Consumer<VersionSensitiveWarningsHandler> expectationsSetter) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe this should be protected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was protected originally but Nik requested it be made public static to allow use from elsewhere.

… and current-version-only clusters.

This is supported by a new VersionSpecificWarningsHandler class with associated unit test.
Closes elastic#36251
@markharwood markharwood force-pushed the fix/36251VersionedWarningsHandler branch from 11f9385 to 033bfca Compare December 11, 2018 15:37
@markharwood markharwood merged commit a9eccbc into elastic:master Dec 11, 2018
@markharwood
Copy link
Contributor Author

Thanks for the reviews @nik9000 and @javanna

jasontedor added a commit to dnhatn/elasticsearch that referenced this pull request Dec 11, 2018
* elastic/master: (36 commits)
  Add check for minimum required Docker version (elastic#36497)
  Minor search controller changes (elastic#36479)
  Add default methods to DocValueFormat (elastic#36480)
  Fix the mixed cluster REST test explain/11_basic_with_types.
  Modify `BigArrays` to take name of circuit breaker (elastic#36461)
  Move LoggedExec to minimumRuntime source set (elastic#36453)
  added 6.5.4 version
  Add test logging for elastic#35644
  Tests- added helper methods to ESRestTestCase for checking warnings (elastic#36443)
  SQL: move requests' parameters to requests JSON body (elastic#36149)
  [Zen2] Respect the no_master_block setting (elastic#36478)
  Require soft-deletes when access changes snapshot (elastic#36446)
  Switch more tests to zen2 (elastic#36367)
  [Painless] Add extensive tests for def to primitive casts (elastic#36455)
  Add setting to bypass Rollover action (elastic#36235)
  Try running CI against Zulu (elastic#36391)
  [DOCS] Reworked the shard allocation filtering info.  (elastic#36456)
  Log [initial_master_nodes] on formation failure (elastic#36466)
  converting ForbiddenPatternsTask to .java (elastic#36194)
  fixed typo
  ...
markharwood added a commit that referenced this pull request Dec 18, 2018
…36443)

Added helper methods to ESRestTestCase for checking warnings in mixed and current-version-only clusters.
This is supported by a new VersionSpecificWarningsHandler class with associated unit test.
Closes #36251
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Clients/Java Low Level REST Client Minimal dependencies Java Client for Elasticsearch >test Issues or PRs that are addressing/adding tests v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants