From a67334a15b8ec12ee060e427882404578251ffdb Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Mon, 11 Oct 2021 11:29:50 -0400 Subject: [PATCH] Replace securemock with mock-maker (test support), update Mockito to 3.12.4 (#1332) (#1354) 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 c61dfbe7d2443..09d5a34aa30ea 100644 --- a/buildSrc/version.properties +++ b/buildSrc/version.properties @@ -36,10 +36,10 @@ httpasyncclient = 4.1.4 commonslogging = 1.1.3 commonscodec = 1.13 hamcrest = 2.1 -securemock = 1.2 mocksocket = 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 e296ccf9d9f15..2271fed252793 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 554d79700dd14..7830e23d06636 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.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 void createRestClient() { 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 void testInternalHttpRequest() throws Exception { 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 057446c981834..f81f4ccc3b1e8 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 f0aa85b4c8274..6cb6f0b7cf116 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.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 548f794bdff27..078c56fc94fb5 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.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 void testSelectorTimeoutWillBeReducedIfTaskSooner() throws Exception { 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 void testSelectorClosedExceptionIsNotCaughtWhileRunning() throws IOExcept 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 c5af42dcfdf50..e95bf0115968e 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.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.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 3e70d8618742d..9c4e757303cac 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 529078100e4bd..d0403c7483750 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.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 */ @@ -198,7 +192,12 @@ public void setupAction() { 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); @@ -671,6 +670,7 @@ public void testFindDefaultPipelineFromV2TemplateMatch() { })); 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 cc0a2a803a231..fe96692a405b0 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 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 TaskManager getTaskManager() { 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 b8f56a126a1d3..73e4a565ec09e 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.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 TaskManager getTaskManager() { 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 3dbf3f34e213a..6c06d3460a0b6 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 @@ 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 57ee8913aeb17..a65ec285d5976 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.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 BoundTransportAddress boundAddress() { 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 792dc14dd7e44..b797d4e1e9fbf 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 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 37b9162e1fa33..c6961317b30f3 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.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 b2e2b96d2c629..9b91070e29a1b 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.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 afbf9411451b8..3c7dde33b23b2 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 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 void testFailingListenerAfterTimeout() throws InterruptedException { 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 d3f843889ad00..c77c7c3ea5004 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.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 void testFindEarliestLastModifiedAge() throws IOException { 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 void testFindEarliestLastModifiedAge() throws IOException { 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 bfccc2c6c05f7..673fc6ba4f3bf 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.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 void testExecuteBulkPipelineDoesNotExist() { 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 void testBulkRequestExecutionWithFailures() throws Exception { 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 @@ private CompoundProcessor mockCompoundProcessor() { return processor; } - private class IngestDocumentMatcher extends ArgumentMatcher { + private class IngestDocumentMatcher implements ArgumentMatcher { private final IngestDocument ingestDocument; @@ -1512,13 +1497,8 @@ private class IngestDocumentMatcher extends ArgumentMatcher { } @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 950ecf3321e17..5d5f353c9fff1 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.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 a1243b032b93b..ff1605548fbf6 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.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 void testPreProcess() throws Exception { 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 void testClearQueryCancellationsOnClose() throws IOException { 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 676d9e648cd1a..7c0ba75fbd795 100644 --- a/test/framework/build.gradle +++ b/test/framework/build.gradle @@ -45,8 +45,10 @@ 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.elasticsearch:mocksocket:${versions.mocksocket}" + 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,-rawtypes,-unchecked' @@ -72,15 +74,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 8b860ff7beb2d..14c209b698ad8 100644 --- a/test/framework/src/main/java/org/opensearch/bootstrap/BootstrapForTesting.java +++ b/test/framework/src/main/java/org/opensearch/bootstrap/BootstrapForTesting.java @@ -43,6 +43,7 @@ import org.opensearch.common.io.PathUtils; import org.opensearch.common.network.IfConfig; 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; @@ -147,8 +148,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"); } @@ -161,6 +162,8 @@ public boolean implies(ProtectionDomain domain, Permission permission) { return opensearchPolicy.implies(domain, permission) || testFramework.implies(domain, permission); } }); + + PriviledgedMockMaker.createAccessControlContext(); System.setSecurityManager(SecureSM.createTestSecureSM()); 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 0000000000000..f5f6843e817ea --- /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 0000000000000..e1795b7b9b3d6 --- /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