From 33a9f40c9c7ab5a1f0f8ec8dec2b08aa393d012f Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Fri, 8 Mar 2019 15:15:22 +0100 Subject: [PATCH] Add realistic hlrc response serialization test base class. The base class facilitates generating a server side response test instance, that gets serialized as xcontent, which then gets parsed into a hlrc response instance, which then gets asserted against the server side response instance. This way of testing is more realistic then how hlrc response classes are tested today, which basically tests that serialization works by generating hlrc response instance, serialize that to xcontent and then parse it back to a hlrc response instance. Besides adding a base test class, this change also cuts AcknowledgedResponseTests and BroadcastResponseTests over to use this base class. Relates to #39745 --- .../client/AbstractResponseTestCase.java | 61 +++++++++++++++++ .../core/AcknowledgedResponseTests.java | 32 +++++---- .../client/core/BroadcastResponseTests.java | 65 +++++++++---------- 3 files changed, 112 insertions(+), 46 deletions(-) create mode 100644 client/rest-high-level/src/test/java/org/elasticsearch/client/AbstractResponseTestCase.java diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/AbstractResponseTestCase.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/AbstractResponseTestCase.java new file mode 100644 index 0000000000000..0d09d375af7d6 --- /dev/null +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/AbstractResponseTestCase.java @@ -0,0 +1,61 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.client; + +import org.elasticsearch.cluster.ClusterModule; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContent; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; + +public abstract class AbstractResponseTestCase extends ESTestCase { + + private static final int NUMBER_OF_TEST_RUNS = 20; + + public final void testFromXContent() throws IOException { + for (int i = 0; i < NUMBER_OF_TEST_RUNS; i++) { + final SERVER_INSTANCE serverTestInstance = createServerTestInstance(); + + final XContentType xContentType = randomFrom(XContentType.values()); + final BytesReference bytes = toShuffledXContent(serverTestInstance, xContentType, ToXContent.EMPTY_PARAMS, randomBoolean()); + + final XContent xContent = XContentFactory.xContent(xContentType); + final XContentParser parser = xContent.createParser( + new NamedXContentRegistry(ClusterModule.getNamedXWriteables()), + LoggingDeprecationHandler.INSTANCE, + bytes.streamInput()); + final HLRC_INSTANCE hlrcInstance = doParseInstance(parser); + assertInstances(serverTestInstance, hlrcInstance); + } + } + + protected abstract SERVER_INSTANCE createServerTestInstance(); + + protected abstract HLRC_INSTANCE doParseInstance(XContentParser parser) throws IOException; + + protected abstract void assertInstances(final SERVER_INSTANCE serverTestInstance, final HLRC_INSTANCE hlrcInstance); + +} diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/core/AcknowledgedResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/core/AcknowledgedResponseTests.java index c05ea470c219f..33c3066825ebd 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/core/AcknowledgedResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/core/AcknowledgedResponseTests.java @@ -18,27 +18,35 @@ */ package org.elasticsearch.client.core; +import org.elasticsearch.client.AbstractResponseTestCase; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; -import static org.elasticsearch.test.AbstractXContentTestCase.xContentTester; +import static org.hamcrest.Matchers.is; -public class AcknowledgedResponseTests extends ESTestCase { +public class AcknowledgedResponseTests extends AbstractResponseTestCase { - public void testFromXContent() throws IOException { - xContentTester(this::createParser, - this::createTestInstance, - AcknowledgedResponseTests::toXContent, - AcknowledgedResponse::fromXContent) - .supportsUnknownFields(false) - .test(); + @Override + protected org.elasticsearch.action.support.master.AcknowledgedResponse createServerTestInstance() { + return new org.elasticsearch.action.support.master.AcknowledgedResponse(randomBoolean()); } - private AcknowledgedResponse createTestInstance() { - return new AcknowledgedResponse(randomBoolean()); + + @Override + protected AcknowledgedResponse doParseInstance(XContentParser parser) throws IOException { + return AcknowledgedResponse.fromXContent(parser); + } + + @Override + protected void assertInstances(org.elasticsearch.action.support.master.AcknowledgedResponse serverTestInstance, + AcknowledgedResponse hlrcInstance) { + assertThat(hlrcInstance.isAcknowledged(), is(serverTestInstance.isAcknowledged())); } + // Still needed for StopRollupJobResponseTests and StartRollupJobResponseTests test classes + // This method can't be moved to these classes because getFieldName() method isn't accessible from these test classes. public static void toXContent(AcknowledgedResponse response, XContentBuilder builder) throws IOException { builder.startObject(); { diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/core/BroadcastResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/core/BroadcastResponseTests.java index 96438725d4ef0..b34fc996e4662 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/core/BroadcastResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/core/BroadcastResponseTests.java @@ -20,17 +20,9 @@ package org.elasticsearch.client.core; import org.elasticsearch.action.support.DefaultShardOperationFailedException; -import org.elasticsearch.cluster.ClusterModule; -import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; -import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContent; -import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.client.AbstractResponseTestCase; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.seqno.RetentionLeaseNotFoundException; -import org.elasticsearch.test.ESTestCase; import java.io.IOException; import java.util.ArrayList; @@ -43,44 +35,49 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.isIn; -public class BroadcastResponseTests extends ESTestCase { +public class BroadcastResponseTests extends AbstractResponseTestCase { - public void testFromXContent() throws IOException { - final String index = randomAlphaOfLength(8); - final String id = randomAlphaOfLength(8); + private String index; + private String id; + private Set shardIds; + + @Override + protected org.elasticsearch.action.support.broadcast.BroadcastResponse createServerTestInstance() { + index = randomAlphaOfLength(8); + id = randomAlphaOfLength(8); final int total = randomIntBetween(1, 16); final int successful = total - scaledRandomIntBetween(0, total); final int failed = scaledRandomIntBetween(0, total - successful); final List failures = new ArrayList<>(); - final Set shardIds = new HashSet<>(); + shardIds = new HashSet<>(); for (int i = 0; i < failed; i++) { final DefaultShardOperationFailedException failure = new DefaultShardOperationFailedException( - index, - randomValueOtherThanMany(shardIds::contains, () -> randomIntBetween(0, total - 1)), - new RetentionLeaseNotFoundException(id)); + index, + randomValueOtherThanMany(shardIds::contains, () -> randomIntBetween(0, total - 1)), + new RetentionLeaseNotFoundException(id)); failures.add(failure); shardIds.add(failure.shardId()); } - final org.elasticsearch.action.support.broadcast.BroadcastResponse to = - new org.elasticsearch.action.support.broadcast.BroadcastResponse(total, successful, failed, failures); + return new org.elasticsearch.action.support.broadcast.BroadcastResponse(total, successful, failed, failures); + } - final XContentType xContentType = randomFrom(XContentType.values()); - final BytesReference bytes = toShuffledXContent(to, xContentType, ToXContent.EMPTY_PARAMS, randomBoolean()); + @Override + protected BroadcastResponse doParseInstance(XContentParser parser) throws IOException { + return BroadcastResponse.fromXContent(parser); + } - final XContent xContent = XContentFactory.xContent(xContentType); - final XContentParser parser = xContent.createParser( - new NamedXContentRegistry(ClusterModule.getNamedXWriteables()), - LoggingDeprecationHandler.INSTANCE, - bytes.streamInput()); - final BroadcastResponse from = BroadcastResponse.fromXContent(parser); - assertThat(from.shards().total(), equalTo(total)); - assertThat(from.shards().successful(), equalTo(successful)); - assertThat(from.shards().skipped(), equalTo(0)); - assertThat(from.shards().failed(), equalTo(failed)); - assertThat(from.shards().failures(), hasSize(failed == 0 ? failed : 1)); // failures are grouped - if (failed > 0) { - final DefaultShardOperationFailedException groupedFailure = from.shards().failures().iterator().next(); + @Override + protected void assertInstances(org.elasticsearch.action.support.broadcast.BroadcastResponse serverTestInstance, + BroadcastResponse hlrcInstance) { + assertThat(hlrcInstance.shards().total(), equalTo(serverTestInstance.getTotalShards())); + assertThat(hlrcInstance.shards().successful(), equalTo(serverTestInstance.getSuccessfulShards())); + assertThat(hlrcInstance.shards().skipped(), equalTo(0)); + assertThat(hlrcInstance.shards().failed(), equalTo(serverTestInstance.getFailedShards())); + assertThat(hlrcInstance.shards().failures(), hasSize(hlrcInstance.shards().failed() == 0 ? 0 : 1)); // failures are grouped + if (hlrcInstance.shards().failed() > 0) { + final DefaultShardOperationFailedException groupedFailure = hlrcInstance.shards().failures().iterator().next(); assertThat(groupedFailure.index(), equalTo(index)); assertThat(groupedFailure.shardId(), isIn(shardIds)); assertThat(groupedFailure.reason(), containsString("reason=retention lease with ID [" + id + "] not found"));