This is an automated email from the ASF dual-hosted git repository.

pdallig pushed a commit to branch branch-0.12
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/branch-0.12 by this push:
     new 41f06a3ee0 [ZEPPELIN-6196] Improve and Add Unit Tests for 
Elasticsearch Client and Wrappers
41f06a3ee0 is described below

commit 41f06a3ee0c9a41bb290e982925a7c0a066699fc
Author: Gyeongtae Park <67095975+parkgyeong...@users.noreply.github.com>
AuthorDate: Fri Jun 27 05:19:23 2025 +0900

    [ZEPPELIN-6196] Improve and Add Unit Tests for Elasticsearch Client and 
Wrappers
    
    ### What is this PR for?
    This PR improves the test coverage for the Elasticsearch interpreter module 
by:
    - Refactoring and enhancing the existing unit tests in 
`ElasticsearchClientTypeTest` and `ElasticsearchClientTypeBuilderTest`
    - Adding new unit tests for `AggWrapper` and `HitWrapper`
    
    These changes help ensure better reliability and maintainability of 
Elasticsearch-related components in Apache Zeppelin.
    
    ### What type of PR is it?
    Improvement
    
    ### Todos
    * [x] - Refactor tests for ElasticsearchClientType and Builder
    * [x] - Add new tests for AggWrapper
    * [x] - Add new tests for HitWrapper
    
    ### What is the Jira issue?
    * Jira: https://issues.apache.org/jira/browse/ZEPPELIN-6196
    
    ### How should this be tested?
    The following test classes were added or modified:
    - `ElasticsearchClientTypeTest`
    - `ElasticsearchClientTypeBuilderTest`
    - `AggWrapperTest`
    - `HitWrapperTest`
    
    You can verify these tests by running:
    `./mvnw -pl elasticsearch test`
    
    ### Screenshots (if appropriate)
    N/A
    
    ### Questions:
    * Does the license files need to update? No.
    * Is there breaking changes for older versions? No.
    * Does this needs documentation? No.
    
    Closes #4942 from ParkGyeongTae/improve-elasticsearch-client-tests.
    
    Signed-off-by: Philipp Dallig <philipp.dal...@gmail.com>
    (cherry picked from commit 7a97ffe11da01faebf04a178bffdfa3c0ca0abe9)
    Signed-off-by: Philipp Dallig <philipp.dal...@gmail.com>
---
 .../elasticsearch/action/AggWrapperTest.java       | 70 +++++++++++++++++
 .../elasticsearch/action/HitWrapperTest.java       | 89 ++++++++++++++++++++++
 .../client/ElasticsearchClientTypeBuilderTest.java | 75 +++++-------------
 .../client/ElasticsearchClientTypeTest.java        | 44 +++++++----
 4 files changed, 206 insertions(+), 72 deletions(-)

diff --git 
a/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/action/AggWrapperTest.java
 
