From 0a9c3ae8bcc040e4586b98a0cef5d1c434aab4f9 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Mon, 6 Aug 2018 08:53:44 -0600 Subject: [PATCH] Remove UpdateSettingsTestHelper class (#32557) * Remove UpdateSettingsTestHelper class By making the `settings()` method public on `UpdateSettingsRequest` (I think it should have been in the first place) we can get rid of this class entirely. Mock response objects are now constructed by parsing JSON without making the constructor public. Relates to #29823 --- .../settings/put/UpdateSettingsRequest.java | 2 +- .../settings/put/UpdateSettingsResponse.java | 2 +- .../put/UpdateSettingsTestHelper.java | 48 ------------------- .../SetSingleNodeAllocateStepTests.java | 33 +++++++++---- .../UpdateSettingsStepTests.java | 14 ++++-- 5 files changed, 36 insertions(+), 63 deletions(-) delete mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsTestHelper.java diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsRequest.java index b229e2c9e6a..e7b5d822074 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsRequest.java @@ -88,7 +88,7 @@ public class UpdateSettingsRequest extends AcknowledgedRequest acceptableValues, boolean assertOnlyKeyInSettings, String... expectedIndices) { - assertNotNull(request); - assertArrayEquals(expectedIndices, request.indices()); - assertThat(request.settings().get(settingsKey), anyOf(acceptableValues.stream().map(e -> equalTo(e)).collect(Collectors.toList()))); - if (assertOnlyKeyInSettings) { - assertEquals(1, request.settings().size()); - } - } - - // NORELEASE this isn't nice but it's currently the only way to create an - // UpdateSettingsResponse. Need to see if we can make the constructor public - // in ES - public static UpdateSettingsResponse createMockResponse(boolean acknowledged) { - return new UpdateSettingsResponse(acknowledged); - } -} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/SetSingleNodeAllocateStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/SetSingleNodeAllocateStepTests.java index b523cdb1235..78b3cf50235 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/SetSingleNodeAllocateStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/SetSingleNodeAllocateStepTests.java @@ -10,7 +10,6 @@ import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest; import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsResponse; -import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsTestHelper; import org.elasticsearch.client.AdminClient; import org.elasticsearch.client.Client; import org.elasticsearch.client.IndicesAdminClient; @@ -39,8 +38,13 @@ import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import java.io.IOException; import java.util.HashSet; import java.util.Set; +import java.util.stream.Collectors; + +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.equalTo; public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase { @@ -80,7 +84,18 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase acceptableValues, boolean assertOnlyKeyInSettings, + String... expectedIndices) { + assertNotNull(request); + assertArrayEquals(expectedIndices, request.indices()); + assertThat(request.settings().get(settingsKey), anyOf(acceptableValues.stream().map(e -> equalTo(e)).collect(Collectors.toList()))); + if (assertOnlyKeyInSettings) { + assertEquals(1, request.settings().size()); + } + } + + public void testPerformActionNoAttrs() throws IOException { IndexMetaData indexMetaData = IndexMetaData.builder(randomAlphaOfLength(10)).settings(settings(Version.CURRENT)) .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5)).build(); Index index = indexMetaData.getIndex(); @@ -101,7 +116,7 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase listener = (ActionListener) invocation.getArguments()[1]; - UpdateSettingsTestHelper.assertSettingsRequestContainsValueFrom(request, + assertSettingsRequestContainsValueFrom(request, IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_name", validNodeNames, true, indexMetaData.getIndex().getName()); listener.onFailure(exception); @@ -325,7 +340,8 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase validNodeNames, DiscoveryNodes.Builder nodes) { + private void assertNodeSelected(IndexMetaData indexMetaData, Index index, + Set validNodeNames, DiscoveryNodes.Builder nodes) throws IOException { ImmutableOpenMap.Builder indices = ImmutableOpenMap. builder().fPut(index.getName(), indexMetaData); IndexRoutingTable.Builder indexRoutingTable = IndexRoutingTable.builder(index) @@ -340,6 +356,7 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase() { @Override @@ -347,10 +364,10 @@ public class SetSingleNodeAllocateStepTests extends AbstractStepTestCase listener = (ActionListener) invocation.getArguments()[1]; - UpdateSettingsTestHelper.assertSettingsRequestContainsValueFrom(request, + assertSettingsRequestContainsValueFrom(request, IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_name", validNodeNames, true, indexMetaData.getIndex().getName()); - listener.onResponse(UpdateSettingsTestHelper.createMockResponse(true)); + listener.onResponse(new UpdateSettingsResponse(true)); return null; } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/UpdateSettingsStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/UpdateSettingsStepTests.java index b62e5ef0626..a592e0bdffb 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/UpdateSettingsStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/UpdateSettingsStepTests.java @@ -11,7 +11,6 @@ import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest; import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsResponse; -import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsTestHelper; import org.elasticsearch.client.AdminClient; import org.elasticsearch.client.Client; import org.elasticsearch.client.IndicesAdminClient; @@ -24,6 +23,8 @@ import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import static org.hamcrest.Matchers.equalTo; + public class UpdateSettingsStepTests extends AbstractStepTestCase { private Client client; @@ -70,7 +71,7 @@ public class UpdateSettingsStepTests extends AbstractStepTestCase() { @Override @@ -88,8 +90,9 @@ public class UpdateSettingsStepTests extends AbstractStepTestCase listener = (ActionListener) invocation.getArguments()[1]; - UpdateSettingsTestHelper.assertSettingsRequest(request, step.getSettings(), indexMetaData.getIndex().getName()); - listener.onResponse(UpdateSettingsTestHelper.createMockResponse(true)); + assertThat(request.settings(), equalTo(step.getSettings())); + assertThat(request.indices(), equalTo(new String[] {indexMetaData.getIndex().getName()})); + listener.onResponse(new UpdateSettingsResponse(true)); return null; } @@ -135,7 +138,8 @@ public class UpdateSettingsStepTests extends AbstractStepTestCase listener = (ActionListener) invocation.getArguments()[1]; - UpdateSettingsTestHelper.assertSettingsRequest(request, step.getSettings(), indexMetaData.getIndex().getName()); + assertThat(request.settings(), equalTo(step.getSettings())); + assertThat(request.indices(), equalTo(new String[] {indexMetaData.getIndex().getName()})); listener.onFailure(exception); return null; }