From e9635d6bfeade4a50ee75a7c2f86f909481e41c1 Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Sun, 10 Oct 2021 14:18:54 -0400 Subject: [PATCH] Replace securemock with mock-maker (test support), update Mockito to 3.12.4 (#1332) Signed-off-by: Andriy Redko --- buildSrc/version.properties | 6 +- client/rest/build.gradle | 1 + .../client/RestClientSingleHostTests.java | 5 +- client/sniffer/build.gradle | 1 + .../upgrade/InstallPluginsTaskTests.java | 2 +- .../org/opensearch/nio/NioSelectorTests.java | 6 +- .../percolator/QueryBuilderStoreTests.java | 2 +- .../bootstrap/test-framework.policy | 23 ++- .../bulk/TransportBulkActionIngestTests.java | 14 +- .../get/TransportMultiGetActionTests.java | 13 +- .../TransportMultiTermVectorsActionTests.java | 13 +- .../ConsistentSettingsServiceTests.java | 2 +- .../discovery/SeedHostsResolverTests.java | 3 +- .../GlobalCheckpointSyncActionTests.java | 4 +- ...tentionLeaseBackgroundSyncActionTests.java | 2 +- .../seqno/RetentionLeaseSyncActionTests.java | 2 +- .../shard/GlobalCheckpointListenersTests.java | 5 +- .../index/translog/TranslogTests.java | 6 +- .../opensearch/ingest/IngestServiceTests.java | 36 +---- .../action/cat/RestRecoveryActionTests.java | 2 +- .../search/DefaultSearchContextTests.java | 7 +- test/framework/build.gradle | 15 +- .../bootstrap/BootstrapForTesting.java | 7 +- .../mockito/plugin/PriviledgedMockMaker.java | 145 ++++++++++++++++++ .../org.mockito.plugins.MockMaker | 1 + 25 files changed, 240 insertions(+), 83 deletions(-) create mode 100644 test/framework/src/main/java/org/opensearch/mockito/plugin/PriviledgedMockMaker.java create mode 100644 test/framework/src/main/resources/mockito-extensions/org.mockito.plugins.MockMaker diff --git a/buildSrc/version.properties b/buildSrc/version.properties index f3067acb3ca..cda7419e50e 100644 --- a/buildSrc/version.properties +++ b/buildSrc/version.properties @@ -36,9 +36,9 @@ httpasyncclient = 4.1.4 commonslogging = 1.1.3 commonscodec = 1.13 hamcrest = 2.1 -securemock = 1.2 -mockito = 1.9.5 -objenesis = 1.0 +mockito = 3.12.4 +objenesis = 3.2 +bytebuddy = 1.11.13 # benchmark dependencies jmh = 1.19 diff --git a/client/rest/build.gradle b/client/rest/build.gradle index e296ccf9d9f..2271fed2527 100644 --- a/client/rest/build.gradle +++ b/client/rest/build.gradle @@ -53,6 +53,7 @@ dependencies { testImplementation "org.hamcrest:hamcrest:${versions.hamcrest}" testImplementation "org.mockito:mockito-core:${versions.mockito}" testImplementation "org.objenesis:objenesis:${versions.objenesis}" + testImplementation "net.bytebuddy:byte-buddy:${versions.bytebuddy}" } tasks.withType(CheckForbiddenApis).configureEach { diff --git a/client/rest/src/test/java/org/opensearch/client/RestClientSingleHostTests.java b/client/rest/src/test/java/org/opensearch/client/RestClientSingleHostTests.java index 554d79700dd..7830e23d066 100644 --- a/client/rest/src/test/java/org/opensearch/client/RestClientSingleHostTests.java +++ b/client/rest/src/test/java/org/opensearch/client/RestClientSingleHostTests.java @@ -101,6 +101,7 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.nullable; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -137,7 +138,7 @@ public class RestClientSingleHostTests extends RestClientTestCase { static CloseableHttpAsyncClient mockHttpClient(final ExecutorService exec) { CloseableHttpAsyncClient httpClient = mock(CloseableHttpAsyncClient.class); when(httpClient.execute(any(HttpAsyncRequestProducer.class), any(HttpAsyncResponseConsumer.class), - any(HttpClientContext.class), any(FutureCallback.class))).thenAnswer((Answer>) invocationOnMock -> { + any(HttpClientContext.class), nullable(FutureCallback.class))).thenAnswer((Answer>) invocationOnMock -> { final HttpAsyncRequestProducer requestProducer = (HttpAsyncRequestProducer) invocationOnMock.getArguments()[0]; final FutureCallback futureCallback = (FutureCallback) invocationOnMock.getArguments()[3]; @@ -215,7 +216,7 @@ public class RestClientSingleHostTests extends RestClientTestCase { for (String httpMethod : getHttpMethods()) { HttpUriRequest expectedRequest = performRandomRequest(httpMethod); verify(httpClient, times(++times)).execute(requestArgumentCaptor.capture(), - any(HttpAsyncResponseConsumer.class), any(HttpClientContext.class), any(FutureCallback.class)); + any(HttpAsyncResponseConsumer.class), any(HttpClientContext.class), nullable(FutureCallback.class)); HttpUriRequest actualRequest = (HttpUriRequest)requestArgumentCaptor.getValue().generateRequest(); assertEquals(expectedRequest.getURI(), actualRequest.getURI()); assertEquals(expectedRequest.getClass(), actualRequest.getClass()); diff --git a/client/sniffer/build.gradle b/client/sniffer/build.gradle index 057446c9818..f81f4ccc3b1 100644 --- a/client/sniffer/build.gradle +++ b/client/sniffer/build.gradle @@ -49,6 +49,7 @@ dependencies { testImplementation "junit:junit:${versions.junit}" testImplementation "org.mockito:mockito-core:${versions.mockito}" testImplementation "org.objenesis:objenesis:${versions.objenesis}" + testImplementation "net.bytebuddy:byte-buddy:${versions.bytebuddy}" } tasks.named('forbiddenApisMain').configure { diff --git a/distribution/tools/upgrade-cli/src/test/java/org/opensearch/upgrade/InstallPluginsTaskTests.java b/distribution/tools/upgrade-cli/src/test/java/org/opensearch/upgrade/InstallPluginsTaskTests.java index f0aa85b4c82..6cb6f0b7cf1 100644 --- a/distribution/tools/upgrade-cli/src/test/java/org/opensearch/upgrade/InstallPluginsTaskTests.java +++ b/distribution/tools/upgrade-cli/src/test/java/org/opensearch/upgrade/InstallPluginsTaskTests.java @@ -12,8 +12,8 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; -import org.elasticsearch.mock.orig.Mockito; import org.junit.Before; +import org.mockito.Mockito; import org.opensearch.cli.MockTerminal; import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; diff --git a/libs/nio/src/test/java/org/opensearch/nio/NioSelectorTests.java b/libs/nio/src/test/java/org/opensearch/nio/NioSelectorTests.java index 548f794bdff..078c56fc94f 100644 --- a/libs/nio/src/test/java/org/opensearch/nio/NioSelectorTests.java +++ b/libs/nio/src/test/java/org/opensearch/nio/NioSelectorTests.java @@ -52,7 +52,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BiConsumer; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.isNull; import static org.mockito.Matchers.same; import static org.mockito.Mockito.doAnswer; @@ -192,7 +192,7 @@ public class NioSelectorTests extends OpenSearchTestCase { public void testSelectorClosedExceptionIsNotCaughtWhileRunning() throws IOException { boolean closedSelectorExceptionCaught = false; - when(rawSelector.select(anyInt())).thenThrow(new ClosedSelectorException()); + when(rawSelector.select(anyLong())).thenThrow(new ClosedSelectorException()); try { this.selector.singleLoop(); } catch (ClosedSelectorException e) { @@ -205,7 +205,7 @@ public class NioSelectorTests extends OpenSearchTestCase { public void testIOExceptionWhileSelect() throws IOException { IOException ioException = new IOException(); - when(rawSelector.select(anyInt())).thenThrow(ioException); + when(rawSelector.select(anyLong())).thenThrow(ioException); this.selector.singleLoop(); diff --git a/modules/percolator/src/test/java/org/opensearch/percolator/QueryBuilderStoreTests.java b/modules/percolator/src/test/java/org/opensearch/percolator/QueryBuilderStoreTests.java index c5af42dcfdf..e95bf011596 100644 --- a/modules/percolator/src/test/java/org/opensearch/percolator/QueryBuilderStoreTests.java +++ b/modules/percolator/src/test/java/org/opensearch/percolator/QueryBuilderStoreTests.java @@ -41,6 +41,7 @@ import org.apache.lucene.index.NoMergePolicy; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; import org.apache.lucene.store.Directory; +import org.mockito.Mockito; import org.opensearch.LegacyESVersion; import org.opensearch.Version; import org.opensearch.cluster.metadata.IndexMetadata; @@ -56,7 +57,6 @@ import org.opensearch.index.mapper.Mapper; import org.opensearch.index.mapper.ParseContext; import org.opensearch.index.query.QueryShardContext; import org.opensearch.index.query.TermQueryBuilder; -import org.elasticsearch.mock.orig.Mockito; import org.opensearch.search.SearchModule; import org.opensearch.search.aggregations.support.CoreValuesSourceType; import org.opensearch.test.OpenSearchTestCase; diff --git a/server/src/main/resources/org/opensearch/bootstrap/test-framework.policy b/server/src/main/resources/org/opensearch/bootstrap/test-framework.policy index bddb54d1ceb..bcf0b704374 100644 --- a/server/src/main/resources/org/opensearch/bootstrap/test-framework.policy +++ b/server/src/main/resources/org/opensearch/bootstrap/test-framework.policy @@ -34,7 +34,7 @@ //// These are mock objects and test management that we allow test framework libs //// to provide on our behalf. But tests themselves cannot do this stuff! -grant codeBase "${codebase.securemock}" { +grant codeBase "${codebase.mockito-core}" { // needed to access ReflectionFactory (see below) permission java.lang.RuntimePermission "accessClassInPackage.sun.reflect"; // needed for reflection in ibm jdk @@ -44,6 +44,27 @@ grant codeBase "${codebase.securemock}" { // needed for spy interception, etc permission java.lang.RuntimePermission "accessDeclaredMembers"; permission java.lang.reflect.ReflectPermission "suppressAccessChecks"; + permission java.lang.RuntimePermission "getClassLoader"; +}; + +grant codeBase "${codebase.objenesis}" { + permission java.lang.RuntimePermission "reflectionFactoryAccess"; + permission java.lang.RuntimePermission "accessClassInPackage.sun.reflect"; + permission java.lang.RuntimePermission "accessDeclaredMembers"; + permission java.lang.reflect.ReflectPermission "suppressAccessChecks"; +}; + +grant codeBase "${codebase.byte-buddy}" { + permission java.lang.RuntimePermission "getClassLoader"; + permission java.lang.RuntimePermission "createClassLoader"; + permission java.lang.RuntimePermission "accessDeclaredMembers"; + permission java.lang.RuntimePermission "net.bytebuddy.createJavaDispatcher"; + permission java.lang.RuntimePermission "accessClassInPackage.sun.misc"; + permission java.lang.reflect.ReflectPermission "suppressAccessChecks"; + permission java.lang.reflect.ReflectPermission "newProxyInPackage.net.bytebuddy.utility"; + permission java.lang.reflect.ReflectPermission "newProxyInPackage.net.bytebuddy.dynamic.loading"; + permission java.lang.reflect.ReflectPermission "newProxyInPackage.net.bytebuddy.description.type"; + permission java.lang.reflect.ReflectPermission "newProxyInPackage.net.bytebuddy.description.method"; }; grant codeBase "${codebase.lucene-test-framework}" { diff --git a/server/src/test/java/org/opensearch/action/bulk/TransportBulkActionIngestTests.java b/server/src/test/java/org/opensearch/action/bulk/TransportBulkActionIngestTests.java index 084d3b060c5..90e4395b588 100644 --- a/server/src/test/java/org/opensearch/action/bulk/TransportBulkActionIngestTests.java +++ b/server/src/test/java/org/opensearch/action/bulk/TransportBulkActionIngestTests.java @@ -79,8 +79,6 @@ import org.opensearch.transport.TransportResponseHandler; import org.opensearch.transport.TransportService; import org.junit.Before; import org.mockito.ArgumentCaptor; -import org.mockito.Captor; -import org.mockito.MockitoAnnotations; import java.util.Arrays; import java.util.Collections; @@ -125,13 +123,9 @@ public class TransportBulkActionIngestTests extends OpenSearchTestCase { ThreadPool threadPool; /** Arguments to callbacks we want to capture, but which require generics, so we must use @Captor */ - @Captor ArgumentCaptor> failureHandler; - @Captor ArgumentCaptor> completionHandler; - @Captor ArgumentCaptor> remoteResponseHandler; - @Captor ArgumentCaptor>> bulkDocsItr; /** The actual action we want to test, with real indexing mocked */ @@ -199,7 +193,12 @@ public class TransportBulkActionIngestTests extends OpenSearchTestCase { threadPool = mock(ThreadPool.class); final ExecutorService direct = OpenSearchExecutors.newDirectExecutorService(); when(threadPool.executor(anyString())).thenReturn(direct); - MockitoAnnotations.initMocks(this); + + bulkDocsItr = ArgumentCaptor.forClass(Iterable.class); + failureHandler = ArgumentCaptor.forClass(BiConsumer.class); + completionHandler = ArgumentCaptor.forClass(BiConsumer.class); + remoteResponseHandler = ArgumentCaptor.forClass(TransportResponseHandler.class); + // setup services that will be called by action transportService = mock(TransportService.class); clusterService = mock(ClusterService.class); @@ -672,6 +671,7 @@ public class TransportBulkActionIngestTests extends OpenSearchTestCase { })); assertEquals("pipeline2", indexRequest.getPipeline()); + verify(ingestService).executeBulkRequest(eq(1), bulkDocsItr.capture(), failureHandler.capture(), completionHandler.capture(), any(), eq(Names.WRITE)); } diff --git a/server/src/test/java/org/opensearch/action/get/TransportMultiGetActionTests.java b/server/src/test/java/org/opensearch/action/get/TransportMultiGetActionTests.java index cc0a2a803a2..fe96692a405 100644 --- a/server/src/test/java/org/opensearch/action/get/TransportMultiGetActionTests.java +++ b/server/src/test/java/org/opensearch/action/get/TransportMultiGetActionTests.java @@ -77,6 +77,7 @@ import static java.util.Collections.emptySet; import static org.opensearch.common.UUIDs.randomBase64UUID; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; +import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; @@ -144,13 +145,13 @@ public class TransportMultiGetActionTests extends OpenSearchTestCase { when(index2ShardIterator.shardId()).thenReturn(new ShardId(index2, randomInt())); final OperationRouting operationRouting = mock(OperationRouting.class); - when(operationRouting.getShards(eq(clusterState), eq(index1.getName()), anyString(), anyString(), anyString())) - .thenReturn(index1ShardIterator); - when(operationRouting.shardId(eq(clusterState), eq(index1.getName()), anyString(), anyString())) + when(operationRouting.getShards(eq(clusterState), eq(index1.getName()), anyString(), nullable(String.class), + nullable(String.class))).thenReturn(index1ShardIterator); + when(operationRouting.shardId(eq(clusterState), eq(index1.getName()), nullable(String.class), nullable(String.class))) .thenReturn(new ShardId(index1, randomInt())); - when(operationRouting.getShards(eq(clusterState), eq(index2.getName()), anyString(), anyString(), anyString())) - .thenReturn(index2ShardIterator); - when(operationRouting.shardId(eq(clusterState), eq(index2.getName()), anyString(), anyString())) + when(operationRouting.getShards(eq(clusterState), eq(index2.getName()), anyString(), nullable(String.class), + nullable(String.class))).thenReturn(index2ShardIterator); + when(operationRouting.shardId(eq(clusterState), eq(index2.getName()), nullable(String.class), nullable(String.class))) .thenReturn(new ShardId(index2, randomInt())); clusterService = mock(ClusterService.class); diff --git a/server/src/test/java/org/opensearch/action/termvectors/TransportMultiTermVectorsActionTests.java b/server/src/test/java/org/opensearch/action/termvectors/TransportMultiTermVectorsActionTests.java index b8f56a126a1..73e4a565ec0 100644 --- a/server/src/test/java/org/opensearch/action/termvectors/TransportMultiTermVectorsActionTests.java +++ b/server/src/test/java/org/opensearch/action/termvectors/TransportMultiTermVectorsActionTests.java @@ -80,6 +80,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.nullable; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -145,13 +146,13 @@ public class TransportMultiTermVectorsActionTests extends OpenSearchTestCase { when(index2ShardIterator.shardId()).thenReturn(new ShardId(index2, randomInt())); final OperationRouting operationRouting = mock(OperationRouting.class); - when(operationRouting.getShards(eq(clusterState), eq(index1.getName()), anyString(), anyString(), anyString())) - .thenReturn(index1ShardIterator); - when(operationRouting.shardId(eq(clusterState), eq(index1.getName()), anyString(), anyString())) + when(operationRouting.getShards(eq(clusterState), eq(index1.getName()), anyString(), nullable(String.class), + nullable(String.class))).thenReturn(index1ShardIterator); + when(operationRouting.shardId(eq(clusterState), eq(index1.getName()), nullable(String.class), nullable(String.class))) .thenReturn(new ShardId(index1, randomInt())); - when(operationRouting.getShards(eq(clusterState), eq(index2.getName()), anyString(), anyString(), anyString())) - .thenReturn(index2ShardIterator); - when(operationRouting.shardId(eq(clusterState), eq(index2.getName()), anyString(), anyString())) + when(operationRouting.getShards(eq(clusterState), eq(index2.getName()), anyString(), nullable(String.class), + nullable(String.class))).thenReturn(index2ShardIterator); + when(operationRouting.shardId(eq(clusterState), eq(index2.getName()), nullable(String.class), nullable(String.class))) .thenReturn(new ShardId(index2, randomInt())); clusterService = mock(ClusterService.class); diff --git a/server/src/test/java/org/opensearch/common/settings/ConsistentSettingsServiceTests.java b/server/src/test/java/org/opensearch/common/settings/ConsistentSettingsServiceTests.java index 3dbf3f34e21..6c06d3460a0 100644 --- a/server/src/test/java/org/opensearch/common/settings/ConsistentSettingsServiceTests.java +++ b/server/src/test/java/org/opensearch/common/settings/ConsistentSettingsServiceTests.java @@ -35,9 +35,9 @@ package org.opensearch.common.settings; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.ClusterStateUpdateTask; import org.opensearch.cluster.service.ClusterService; -import org.elasticsearch.mock.orig.Mockito; import org.opensearch.test.OpenSearchTestCase; import org.junit.Before; +import org.mockito.Mockito; import org.mockito.stubbing.Answer; import java.util.Arrays; diff --git a/server/src/test/java/org/opensearch/discovery/SeedHostsResolverTests.java b/server/src/test/java/org/opensearch/discovery/SeedHostsResolverTests.java index 57ee8913aeb..a65ec285d59 100644 --- a/server/src/test/java/org/opensearch/discovery/SeedHostsResolverTests.java +++ b/server/src/test/java/org/opensearch/discovery/SeedHostsResolverTests.java @@ -70,7 +70,6 @@ import java.util.List; import java.util.Set; import java.util.Stack; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; @@ -420,6 +419,6 @@ public class SeedHostsResolverTests extends OpenSearchTestCase { assertThat(transportAddresses, hasSize(1)); // only one of the two is valid and will be used assertThat(transportAddresses.get(0).getAddress(), equalTo("127.0.0.1")); assertThat(transportAddresses.get(0).getPort(), equalTo(9301)); - verify(logger).warn(eq("failed to resolve host [127.0.0.1:9300:9300]"), Matchers.any(ExecutionException.class)); + verify(logger).warn(eq("failed to resolve host [127.0.0.1:9300:9300]"), Matchers.any(IllegalArgumentException.class)); } } diff --git a/server/src/test/java/org/opensearch/index/seqno/GlobalCheckpointSyncActionTests.java b/server/src/test/java/org/opensearch/index/seqno/GlobalCheckpointSyncActionTests.java index 792dc14dd7e..b797d4e1e9f 100644 --- a/server/src/test/java/org/opensearch/index/seqno/GlobalCheckpointSyncActionTests.java +++ b/server/src/test/java/org/opensearch/index/seqno/GlobalCheckpointSyncActionTests.java @@ -52,11 +52,11 @@ import org.opensearch.transport.TransportService; import java.util.Collections; -import static org.elasticsearch.mock.orig.Mockito.never; -import static org.elasticsearch.mock.orig.Mockito.when; import static org.opensearch.test.ClusterServiceUtils.createClusterService; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class GlobalCheckpointSyncActionTests extends OpenSearchTestCase { diff --git a/server/src/test/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncActionTests.java b/server/src/test/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncActionTests.java index 37b9162e1fa..c6961317b30 100644 --- a/server/src/test/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncActionTests.java +++ b/server/src/test/java/org/opensearch/index/seqno/RetentionLeaseBackgroundSyncActionTests.java @@ -58,11 +58,11 @@ import java.util.Collections; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; -import static org.elasticsearch.mock.orig.Mockito.when; import static org.opensearch.test.ClusterServiceUtils.createClusterService; import static org.hamcrest.Matchers.sameInstance; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class RetentionLeaseBackgroundSyncActionTests extends OpenSearchTestCase { diff --git a/server/src/test/java/org/opensearch/index/seqno/RetentionLeaseSyncActionTests.java b/server/src/test/java/org/opensearch/index/seqno/RetentionLeaseSyncActionTests.java index 28995786302..a25de878523 100644 --- a/server/src/test/java/org/opensearch/index/seqno/RetentionLeaseSyncActionTests.java +++ b/server/src/test/java/org/opensearch/index/seqno/RetentionLeaseSyncActionTests.java @@ -58,11 +58,11 @@ import java.util.Collections; import java.util.concurrent.atomic.AtomicBoolean; import static java.util.Collections.emptyMap; -import static org.elasticsearch.mock.orig.Mockito.when; import static org.opensearch.test.ClusterServiceUtils.createClusterService; import static org.hamcrest.Matchers.sameInstance; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class RetentionLeaseSyncActionTests extends OpenSearchTestCase { diff --git a/server/src/test/java/org/opensearch/index/shard/GlobalCheckpointListenersTests.java b/server/src/test/java/org/opensearch/index/shard/GlobalCheckpointListenersTests.java index afbf9411451..3c7dde33b23 100644 --- a/server/src/test/java/org/opensearch/index/shard/GlobalCheckpointListenersTests.java +++ b/server/src/test/java/org/opensearch/index/shard/GlobalCheckpointListenersTests.java @@ -66,12 +66,11 @@ import java.util.concurrent.atomic.AtomicLong; import static org.opensearch.index.seqno.SequenceNumbers.NO_OPS_PERFORMED; import static org.opensearch.index.seqno.SequenceNumbers.UNASSIGNED_SEQ_NO; -import static org.hamcrest.Matchers.any; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasToString; import static org.hamcrest.Matchers.instanceOf; -import static org.mockito.Matchers.argThat; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; @@ -646,7 +645,7 @@ public class GlobalCheckpointListenersTests extends OpenSearchTestCase { doAnswer(invocationOnMock -> { latch.countDown(); return null; - }).when(mockLogger).warn(argThat(any(String.class)), argThat(any(RuntimeException.class))); + }).when(mockLogger).warn(any(String.class), any(RuntimeException.class)); final GlobalCheckpointListeners globalCheckpointListeners = new GlobalCheckpointListeners(shardId, scheduler, mockLogger); final TimeValue timeout = TimeValue.timeValueMillis(randomIntBetween(1, 50)); diff --git a/server/src/test/java/org/opensearch/index/translog/TranslogTests.java b/server/src/test/java/org/opensearch/index/translog/TranslogTests.java index d3f843889ad..c77c7c3ea50 100644 --- a/server/src/test/java/org/opensearch/index/translog/TranslogTests.java +++ b/server/src/test/java/org/opensearch/index/translog/TranslogTests.java @@ -160,7 +160,7 @@ import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.stub; +import static org.mockito.Mockito.when; @LuceneTestCase.SuppressFileSystems("ExtrasFS") public class TranslogTests extends OpenSearchTestCase { @@ -411,7 +411,7 @@ public class TranslogTests extends OpenSearchTestCase { long period = randomLongBetween(10000, 1000000); periods[numberOfReaders] = period; TranslogWriter w = mock(TranslogWriter.class); - stub(w.getLastModifiedTime()).toReturn(fixedTime - period); + when(w.getLastModifiedTime()).thenReturn(fixedTime - period); assertThat(Translog.findEarliestLastModifiedAge(fixedTime, new ArrayList<>(), w), equalTo(period)); for (int i = 0; i < numberOfReaders; i++) { @@ -420,7 +420,7 @@ public class TranslogTests extends OpenSearchTestCase { List readers = new ArrayList<>(); for (long l : periods) { TranslogReader r = mock(TranslogReader.class); - stub(r.getLastModifiedTime()).toReturn(fixedTime - l); + when(r.getLastModifiedTime()).thenReturn(fixedTime - l); readers.add(r); } assertThat(Translog.findEarliestLastModifiedAge(fixedTime, readers, w), equalTo diff --git a/server/src/test/java/org/opensearch/ingest/IngestServiceTests.java b/server/src/test/java/org/opensearch/ingest/IngestServiceTests.java index bfccc2c6c05..673fc6ba4f3 100644 --- a/server/src/test/java/org/opensearch/ingest/IngestServiceTests.java +++ b/server/src/test/java/org/opensearch/ingest/IngestServiceTests.java @@ -77,7 +77,6 @@ import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.MockLogAppender; import org.opensearch.threadpool.ThreadPool; import org.opensearch.threadpool.ThreadPool.Names; -import org.hamcrest.CustomTypeSafeMatcher; import org.junit.Before; import org.mockito.ArgumentMatcher; import org.mockito.invocation.InvocationOnMock; @@ -724,19 +723,8 @@ public class IngestServiceTests extends OpenSearchTestCase { ingestService.executeBulkRequest(bulkRequest.numberOfActions(), bulkRequest.requests(), failureHandler, completionHandler, indexReq -> {}, Names.WRITE); verify(failureHandler, times(1)).accept( - argThat(new CustomTypeSafeMatcher("failure handler was not called with the expected arguments") { - @Override - protected boolean matchesSafely(Integer item) { - return item == 2; - } - - }), - argThat(new CustomTypeSafeMatcher("failure handler was not called with the expected arguments") { - @Override - protected boolean matchesSafely(IllegalArgumentException iae) { - return "pipeline with id [does_not_exist] does not exist".equals(iae.getMessage()); - } - }) + argThat((Integer item) -> item == 2), + argThat((IllegalArgumentException iae) -> "pipeline with id [does_not_exist] does not exist".equals(iae.getMessage())) ); verify(completionHandler, times(1)).accept(Thread.currentThread(), null); } @@ -995,12 +983,9 @@ public class IngestServiceTests extends OpenSearchTestCase { ingestService.executeBulkRequest(numRequest, bulkRequest.requests(), requestItemErrorHandler, completionHandler, indexReq -> {}, Names.WRITE); - verify(requestItemErrorHandler, times(numIndexRequests)).accept(anyInt(), argThat(new ArgumentMatcher() { - @Override - public boolean matches(final Object o) { - return ((Exception)o).getCause().equals(error); - } - })); + verify(requestItemErrorHandler, times(numIndexRequests)).accept(anyInt(), + argThat(o -> o.getCause().equals(error))); + verify(completionHandler, times(1)).accept(Thread.currentThread(), null); } @@ -1499,7 +1484,7 @@ public class IngestServiceTests extends OpenSearchTestCase { return processor; } - private class IngestDocumentMatcher extends ArgumentMatcher { + private class IngestDocumentMatcher implements ArgumentMatcher { private final IngestDocument ingestDocument; @@ -1512,13 +1497,8 @@ public class IngestServiceTests extends OpenSearchTestCase { } @Override - public boolean matches(Object o) { - if (o.getClass() == IngestDocument.class) { - IngestDocument otherIngestDocument = (IngestDocument) o; - //ingest metadata will not be the same (timestamp differs every time) - return Objects.equals(ingestDocument.getSourceAndMetadata(), otherIngestDocument.getSourceAndMetadata()); - } - return false; + public boolean matches(IngestDocument o) { + return Objects.equals(ingestDocument.getSourceAndMetadata(), o.getSourceAndMetadata()); } } diff --git a/server/src/test/java/org/opensearch/rest/action/cat/RestRecoveryActionTests.java b/server/src/test/java/org/opensearch/rest/action/cat/RestRecoveryActionTests.java index 950ecf3321e..5d5f353c9ff 100644 --- a/server/src/test/java/org/opensearch/rest/action/cat/RestRecoveryActionTests.java +++ b/server/src/test/java/org/opensearch/rest/action/cat/RestRecoveryActionTests.java @@ -55,9 +55,9 @@ import java.util.List; import java.util.Locale; import java.util.Map; -import static org.elasticsearch.mock.orig.Mockito.when; import static org.hamcrest.CoreMatchers.equalTo; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class RestRecoveryActionTests extends OpenSearchTestCase { diff --git a/server/src/test/java/org/opensearch/search/DefaultSearchContextTests.java b/server/src/test/java/org/opensearch/search/DefaultSearchContextTests.java index a1243b032b9..ff1605548fb 100644 --- a/server/src/test/java/org/opensearch/search/DefaultSearchContextTests.java +++ b/server/src/test/java/org/opensearch/search/DefaultSearchContextTests.java @@ -85,6 +85,7 @@ import static org.hamcrest.Matchers.is; import static org.mockito.Matchers.anyObject; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.nullable; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -122,7 +123,8 @@ public class DefaultSearchContextTests extends OpenSearchTestCase { when(indexCache.query()).thenReturn(queryCache); when(indexService.cache()).thenReturn(indexCache); QueryShardContext queryShardContext = mock(QueryShardContext.class); - when(indexService.newQueryShardContext(eq(shardId.id()), anyObject(), anyObject(), anyString())).thenReturn(queryShardContext); + when(indexService.newQueryShardContext(eq(shardId.id()), anyObject(), anyObject(), + nullable(String.class))).thenReturn(queryShardContext); MapperService mapperService = mock(MapperService.class); when(mapperService.hasNested()).thenReturn(randomBoolean()); when(indexService.mapperService()).thenReturn(mapperService); @@ -267,7 +269,8 @@ public class DefaultSearchContextTests extends OpenSearchTestCase { IndexService indexService = mock(IndexService.class); QueryShardContext queryShardContext = mock(QueryShardContext.class); - when(indexService.newQueryShardContext(eq(shardId.id()), anyObject(), anyObject(), anyString())).thenReturn(queryShardContext); + when(indexService.newQueryShardContext(eq(shardId.id()), anyObject(), anyObject(), + nullable(String.class))).thenReturn(queryShardContext); BigArrays bigArrays = new MockBigArrays(new MockPageCacheRecycler(Settings.EMPTY), new NoneCircuitBreakerService()); diff --git a/test/framework/build.gradle b/test/framework/build.gradle index 984401736da..42197bf5e29 100644 --- a/test/framework/build.gradle +++ b/test/framework/build.gradle @@ -45,7 +45,9 @@ dependencies { api "org.apache.lucene:lucene-codecs:${versions.lucene}" api "commons-logging:commons-logging:${versions.commonslogging}" api "commons-codec:commons-codec:${versions.commonscodec}" - api "org.elasticsearch:securemock:${versions.securemock}" + api "org.mockito:mockito-core:${versions.mockito}" + api "net.bytebuddy:byte-buddy:${versions.bytebuddy}" + api "org.objenesis:objenesis:${versions.objenesis}" } compileJava.options.compilerArgs -= '-Xlint:cast' @@ -73,15 +75,14 @@ thirdPartyAudit.ignoreMissingClasses( 'org.apache.log4j.Level', 'org.apache.log4j.Logger', 'org.apache.log4j.Priority', - // TODO - OpenSearch remove this missing classes. Issue: https://github.com/opensearch-project/OpenSearch/issues/420 - 'org.apache.tools.ant.BuildException', - 'org.apache.tools.ant.DirectoryScanner', - 'org.apache.tools.ant.Task', - 'org.apache.tools.ant.types.FileSet', + 'org.mockito.internal.creation.bytebuddy.inject.MockMethodDispatcher', + 'org.opentest4j.AssertionFailedError', + 'net.bytebuddy.agent.ByteBuddyAgent' ) // TODO - OpenSearch remove this violation. Issue: https://github.com/opensearch-project/OpenSearch/issues/420 thirdPartyAudit.ignoreViolations( - 'org.objenesis.instantiator.sun.UnsafeFactoryInstantiator' + 'org.objenesis.instantiator.sun.UnsafeFactoryInstantiator', + 'org.objenesis.instantiator.util.UnsafeUtils' ) test { diff --git a/test/framework/src/main/java/org/opensearch/bootstrap/BootstrapForTesting.java b/test/framework/src/main/java/org/opensearch/bootstrap/BootstrapForTesting.java index 0a12d8830ba..5483447ffdc 100644 --- a/test/framework/src/main/java/org/opensearch/bootstrap/BootstrapForTesting.java +++ b/test/framework/src/main/java/org/opensearch/bootstrap/BootstrapForTesting.java @@ -44,6 +44,7 @@ import org.opensearch.common.io.PathUtils; import org.opensearch.common.network.IfConfig; import org.opensearch.common.network.NetworkAddress; import org.opensearch.common.settings.Settings; +import org.opensearch.mockito.plugin.PriviledgedMockMaker; import org.opensearch.plugins.PluginInfo; import org.opensearch.secure_sm.SecureSM; import org.junit.Assert; @@ -151,8 +152,8 @@ public class BootstrapForTesting { addClassCodebase(codebases,"opensearch", "org.opensearch.plugins.PluginsService"); if (System.getProperty("tests.gradle") == null) { // intellij and eclipse don't package our internal libs, so we need to set the codebases for them manually - addClassCodebase(codebases,"plugin-classloader", "org.opensearch.plugins.ExtendedPluginsClassLoader"); - addClassCodebase(codebases,"opensearch-nio", "org.opensearch.nio.ChannelFactory"); + addClassCodebase(codebases, "plugin-classloader", "org.opensearch.plugins.ExtendedPluginsClassLoader"); + addClassCodebase(codebases, "opensearch-nio", "org.opensearch.nio.ChannelFactory"); addClassCodebase(codebases, "opensearch-secure-sm", "org.opensearch.secure_sm.SecureSM"); addClassCodebase(codebases, "opensearch-rest-client", "org.opensearch.client.RestClient"); } @@ -165,6 +166,8 @@ public class BootstrapForTesting { return opensearchPolicy.implies(domain, permission) || testFramework.implies(domain, permission); } }); + // Create access control context for mocking + PriviledgedMockMaker.createAccessControlContext(); System.setSecurityManager(SecureSM.createTestSecureSM(getTrustedHosts())); Security.selfTest(); diff --git a/test/framework/src/main/java/org/opensearch/mockito/plugin/PriviledgedMockMaker.java b/test/framework/src/main/java/org/opensearch/mockito/plugin/PriviledgedMockMaker.java new file mode 100644 index 00000000000..f5f6843e817 --- /dev/null +++ b/test/framework/src/main/java/org/opensearch/mockito/plugin/PriviledgedMockMaker.java @@ -0,0 +1,145 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.mockito.plugin; + +import org.mockito.Incubating; +import org.mockito.MockedConstruction; +import org.mockito.internal.creation.bytebuddy.ByteBuddyMockMaker; +import org.mockito.internal.util.reflection.LenientCopyTool; +import org.mockito.invocation.MockHandler; +import org.mockito.mock.MockCreationSettings; +import org.mockito.plugins.MockMaker; +import org.opensearch.common.SuppressForbidden; + +import java.security.AccessControlContext; +import java.security.AccessController; +import java.security.DomainCombiner; +import java.security.PrivilegedAction; +import java.security.ProtectionDomain; +import java.util.Arrays; +import java.util.Optional; +import java.util.function.Function; + +/** + * Mockito plugin which wraps the Mockito calls into priviledged execution blocks and respects + * SecurityManager presence. + */ +@SuppressForbidden(reason = "allow URL#getFile() to be used in tests") +public class PriviledgedMockMaker implements MockMaker { + private static AccessControlContext context; + private final ByteBuddyMockMaker delegate; + + /** + * Create dedicated AccessControlContext to use the Mockito protection domain (test only) + * so to relax the security constraints for the test cases which rely on mocks. This plugin + * wraps the mock/spy creation into priviledged action using the custom access control context + * since Mockito does not support SecurityManager out of the box. The method has to be called by + * test framework before the SecurityManager is being set, otherwise additional permissions have + * to be granted to the caller: + * + * permission java.security.Permission "createAccessControlContext" + * + */ + public static void createAccessControlContext() { + // This combiner, if bound to an access control context, will unconditionally + // substitute the call chain protection domains with the 'mockito-core' one if it + // is present. The security checks are relaxed intentionally to trust mocking + // implementation if it is part of the call chain. + final DomainCombiner combiner = (current, assigned) -> Arrays + .stream(current) + .filter(pd -> + pd + .getCodeSource() + .getLocation() + .getFile() + .contains("mockito-core") /* check mockito-core only */) + .findAny() + .map(pd -> new ProtectionDomain[] { pd }) + .orElse(current); + + // Bind combiner to an access control context (the combiner stateless and shareable) + final AccessControlContext wrapper = new AccessControlContext(AccessController.getContext(), combiner); + + // Create new access control context with dedicated combiner + context = AccessController.doPrivileged( + (PrivilegedAction) AccessController::getContext, + wrapper); + } + + /** + * Construct an instance of the priviledged mock maker using ByteBuddyMockMaker under the hood. + */ + public PriviledgedMockMaker() { + delegate = AccessController.doPrivileged( + (PrivilegedAction) () -> new ByteBuddyMockMaker(), + context); + } + + @SuppressWarnings("rawtypes") + @Override + public T createMock(MockCreationSettings settings, MockHandler handler) { + return AccessController.doPrivileged( + (PrivilegedAction) () -> delegate.createMock(settings, handler), + context); + } + + @SuppressWarnings("rawtypes") + @Override + public Optional createSpy(MockCreationSettings settings, MockHandler handler, T object) { + // The ByteBuddyMockMaker does not implement createSpy and relies on Mockito's fallback + return AccessController.doPrivileged( + (PrivilegedAction >) () -> { + T instance = delegate.createMock(settings, handler); + new LenientCopyTool().copyToMock(object, instance); + return Optional.of(instance); + }, + context); + } + + @SuppressWarnings("rawtypes") + @Override + public MockHandler getHandler(Object mock) { + return delegate.getHandler(mock); + } + + @SuppressWarnings("rawtypes") + @Override + public void resetMock(Object mock, MockHandler newHandler, MockCreationSettings settings) { + AccessController.doPrivileged( + (PrivilegedAction) () -> { + delegate.resetMock(mock, newHandler, settings); + return null; + }, context); + } + + @Override + @Incubating + public TypeMockability isTypeMockable(Class type) { + return delegate.isTypeMockable(type); + } + + @SuppressWarnings("rawtypes") + @Override + public StaticMockControl createStaticMock(Class type, MockCreationSettings settings, MockHandler handler) { + return delegate.createStaticMock(type, settings, handler); + } + + @Override + public ConstructionMockControl createConstructionMock(Class type, + Function> settingsFactory, + Function> handlerFactory, + MockedConstruction.MockInitializer mockInitializer) { + return delegate.createConstructionMock(type, settingsFactory, handlerFactory, mockInitializer); + } + + @Override + public void clearAllCaches() { + delegate.clearAllCaches(); + } +} diff --git a/test/framework/src/main/resources/mockito-extensions/org.mockito.plugins.MockMaker b/test/framework/src/main/resources/mockito-extensions/org.mockito.plugins.MockMaker new file mode 100644 index 00000000000..e1795b7b9b3 --- /dev/null +++ b/test/framework/src/main/resources/mockito-extensions/org.mockito.plugins.MockMaker @@ -0,0 +1 @@ +org.opensearch.mockito.plugin.PriviledgedMockMaker \ No newline at end of file