b/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/action/AggWrapperTest.java
new file mode 100644
index 0000000000..4ea2356f1d
--- /dev/null
+++ 
b/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/action/AggWrapperTest.java
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zeppelin.elasticsearch.action;
+
+import org.apache.zeppelin.elasticsearch.action.AggWrapper.AggregationType;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class AggWrapperTest {
+
+  @Test
+  @DisplayName("should store type and result correctly when type is SIMPLE")
+  void shouldStoreSimpleAggregationCorrectly() {
+    // Given
+    AggregationType type = AggregationType.SIMPLE;
+    String result = "{\"value\":42}";
+
+    // When
+    AggWrapper wrapper = new AggWrapper(type, result);
+
+    // Then
+    assertEquals(type, wrapper.getType());
+    assertEquals(result, wrapper.getResult());
+  }
+
+  @Test
+  @DisplayName("should store type and result correctly when type is 
MULTI_BUCKETS")
+  void shouldStoreMultiBucketsAggregationCorrectly() {
+    // Given
+    String result = "[{\"key\":\"a\"},{\"key\":\"b\"}]";
+
+    // When
+    AggWrapper wrapper = new AggWrapper(AggregationType.MULTI_BUCKETS, result);
+
+    // Then
+    assertEquals(AggregationType.MULTI_BUCKETS, wrapper.getType());
+    assertEquals(result, wrapper.getResult());
+  }
+
+  @Test
+  @DisplayName("AggregationType should contain SIMPLE and MULTI_BUCKETS in 
order")
+  void shouldContainExpectedAggregationTypes() {
+    // Given
+    AggregationType[] expected = {
+        AggregationType.SIMPLE,
+        AggregationType.MULTI_BUCKETS
+    };
+
+    // Then
+    assertArrayEquals(expected, AggregationType.values());
+  }
+}
diff --git 
a/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/action/HitWrapperTest.java
 
b/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/action/HitWrapperTest.java
new file mode 100644
index 0000000000..e90194d5f6
--- /dev/null
+++ 
b/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/action/HitWrapperTest.java
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zeppelin.elasticsearch.action;
+
+import com.google.gson.JsonObject;
+import com.google.gson.JsonSyntaxException;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+class HitWrapperTest {
+
+  private final String validJson = "{\"foo\":\"bar\",\"num\":42}";
+
+  @Test
+  @DisplayName("should store index, type, id, and source correctly")
+  void shouldStoreFieldsCorrectly() {
+    // Given
+    String index = "my-index";
+    String type = "_doc";
+    String id = "123";
+
+    // When
+    HitWrapper hit = new HitWrapper(index, type, id, validJson);
+
+    // Then
+    assertEquals(index, hit.getIndex());
+    assertEquals(type, hit.getType());
+    assertEquals(id, hit.getId());
+    assertEquals(validJson, hit.getSourceAsString());
+  }
+
+  @Test
+  @DisplayName("should parse valid JSON source into JsonObject")
+  void shouldParseSourceAsJsonObject() {
+    // Given
+    HitWrapper hit = new HitWrapper(validJson);
+
+    // When
+    JsonObject json = hit.getSourceAsJsonObject();
+
+    // Then
+    assertEquals("bar", json.get("foo").getAsString());
+    assertEquals(42, json.get("num").getAsInt());
+  }
+
+  @Test
+  @DisplayName("should allow construction with source only (null 
index/type/id)")
+  void shouldSupportConstructorWithSourceOnly() {
+    // Given
+    HitWrapper hit = new HitWrapper(validJson);
+
+    // Then
+    assertNull(hit.getIndex());
+    assertNull(hit.getType());
+    assertNull(hit.getId());
+    assertEquals(validJson, hit.getSourceAsString());
+  }
+
+  @Test
+  @DisplayName("should throw JsonSyntaxException for invalid JSON")
+  void shouldThrowForInvalidJsonSource() {
+    // Given
+    String invalidJson = "{not_json}";
+    HitWrapper hit = new HitWrapper(invalidJson);
+
+    // Then
+    assertThrows(JsonSyntaxException.class, hit::getSourceAsJsonObject,
+        "Invalid JSON string should throw JsonSyntaxException");
+  }
+}
diff --git 
a/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/client/ElasticsearchClientTypeBuilderTest.java
 
b/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/client/ElasticsearchClientTypeBuilderTest.java
index 39b019cd11..03fca638f1 100644
--- 
a/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/client/ElasticsearchClientTypeBuilderTest.java
+++ 
b/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/client/ElasticsearchClientTypeBuilderTest.java
@@ -17,69 +17,30 @@
 
 package org.apache.zeppelin.elasticsearch.client;
 
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 
-import static 
org.apache.zeppelin.elasticsearch.client.ElasticsearchClientType.HTTP;
-import static 
org.apache.zeppelin.elasticsearch.client.ElasticsearchClientType.HTTPS;
-import static 
org.apache.zeppelin.elasticsearch.client.ElasticsearchClientType.TRANSPORT;
-import static 
org.apache.zeppelin.elasticsearch.client.ElasticsearchClientType.UNKNOWN;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
-import org.junit.jupiter.api.Test;
-
 class ElasticsearchClientTypeBuilderTest {
 
-  @Test
-  void it_should_return_transport_as_default_value_when_property_is_empty() {
-    //GIVEN
-    String empty = "";
-    //WHEN
-    ElasticsearchClientType clientType =
-        ElasticsearchClientTypeBuilder.withPropertyValue(empty).build();
-    //THEN
-    assertEquals(TRANSPORT, clientType);
-  }
-
-  @Test
-  void it_should_return_transport_as_default_value_when_property_is_null() {
-    //GIVEN
-    String nullValue = null;
-    //WHEN
-    ElasticsearchClientType clientType =
-        ElasticsearchClientTypeBuilder.withPropertyValue(nullValue).build();
-    //THEN
-    assertEquals(TRANSPORT, clientType);
-  }
-
-  @Test
-  void it_should_return_client_type_when_property_value_exists() {
-    //GIVEN
-    String clientType = "https";
-    //WHEN
-    ElasticsearchClientType esClientType =
-        ElasticsearchClientTypeBuilder.withPropertyValue(clientType).build();
-    //THEN
-    assertEquals(HTTPS, esClientType);
-  }
-
-  @Test
-  void 
it_should_return_client_type_and_ignore_case_when_property_value_exists() {
-    //GIVEN
-    String clientType = "hTtP";
-    //WHEN
-    ElasticsearchClientType esClientType =
-        ElasticsearchClientTypeBuilder.withPropertyValue(clientType).build();
-    //THEN
-    assertEquals(HTTP, esClientType);
+  private ElasticsearchClientType buildFrom(String propertyValue) {
+    return 
ElasticsearchClientTypeBuilder.withPropertyValue(propertyValue).build();
   }
 
-  @Test
-  void it_should_return_unknown_when_property_value_does_not_exist() {
-    //GIVEN
-    String unknownValue = "an_unknown_value";
-    //WHEN
-    ElasticsearchClientType esClientType =
-        ElasticsearchClientTypeBuilder.withPropertyValue(unknownValue).build();
-    //THEN
-    assertEquals(UNKNOWN, esClientType);
+  @ParameterizedTest(name = "property = \"{0}\" → expected = {1}")
+  @CsvSource({
+      "'', TRANSPORT",
+      "null, TRANSPORT",
+      "https, HTTPS",
+      "hTtP, HTTP",
+      "an_unknown_value, UNKNOWN"
+  })
+  @DisplayName("should resolve correct ElasticsearchClientType from property 
string")
+  void shouldResolveClientTypeFromProperty(String property, 
ElasticsearchClientType expected) {
+    String resolved = "null".equals(property) ? null : property;
+    assertEquals(expected, buildFrom(resolved),
+        String.format("Expected %s for input '%s'", expected, property));
   }
 }
diff --git 
a/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/client/ElasticsearchClientTypeTest.java
 
b/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/client/ElasticsearchClientTypeTest.java
index 9ff921627b..7260d22f30 100644
--- 
a/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/client/ElasticsearchClientTypeTest.java
+++ 
b/elasticsearch/src/test/java/org/apache/zeppelin/elasticsearch/client/ElasticsearchClientTypeTest.java
@@ -17,27 +17,41 @@
 
 package org.apache.zeppelin.elasticsearch.client;
 
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
 
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
+class ElasticsearchClientTypeTest {
 
-import org.junit.jupiter.api.Test;
+  @ParameterizedTest
+  @EnumSource(value = ElasticsearchClientType.class, names = {"HTTP", "HTTPS"})
+  @DisplayName("should be marked as HTTP-based when client type is HTTP or 
HTTPS")
+  void shouldBeHttpWhenTypeIsHttpOrHttps(ElasticsearchClientType type) {
+    assertTrue(type.isHttp(), type + " should be marked as HTTP-based");
+  }
 
-class ElasticsearchClientTypeTest {
+  @ParameterizedTest
+  @EnumSource(value = ElasticsearchClientType.class, names = {"TRANSPORT", 
"UNKNOWN"})
+  @DisplayName("should NOT be marked as HTTP-based when client type is 
TRANSPORT or UNKNOWN")
+  void shouldNotBeHttpWhenTypeIsTransportOrUnknown(ElasticsearchClientType 
type) {
+    assertFalse(type.isHttp(), type + " should NOT be marked as HTTP-based");
+  }
 
   @Test
-  void it_should_return_http_when_reducing_on_http_types() {
-    //GIVEN
-    List<ElasticsearchClientType> httpTypes =
-        new ArrayList<>(Arrays.asList(ElasticsearchClientType.HTTP, 
ElasticsearchClientType.HTTPS));
-    //WHEN
-    Boolean httpTypesReduced = httpTypes.stream()
-        .map(ElasticsearchClientType::isHttp)
-        .reduce(true, (ident, elasticsearchClientType) -> ident && 
elasticsearchClientType);
-    //THEN
-    assertTrue(httpTypesReduced);
+  @DisplayName("should contain all expected enum values in order")
+  void shouldContainAllEnumValuesInOrder() {
+    ElasticsearchClientType[] expected = {
+        ElasticsearchClientType.HTTP,
+        ElasticsearchClientType.HTTPS,
+        ElasticsearchClientType.TRANSPORT,
+        ElasticsearchClientType.UNKNOWN
+    };
+    assertArrayEquals(expected, ElasticsearchClientType.values(), "Enum values 
should match " +
+        "declared order");
   }
 }

Reply via email to