reta commented on code in PR #105:
URL: 
https://github.com/apache/flink-connector-elasticsearch/pull/105#discussion_r1618900785


##########
flink-connector-elasticsearch8/src/test/java/org/apache/flink/connector/elasticsearch/sink/ElasticsearchSinkBaseITCase.java:
##########
@@ -21,86 +21,144 @@
 
 package org.apache.flink.connector.elasticsearch.sink;
 
+import org.apache.flink.connector.base.sink.writer.ElementConverter;
+import org.apache.flink.testutils.junit.extensions.parameterized.Parameter;
+import 
org.apache.flink.testutils.junit.extensions.parameterized.ParameterizedTestExtension;
+import org.apache.flink.testutils.junit.extensions.parameterized.Parameters;
+
+import co.elastic.clients.elasticsearch.core.bulk.BulkOperationVariant;
+import co.elastic.clients.elasticsearch.core.bulk.IndexOperation;
 import org.apache.http.HttpHost;
+import org.apache.http.auth.AuthScope;
+import org.apache.http.auth.UsernamePasswordCredentials;
+import org.apache.http.client.CredentialsProvider;
+import org.apache.http.impl.client.BasicCredentialsProvider;
 import org.apache.http.util.EntityUtils;
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.Response;
 import org.elasticsearch.client.RestClient;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testcontainers.containers.output.Slf4jLogConsumer;
+import org.testcontainers.containers.wait.strategy.LogMessageWaitStrategy;
 import org.testcontainers.containers.wait.strategy.Wait;
 import org.testcontainers.elasticsearch.ElasticsearchContainer;
-import org.testcontainers.junit.jupiter.Container;
-import org.testcontainers.junit.jupiter.Testcontainers;
 import org.testcontainers.utility.DockerImageName;
 
 import java.io.IOException;
 import java.time.Duration;
+import java.util.Arrays;
+import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-/** Base Integration tests class. */
-@Testcontainers
-public class ElasticsearchSinkBaseITCase {
+/**
+ * {@link ElasticsearchSinkBaseITCase} is the base class for integration tests.
+ *
+ * <p>It is extended with the {@link ParameterizedTestExtension} for 
parameterized testing against
+ * secure and non-secure Elasticsearch clusters. Tests must be annotated by 
{@link TestTemplate} in
+ * order to be parameterized.
+ *
+ * <p>The cluster is running via test containers. In order to reuse the 
singleton containers by all
+ * inheriting test classes, we manage their lifecycle. The two containers are 
started only once when
+ * this class is loaded. At the end of the test suite the Ryuk container that 
is started by
+ * Testcontainers core will take care of stopping the singleton container.
+ */
+@ExtendWith(ParameterizedTestExtension.class)

Review Comment:
   Thanks @liuml07 , I think the usage of 
   
   ```suggestion
   @ExtendWith(ParameterizedTestExtension.class)
   ```
   
   is not really bringing value here since the test cases depend on static 
containers (and  parameterized tests do not support that). However, I think you 
have quite interesting idea to explore, I would suggest to try that:
    - create 2 containers (as you do now but using the `@Testcontainers` 
extension):
       ```
       @Container
        public static final ElasticsearchContainer ES_CONTAINER = 
createElasticsearchContainer();
   
       @Container
       public static final ElasticsearchContainer ES_SECURE_CONTAINER = 
createElasticsearchSecureContainer();
       ```
    - consequently, use 2 `RestClient` clients, secure and non-secure
    - introduce abstract method, fe `abstract boolean enableSecurity()` so each 
test case would have to override and the base test case could use to make the 
decision which client  + customization to use
   
   WDYT? Thank you.
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to