You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by pd...@apache.org on 2025/06/26 20:19:50 UTC
(zeppelin) branch branch-0.12 updated: [ZEPPELIN-6196] Improve and Add Unit Tests for Elasticsearch Client and Wrappers
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 <67...@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 <ph...@gmail.com>
(cherry picked from commit 7a97ffe11da01faebf04a178bffdfa3c0ca0abe9)
Signed-off-by: Philipp Dallig <ph...@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");
}
}