liuml07 commented on code in PR #104:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/104#discussion_r1613758123
##########
flink-connector-elasticsearch8/src/test/java/org/apache/flink/connector/elasticsearch/sink/Elasticsearch8TestUtils.java:
##########
@@ -0,0 +1,69 @@
+package org.apache.flink.connector.elasticsearch.sink;
+
+import org.apache.http.util.EntityUtils;
+import org.elasticsearch.client.Request;
+import org.elasticsearch.client.Response;
+import org.elasticsearch.client.RestClient;
+import org.testcontainers.utility.DockerImageName;
+
+import java.io.IOException;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/** Utility class for Elasticsearch8 tests. */
+public class Elasticsearch8TestUtils {
Review Comment:
Indeed the parameterized tests will reuse the code at the max level and
still keep separate cases/parameters independent. I thought of this idea
previously but realized the `ES_CONTAINER` field is static (to amortize cost of
container setup) and annotated `@Container` for managed lifecycle.
When parameterized, there will be multiple if-else checks depending on the
parameter in the base class and child test classes, mainly for ES client and
sink builder. This is not a problem per se, and just needs a bit more
refactoring.
I'll move the fields / static methods back to the base class for now, and
take another look at parameterized tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
