Jiabao-Sun commented on code in PR #19811:
URL: https://github.com/apache/flink/pull/19811#discussion_r1735459879
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/hashtable/BinaryHashTableTest.java:
##########
@@ -56,7 +57,7 @@
import static org.assertj.core.api.Assertions.fail;
/** Hash table it case for binary row. */
-@RunWith(Parameterized.class)
+@ExtendWith(ParameterizedTestExtension.class)
public class BinaryHashTableTest {
Review Comment:
How about moving the inner class `ConstantsKeyValuePairsIterator` to
`org.apache.flink.table.runtime.util` so the visibility of this class can be
package private.
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/data/BinaryStringDataTest.java:
##########
@@ -74,7 +75,7 @@ public BinaryStringDataTest(Mode mode) {
this.mode = mode;
}
- @Parameterized.Parameters(name = "{0}")
+ @Parameters(name = "mode-{0}")
public static List<Mode> getVarSeg() {
Review Comment:
NIT: the modifier can be private here
```suggestion
private static List<Mode> getVarSeg() {
```
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/operators/join/window/WindowJoinOperatorTest.java:
##########
@@ -75,13 +76,13 @@ public WindowJoinOperatorTest(ZoneId shiftTimeZone) {
this.shiftTimeZone = shiftTimeZone;
}
- @Parameterized.Parameters(name = "TimeZone = {0}")
+ @Parameters(name = "TimeZone = {0}")
public static Collection<Object[]> runMode() {
Review Comment:
NIT: can be private here
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/operators/join/RandomSortMergeInnerJoinTest.java:
##########
@@ -80,13 +81,13 @@ public RandomSortMergeInnerJoinTest(boolean leftIsSmall) {
this.leftIsSmall = leftIsSmall;
}
- @Parameterized.Parameters
+ @Parameters(name = "leftIsSmaller={0}")
public static Collection<Boolean> parameters() {
Review Comment:
NIT: can be private here
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/types/DataTypePrecisionFixerTest.java:
##########
@@ -49,10 +48,8 @@
import static org.assertj.core.api.Assertions.assertThat;
/** Tests for {@link DataTypePrecisionFixer}. */
-@RunWith(Parameterized.class)
public class DataTypePrecisionFixerTest {
- @Parameterized.Parameters(name = "{index}: [From: {0}, To: {1}]")
public static List<TestSpec> testData() {
Review Comment:
```suggestion
private static List<TestSpec> testData() {
```
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/hashtable/BinaryHashTableTest.java:
##########
@@ -70,28 +71,28 @@ public BinaryHashTableTest(boolean useCompress) {
this.useCompress = useCompress;
}
- @Parameterized.Parameters(name = "useCompress-{0}")
+ @Parameters(name = "useCompress-{0}")
public static List<Boolean> getVarSeg() {
Review Comment:
NIT: can be private here.
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/hashtable/BinaryHashTableTest.java:
##########
@@ -772,7 +773,7 @@ public void testSparseProbeSpillingWithOuterJoin() throws
IOException {
* This test validates a bug fix against former memory loss in the case
where a partition was spilled
* during an insert into the same.
*/
- @Test
+ @TestTemplate
public void validateSpillingDuringInsertion() throws IOException,
MemoryAllocationException {
Review Comment:
```suggestion
void validateSpillingDuringInsertion() throws Exception {
```
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/operators/sort/BinaryExternalSorterTest.java:
##########
@@ -72,7 +73,7 @@ public BinaryExternalSorterTest(boolean spillCompress,
boolean asyncMerge) {
}
}
- @Parameterized.Parameters(name = "spillCompress-{0} asyncMerge-{1}")
+ @Parameters(name = "spillCompress-{0} asyncMerge-{1}")
Review Comment:
NIT: can be private
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/operators/join/AsyncLookupJoinHarnessTest.java:
##########
@@ -75,16 +76,16 @@
import static org.assertj.core.api.Assertions.fail;
/** Harness tests for {@link LookupJoinRunner} and {@link
LookupJoinWithCalcRunner}. */
-@RunWith(Parameterized.class)
-public class AsyncLookupJoinHarnessTest {
+@ExtendWith(ParameterizedTestExtension.class)
+class AsyncLookupJoinHarnessTest {
private static final int ASYNC_BUFFER_CAPACITY = 100;
private static final int ASYNC_TIMEOUT_MS = 3000;
- @Parameterized.Parameter public boolean orderedResult;
+ public boolean orderedResult;
Review Comment:
NIT: can be private
```suggestion
private boolean orderedResult;
```
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/types/LogicalTypeAssignableTest.java:
##########
@@ -63,10 +60,8 @@
import static org.assertj.core.api.Assertions.assertThat;
/** Tests for {@link PlannerTypeUtils#isAssignable(LogicalType, LogicalType)}.
*/
-@RunWith(Parameterized.class)
public class LogicalTypeAssignableTest {
- @Parameters(name = "{index}: [{0} COMPATIBLE {1} => {2}")
public static List<Object[]> testData() {
Review Comment:
```suggestion
private static List<Object[]> testData() {
```
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/operators/rank/window/WindowRankOperatorTest.java:
##########
@@ -127,7 +128,7 @@ public WindowRankOperatorTest(ZoneId shiftTimeZone) {
this.shiftTimeZone = shiftTimeZone;
}
- @Parameterized.Parameters(name = "TimeZone = {0}")
+ @Parameters(name = "TimeZone = {0}")
public static Collection<Object[]> runMode() {
Review Comment:
NIT: can be private
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/operators/window/tvf/slicing/WindowedSliceAssignerTest.java:
##########
@@ -31,22 +33,22 @@
import static org.assertj.core.api.Assertions.assertThat;
/** Tests for {@link SliceAssigners.WindowedSliceAssigner}. */
-@RunWith(Parameterized.class)
-public class WindowedSliceAssignerTest extends SliceAssignerTestBase {
+@ExtendWith(ParameterizedTestExtension.class)
+class WindowedSliceAssignerTest extends SliceAssignerTestBase {
- @Parameterized.Parameter public ZoneId shiftTimeZone;
+ public ZoneId shiftTimeZone;
Review Comment:
```suggestion
private ZoneId shiftTimeZone;
```
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/types/DataTypePrecisionFixerTest.java:
##########
@@ -49,10 +48,8 @@
import static org.assertj.core.api.Assertions.assertThat;
/** Tests for {@link DataTypePrecisionFixer}. */
-@RunWith(Parameterized.class)
public class DataTypePrecisionFixerTest {
Review Comment:
```suggestion
class DataTypePrecisionFixerTest {
```
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/operators/deduplicate/window/RowTimeWindowDeduplicateOperatorTest.java:
##########
@@ -94,7 +95,7 @@ public RowTimeWindowDeduplicateOperatorTest(ZoneId
shiftTimeZone) {
this.shiftTimeZone = shiftTimeZone;
}
- @Parameterized.Parameters(name = "TimeZone = {0}")
+ @Parameters(name = "TimeZone = {0}")
public static Collection<Object[]> runMode() {
Review Comment:
NIT: can be private here
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/operators/window/tvf/operator/UnalignedWindowTableFunctionOperatorTest.java:
##########
@@ -48,15 +49,20 @@
import static org.assertj.core.api.Assertions.assertThat;
/** Test for {@link UnalignedWindowTableFunctionOperator}. */
-@RunWith(Parameterized.class)
-public class UnalignedWindowTableFunctionOperatorTest extends
WindowTableFunctionOperatorTestBase {
+@ExtendWith(ParameterizedTestExtension.class)
+class UnalignedWindowTableFunctionOperatorTest extends
WindowTableFunctionOperatorTestBase {
- public UnalignedWindowTableFunctionOperatorTest(ZoneId shiftTimeZone) {
+ UnalignedWindowTableFunctionOperatorTest(ZoneId shiftTimeZone) {
super(shiftTimeZone);
}
- @Test
- public void testEventTimeSessionWindows() throws Exception {
+ @Parameters(name = "TimeZone = {0}")
+ public static Collection<Object[]> runMode() {
Review Comment:
NIT: can be private
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/util/collections/binary/BytesHashMapTestBase.java:
##########
@@ -37,7 +37,7 @@
import org.apache.flink.table.types.logical.SmallIntType;
import org.apache.flink.table.types.logical.VarCharType;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
Review Comment:
`public abstract class BytesHashMapTestBase` can be package private.
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/types/DataTypePrecisionFixerTest.java:
##########
@@ -114,10 +111,9 @@ public static List<TestSpec> testData() {
.bridgedTo(Time.class))))));
}
- @Parameterized.Parameter public TestSpec testSpec;
-
- @Test
- public void testPrecisionFixing() {
+ @ParameterizedTest(name = "{index}: [From: {0}, To: {1}]")
+ @MethodSource("testData")
+ public void testPrecisionFixing(final TestSpec testSpec) {
Review Comment:
```suggestion
void testPrecisionFixing(final TestSpec testSpec) {
```
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/types/LogicalTypeAssignableTest.java:
##########
@@ -63,10 +60,8 @@
import static org.assertj.core.api.Assertions.assertThat;
/** Tests for {@link PlannerTypeUtils#isAssignable(LogicalType, LogicalType)}.
*/
-@RunWith(Parameterized.class)
public class LogicalTypeAssignableTest {
Review Comment:
```suggestion
class LogicalTypeAssignableTest {
```
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/types/LogicalTypeAssignableTest.java:
##########
@@ -206,16 +201,10 @@ public static List<Object[]> testData() {
});
}
- @Parameter public LogicalType sourceType;
-
- @Parameter(1)
- public LogicalType targetType;
-
- @Parameter(2)
- public boolean equals;
-
- @Test
- public void testAreTypesCompatible() {
+ @ParameterizedTest(name = "{index}: [{0} COMPATIBLE {1} => {2}")
+ @MethodSource("testData")
+ public void testAreTypesCompatible(
Review Comment:
```suggestion
void testAreTypesCompatible(
```
--
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]
