Add more unit tests

Signed-off-by: Jackie Han <[email protected]>
opensearch-project · Oct 3, 2023 · ed5dcc7 · ed5dcc7
1 parent f10ff22
commit ed5dcc7
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 35 deletions.
diff --git a/build.gradle b/build.gradle
@@ -11,7 +11,6 @@ apply plugin: 'com.diffplug.spotless'
 apply from: 'formatter/formatting.gradle'
 // for javadocs and checks spotless doesn't do
 apply plugin: 'checkstyle'
-checkstyleTest.enabled = false
 // for coverage and diff
 apply plugin: 'jacoco'
 apply plugin: 'com.form.diff-coverage'

diff --git a/src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java b/src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java
@@ -8,6 +8,7 @@
  */
 package org.opensearch.flowframework.workflow;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Charsets;
 import com.google.common.io.Resources;
 import org.apache.logging.log4j.LogManager;
@@ -226,7 +227,8 @@ public static String getIndexMappings(String mapping) throws IOException {
      * @param newVersion new index mapping version
      * @param listener action listener, if update index is needed, will pass true to its onResponse method
      */
-    private void shouldUpdateIndex(String indexName, Integer newVersion, ActionListener<Boolean> listener) {
+    @VisibleForTesting
+    protected void shouldUpdateIndex(String indexName, Integer newVersion, ActionListener<Boolean> listener) {
         IndexMetadata indexMetaData = clusterService.state().getMetadata().indices().get(indexName);
         if (indexMetaData == null) {
             listener.onResponse(Boolean.FALSE);

diff --git a/src/test/java/org/opensearch/flowframework/indices/GlobalContextHandlerTests.java b/src/test/java/org/opensearch/flowframework/indices/GlobalContextHandlerTests.java
@@ -34,7 +34,12 @@
 import org.mockito.MockitoAnnotations;
 
 import static org.opensearch.flowframework.common.CommonValue.GLOBAL_CONTEXT_INDEX;
-import static org.mockito.Mockito.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 public class GlobalContextHandlerTests extends OpenSearchTestCase {
     @Mock

diff --git a/src/test/java/org/opensearch/flowframework/workflow/CreateIndexStepTests.java b/src/test/java/org/opensearch/flowframework/workflow/CreateIndexStepTests.java
@@ -10,12 +10,14 @@
 
 import org.opensearch.action.admin.indices.create.CreateIndexRequest;
 import org.opensearch.action.admin.indices.create.CreateIndexResponse;
+import org.opensearch.action.admin.indices.mapping.put.PutMappingRequest;
 import org.opensearch.client.AdminClient;
 import org.opensearch.client.Client;
 import org.opensearch.client.IndicesAdminClient;
 import org.opensearch.cluster.ClusterName;
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.metadata.IndexMetadata;
+import org.opensearch.cluster.metadata.MappingMetadata;
 import org.opensearch.cluster.metadata.Metadata;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.settings.Settings;
@@ -25,7 +27,9 @@
 import org.opensearch.test.OpenSearchTestCase;
 import org.opensearch.threadpool.ThreadPool;
 
-import java.util.*;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -34,30 +38,35 @@
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
-import static org.opensearch.flowframework.common.CommonValue.*;
-import static org.mockito.Mockito.*;
+import static org.opensearch.flowframework.common.CommonValue.GLOBAL_CONTEXT_INDEX;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 public class CreateIndexStepTests extends OpenSearchTestCase {
 
-    private WorkflowData inputData = WorkflowData.EMPTY;
-
-    @Mock
-    private ClusterService clusterService;
+    private static final String META = "_meta";
+    private static final String SCHEMA_VERSION_FIELD = "schemaVersion";
 
+    private WorkflowData inputData = WorkflowData.EMPTY;
     private Client client;
-
     private AdminClient adminClient;
+    private CreateIndexStep createIndexStep;
+    private ThreadContext threadContext;
+    private Metadata metadata;
+    private Map<String, AtomicBoolean> indexMappingUpdated = new HashMap<>();
 
+    @Mock
+    private ClusterService clusterService;
     @Mock
     private IndicesAdminClient indicesAdminClient;
-    private CreateIndexStep createIndexStep;
-    private ThreadContext threadContext;
     @Mock
     private ThreadPool threadPool;
     @Mock
     IndexMetadata indexMetadata;
-    private Metadata metadata;
-    Map<String, AtomicBoolean> indexMappingUpdated = new HashMap<>();
 
     @Override
     public void setUp() throws Exception {
@@ -80,7 +89,6 @@ public void setUp() throws Exception {
 
         createIndexStep = new CreateIndexStep(clusterService, client);
         CreateIndexStep.indexMappingUpdated = indexMappingUpdated;
-
     }
 
     public void testCreateIndexStep() throws ExecutionException, InterruptedException {
@@ -122,24 +130,57 @@ public void testInitIndexIfAbsent_IndexNotPresent() {
         verify(indicesAdminClient).create(any(), any());
     }
 
-    // public void testInitIndexIfAbsent_IndexExist() {
-    // FlowFrameworkIndex index = FlowFrameworkIndex.GLOBAL_CONTEXT;
-    // indexMappingUpdated.put(index.getIndexName(), new AtomicBoolean(false));
-    //
-    // when(metadata.hasIndex(index.getIndexName())).thenReturn(true);
-    // when(metadata.indices()).thenReturn(Map.of(index.getIndexName(), indexMetadata));
-    //
-    // // Mock that the mapping's version is outdated, old version < new version
-    // when(indexMetadata.mapping()).thenReturn(new MappingMetadata(META, Map.of(SCHEMA_VERSION_FIELD, 0)));
-    //
-    // ActionListener<Boolean> listener = mock(ActionListener.class);
-    // createIndexStep.initIndexIfAbsent(index, listener);
-    //
-    // ArgumentCaptor<PutMappingRequest> captor = ArgumentCaptor.forClass(PutMappingRequest.class);
-    // verify(indicesAdminClient).putMapping(captor.capture());
-    //
-    // PutMappingRequest capturedRequest = captor.getValue();
-    // assertEquals(index.getIndexName(), capturedRequest.indices()[0]);
-    // }
+    public void testInitIndexIfAbsent_IndexExist() {
+        FlowFrameworkIndex index = FlowFrameworkIndex.GLOBAL_CONTEXT;
+        indexMappingUpdated.put(index.getIndexName(), new AtomicBoolean(false));
+
+        ClusterState mockClusterState = mock(ClusterState.class);
+        Metadata mockMetadata = mock(Metadata.class);
+        when(clusterService.state()).thenReturn(mockClusterState);
+        when(mockClusterState.metadata()).thenReturn(mockMetadata);
+        when(mockMetadata.hasIndex(index.getIndexName())).thenReturn(true);
+        ActionListener<Boolean> listener = mock(ActionListener.class);
+
+        IndexMetadata mockIndexMetadata = mock(IndexMetadata.class);
+        Map<String, IndexMetadata> mockIndices = mock(Map.class);
+        when(clusterService.state()).thenReturn(mockClusterState);
+        when(mockClusterState.getMetadata()).thenReturn(mockMetadata);
+        when(mockMetadata.indices()).thenReturn(mockIndices);
+        when(mockIndices.get(anyString())).thenReturn(mockIndexMetadata);
+        Map<String, Object> mockMapping = new HashMap<>();
+        Map<String, Object> mockMetaMapping = new HashMap<>();
+        mockMetaMapping.put(SCHEMA_VERSION_FIELD, 1);
+        mockMapping.put(META, mockMetaMapping);
+        MappingMetadata mockMappingMetadata = mock(MappingMetadata.class);
+        when(mockIndexMetadata.mapping()).thenReturn(mockMappingMetadata);
+        when(mockMappingMetadata.getSourceAsMap()).thenReturn(mockMapping);
+
+        createIndexStep.initIndexIfAbsent(index, listener);
+
+        ArgumentCaptor<PutMappingRequest> putMappingRequestArgumentCaptor = ArgumentCaptor.forClass(PutMappingRequest.class);
+        ArgumentCaptor<ActionListener> listenerCaptor = ArgumentCaptor.forClass(ActionListener.class);
+        verify(indicesAdminClient).putMapping(putMappingRequestArgumentCaptor.capture(), listenerCaptor.capture());
+        PutMappingRequest capturedRequest = putMappingRequestArgumentCaptor.getValue();
+        assertEquals(index.getIndexName(), capturedRequest.indices()[0]);
+    }
+
+    public void testInitIndexIfAbsent_IndexExist_returnFalse() {
+        FlowFrameworkIndex index = FlowFrameworkIndex.GLOBAL_CONTEXT;
+        indexMappingUpdated.put(index.getIndexName(), new AtomicBoolean(false));
+
+        ClusterState mockClusterState = mock(ClusterState.class);
+        Metadata mockMetadata = mock(Metadata.class);
+        when(clusterService.state()).thenReturn(mockClusterState);
+        when(mockClusterState.metadata()).thenReturn(mockMetadata);
+        when(mockMetadata.hasIndex(index.getIndexName())).thenReturn(true);
 
+        ActionListener<Boolean> listener = mock(ActionListener.class);
+        Map<String, IndexMetadata> mockIndices = mock(Map.class);
+        when(mockClusterState.getMetadata()).thenReturn(mockMetadata);
+        when(mockMetadata.indices()).thenReturn(mockIndices);
+        when(mockIndices.get(anyString())).thenReturn(null);
+
+        createIndexStep.initIndexIfAbsent(index, listener);
+        assertTrue(indexMappingUpdated.get(index.getIndexName()).get());
+    }
 }