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]