From 29ffc42799140de650e5edfd91954ca1d06d3064 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Thu, 17 Mar 2022 23:00:03 -0700 Subject: [PATCH 1/3] Flip arguments in assertEquals() in ReindexRenamedSettingTests Signed-off-by: Tianli Feng --- .../reindex/ReindexRenamedSettingTests.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRenamedSettingTests.java b/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRenamedSettingTests.java index 8ff84223d371e..f98b831662b34 100644 --- a/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRenamedSettingTests.java +++ b/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRenamedSettingTests.java @@ -41,8 +41,8 @@ public void testReindexSettingsExist() { */ public void testSettingFallback() { assertEquals( - TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(Settings.EMPTY), - TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(Settings.EMPTY) + TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(Settings.EMPTY), + TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(Settings.EMPTY) ); } @@ -51,10 +51,10 @@ public void testSettingFallback() { */ public void testSettingGetValue() { Settings settings = Settings.builder().put("reindex.remote.allowlist", "127.0.0.1:*").build(); - assertEquals(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), Arrays.asList("127.0.0.1:*")); + assertEquals(Arrays.asList("127.0.0.1:*"), TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings)); assertEquals( - TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(settings), - TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getDefault(Settings.EMPTY) + TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getDefault(Settings.EMPTY), + TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(settings) ); } @@ -63,7 +63,7 @@ public void testSettingGetValue() { */ public void testSettingGetValueWithFallback() { Settings settings = Settings.builder().put("reindex.remote.whitelist", "127.0.0.1:*").build(); - assertEquals(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), Arrays.asList("127.0.0.1:*")); + assertEquals(Arrays.asList("127.0.0.1:*"), TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings)); assertSettingDeprecationsAndWarnings(new Setting[] { TransportReindexAction.REMOTE_CLUSTER_WHITELIST }); } @@ -75,8 +75,8 @@ public void testSettingGetValueWhenBothAreConfigured() { .put("reindex.remote.allowlist", "127.0.0.1:*") .put("reindex.remote.whitelist", "[::1]:*, 127.0.0.1:*") .build(); - assertEquals(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), Arrays.asList("127.0.0.1:*")); - assertEquals(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(settings), Arrays.asList("[::1]:*", "127.0.0.1:*")); + assertEquals(Arrays.asList("127.0.0.1:*"), TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings)); + assertEquals(Arrays.asList("[::1]:*", "127.0.0.1:*"), TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(settings)); assertSettingDeprecationsAndWarnings(new Setting[] { TransportReindexAction.REMOTE_CLUSTER_WHITELIST }); } From eccb012e41ecfe4396ceb09bdc080f84ec42d871 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Fri, 18 Mar 2022 11:38:13 -0700 Subject: [PATCH 2/3] Use Hamcrest assertThat and matchers instead of junit methods Signed-off-by: Tianli Feng --- .../reindex/ReindexRenamedSettingTests.java | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRenamedSettingTests.java b/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRenamedSettingTests.java index f98b831662b34..9426fac894c16 100644 --- a/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRenamedSettingTests.java +++ b/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRenamedSettingTests.java @@ -8,6 +8,7 @@ package org.opensearch.index.reindex; +import org.hamcrest.MatcherAssert; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.test.OpenSearchTestCase; @@ -15,6 +16,8 @@ import java.util.Arrays; import java.util.List; +import static org.hamcrest.Matchers.equalTo; + /** * A unit test to validate the former name of the setting 'reindex.remote.allowlist' still take effect, * after it is deprecated, so that the backwards compatibility is maintained. @@ -28,7 +31,7 @@ public class ReindexRenamedSettingTests extends OpenSearchTestCase { */ public void testReindexSettingsExist() { List> settings = plugin.getSettings(); - assertTrue( + MatcherAssert.assertThat( "Both 'reindex.remote.allowlist' and its predecessor should be supported settings of Reindex plugin", settings.containsAll( Arrays.asList(TransportReindexAction.REMOTE_CLUSTER_WHITELIST, TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST) @@ -40,9 +43,9 @@ public void testReindexSettingsExist() { * Validate the default value of the both settings is the same. */ public void testSettingFallback() { - assertEquals( - TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(Settings.EMPTY), - TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(Settings.EMPTY) + MatcherAssert.assertThat( + TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(Settings.EMPTY), + equalTo(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(Settings.EMPTY)) ); } @@ -51,10 +54,10 @@ public void testSettingFallback() { */ public void testSettingGetValue() { Settings settings = Settings.builder().put("reindex.remote.allowlist", "127.0.0.1:*").build(); - assertEquals(Arrays.asList("127.0.0.1:*"), TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings)); - assertEquals( - TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getDefault(Settings.EMPTY), - TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(settings) + MatcherAssert.assertThat(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), equalTo(Arrays.asList("127.0.0.1:*"))); + MatcherAssert.assertThat( + TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(settings), + equalTo(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getDefault(Settings.EMPTY)) ); } @@ -63,7 +66,7 @@ public void testSettingGetValue() { */ public void testSettingGetValueWithFallback() { Settings settings = Settings.builder().put("reindex.remote.whitelist", "127.0.0.1:*").build(); - assertEquals(Arrays.asList("127.0.0.1:*"), TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings)); + MatcherAssert.assertThat(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), equalTo(Arrays.asList("127.0.0.1:*"))); assertSettingDeprecationsAndWarnings(new Setting[] { TransportReindexAction.REMOTE_CLUSTER_WHITELIST }); } @@ -75,8 +78,11 @@ public void testSettingGetValueWhenBothAreConfigured() { .put("reindex.remote.allowlist", "127.0.0.1:*") .put("reindex.remote.whitelist", "[::1]:*, 127.0.0.1:*") .build(); - assertEquals(Arrays.asList("127.0.0.1:*"), TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings)); - assertEquals(Arrays.asList("[::1]:*", "127.0.0.1:*"), TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(settings)); + MatcherAssert.assertThat(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), equalTo(Arrays.asList("127.0.0.1:*"))); + MatcherAssert.assertThat( + TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(settings), + equalTo(Arrays.asList("[::1]:*", "127.0.0.1:*")) + ); assertSettingDeprecationsAndWarnings(new Setting[] { TransportReindexAction.REMOTE_CLUSTER_WHITELIST }); } From ce4adbb2e492e01c106e66d87736b32c70891133 Mon Sep 17 00:00:00 2001 From: Tianli Feng Date: Fri, 18 Mar 2022 14:11:28 -0700 Subject: [PATCH 3/3] Use assertThat() and Hamcrest matchers instead of junit navtive methods Signed-off-by: Tianli Feng --- .../reindex/ReindexRenamedSettingTests.java | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRenamedSettingTests.java b/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRenamedSettingTests.java index 9426fac894c16..00747d85221c6 100644 --- a/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRenamedSettingTests.java +++ b/modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRenamedSettingTests.java @@ -8,7 +8,6 @@ package org.opensearch.index.reindex; -import org.hamcrest.MatcherAssert; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.test.OpenSearchTestCase; @@ -17,6 +16,7 @@ import java.util.List; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItems; /** * A unit test to validate the former name of the setting 'reindex.remote.allowlist' still take effect, @@ -31,11 +31,10 @@ public class ReindexRenamedSettingTests extends OpenSearchTestCase { */ public void testReindexSettingsExist() { List> settings = plugin.getSettings(); - MatcherAssert.assertThat( + assertThat( "Both 'reindex.remote.allowlist' and its predecessor should be supported settings of Reindex plugin", - settings.containsAll( - Arrays.asList(TransportReindexAction.REMOTE_CLUSTER_WHITELIST, TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST) - ) + settings, + hasItems(TransportReindexAction.REMOTE_CLUSTER_WHITELIST, TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST) ); } @@ -43,7 +42,7 @@ public void testReindexSettingsExist() { * Validate the default value of the both settings is the same. */ public void testSettingFallback() { - MatcherAssert.assertThat( + assertThat( TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(Settings.EMPTY), equalTo(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(Settings.EMPTY)) ); @@ -54,8 +53,8 @@ public void testSettingFallback() { */ public void testSettingGetValue() { Settings settings = Settings.builder().put("reindex.remote.allowlist", "127.0.0.1:*").build(); - MatcherAssert.assertThat(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), equalTo(Arrays.asList("127.0.0.1:*"))); - MatcherAssert.assertThat( + assertThat(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), equalTo(Arrays.asList("127.0.0.1:*"))); + assertThat( TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(settings), equalTo(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getDefault(Settings.EMPTY)) ); @@ -66,7 +65,7 @@ public void testSettingGetValue() { */ public void testSettingGetValueWithFallback() { Settings settings = Settings.builder().put("reindex.remote.whitelist", "127.0.0.1:*").build(); - MatcherAssert.assertThat(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), equalTo(Arrays.asList("127.0.0.1:*"))); + assertThat(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), equalTo(Arrays.asList("127.0.0.1:*"))); assertSettingDeprecationsAndWarnings(new Setting[] { TransportReindexAction.REMOTE_CLUSTER_WHITELIST }); } @@ -78,11 +77,8 @@ public void testSettingGetValueWhenBothAreConfigured() { .put("reindex.remote.allowlist", "127.0.0.1:*") .put("reindex.remote.whitelist", "[::1]:*, 127.0.0.1:*") .build(); - MatcherAssert.assertThat(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), equalTo(Arrays.asList("127.0.0.1:*"))); - MatcherAssert.assertThat( - TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(settings), - equalTo(Arrays.asList("[::1]:*", "127.0.0.1:*")) - ); + assertThat(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), equalTo(Arrays.asList("127.0.0.1:*"))); + assertThat(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(settings), equalTo(Arrays.asList("[::1]:*", "127.0.0.1:*"))); assertSettingDeprecationsAndWarnings(new Setting[] { TransportReindexAction.REMOTE_CLUSTER_WHITELIST }); }