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

LLRC: Test for warnings behavior #34143

Merged
merged 3 commits into from
Oct 6, 2018
Merged

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Sep 28, 2018

Add tests for the Low Level REST Client's strict deprecation hangling.

Relates to #33708

Add tests for the Low Level REST Client's strict deprecation hangling.

Relates to elastic#33708
@nik9000 nik9000 added >test Issues or PRs that are addressing/adding tests :Clients/Java Low Level REST Client Minimal dependencies Java Client for Elasticsearch v7.0.0 v6.5.0 labels Sep 28, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@@ -124,9 +124,9 @@ public HttpEntity getEntity() {
final Matcher matcher = WARNING_HEADER_PATTERN.matcher(warning);
if (matcher.matches()) {
warnings.add(matcher.group(1));
continue;
} else {
Copy link
Member Author

Choose a reason for hiding this comment

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

I flipped this around because it lines up a bit better with the rest of our style.

@nik9000 nik9000 requested review from javanna and hub-cap October 1, 2018 19:21
@nik9000 nik9000 added the review label Oct 1, 2018
* Emulates Elasticsearch's DeprecationLogger.formatWarning in simple
* cases. We don't have that available because we're testing against 1.7.
*/
private String formatWarning(String warningBody) {
Copy link
Member

Choose a reason for hiding this comment

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

static ?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

if (strictDeprecationMode) {
try {
restClient.performRequest(request);
fail("expected ResponseException");
Copy link
Member

Choose a reason for hiding this comment

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

maybe clarify that we expect an exception due to a warning as strict deprecation mode is enabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍


}

private void deprecationWarningTest(List<String> warningHeaderTexts, List<String> warningBodyTexts) throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

maybe this can be static too, and I would rename to assertDeprecationWarnings or expectDeprecationWarnings

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@nik9000 nik9000 merged commit d905cc8 into elastic:master Oct 6, 2018
nik9000 added a commit that referenced this pull request Oct 6, 2018
Add tests for the Low Level REST Client's strict deprecation handling.

Relates to #33708
kcm pushed a commit that referenced this pull request Oct 30, 2018
Add tests for the Low Level REST Client's strict deprecation handling.

Relates to #33708
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 v6.5.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants