[GitHub] [kafka] mimaison commented on a change in pull request #10299: KAFKA-10070: parameterize Connect unit tests to remove code duplication

[GitBox <gi...@apache.org>]

mimaison commented on a change in pull request #10299:
URL: https://github.com/apache/kafka/pull/10299#discussion_r593134903



##########
File path: connect/runtime/src/test/java/org/apache/kafka/connect/util/ParameterizedTest.java
##########
@@ -0,0 +1,67 @@
+package org.apache.kafka.connect.util;

Review comment:
       We're missing the license header here

##########
File path: connect/runtime/src/test/java/org/apache/kafka/connect/runtime/ErrorHandlingTaskTest.java
##########
@@ -16,6 +16,7 @@
  */
 package org.apache.kafka.connect.runtime;
 
+import java.util.Collection;

Review comment:
       Should we move this with the other `java.util` imports below?

##########
File path: checkstyle/suppressions.xml
##########
@@ -158,7 +158,7 @@
               files="StreamThread.java"/>
 
     <suppress checks="ClassDataAbstractionCoupling"
-              files="(KStreamImpl|KTableImpl).java"/>
+              files="(KStreamImpl|KTableImpl|WorkerSourceTaskTest).java"/>

Review comment:
       Instead of adding this to Kafka Streams rules, can we move it to the Connect section just above?

##########
File path: connect/runtime/src/test/java/org/apache/kafka/connect/util/ParameterizedTest.java
##########
@@ -0,0 +1,67 @@
+package org.apache.kafka.connect.util;
+
+import java.lang.annotation.Annotation;
+import org.junit.runner.Description;
+import org.junit.runner.manipulation.Filter;
+import org.junit.runner.manipulation.NoTestsRemainException;
+import org.junit.runners.Parameterized;
+
+/**
+ * Running a single parameterized test causes issue as explained in
+ * http://youtrack.jetbrains.com/issue/IDEA-65966 and
+ * https://stackoverflow.com/questions/12798079/initializationerror-with-eclipse-and-junit4-when-executing-a-single-test/18438718#18438718
+ *
+ * As a workaround, the original filter needs to be wrapped and then pass it a deparameterized
+ * description which removes the parameter part (See deparametrizeName)
+ */
+public class ParameterizedTest extends Parameterized {
+
+  public ParameterizedTest (Class<?> klass) throws Throwable {

Review comment:
       Checkstyle fails because it expects indentations to be 4 spaces




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org