sijie commented on a change in pull request #2078: Enhance Pulsar cluster with
container logs
URL: https://github.com/apache/incubator-pulsar/pull/2078#discussion_r200437290
##########
File path:
tests/integration-tests-topologies/src/main/java/org/apache/pulsar/tests/containers/PulsarContainer.java
##########
@@ -24,13 +24,14 @@
import java.util.Objects;
import lombok.extern.slf4j.Slf4j;
import org.apache.pulsar.tests.DockerUtils;
+import org.testcontainers.containers.output.Slf4jLogConsumer;
import org.testcontainers.containers.wait.strategy.HostPortWaitStrategy;
/**
* Abstract Test Container for Pulsar.
*/
@Slf4j
-public abstract class PulsarContainer<SelfT extends PulsarContainer<SelfT>>
extends ChaosContainer<SelfT> {
+public class PulsarContainer<SELF extends PulsarContainer<SELF>> extends
ChaosContainer<SELF> {
Review comment:
1) Ideally please separate all these kind of style code changes in future
pull requests.
2) SelfT is the style we are using in bookkeeper & pulsar.
https://github.com/apache/incubator-pulsar/blob/master/buildtools/src/main/resources/pulsar/checkstyle.xml#L268
although we haven't fully turned on checkstyle on pulsar, it would be better
to stick to them if possible
3) I am not sure why do you remove `abstract` here. ideally the testing code
should not instantiate a pulsar container directly. all testing code should use
it is inherited one.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
