You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/05/24 20:26:06 UTC
[GitHub] [flink] snuyanzin opened a new pull request, #19811: [FLINK-27758][tests][table] Migrate flink-table-runtime to JUnit5
snuyanzin opened a new pull request, #19811:
URL: https://github.com/apache/flink/pull/19811
## What is the purpose of the change
Update the flink-table-runtime module to AssertJ and JUnit 5 following the [JUnit 5 Migration Guide](https://docs.google.com/document/d/1514Wa_aNB9bJUen4xm5uiuXOooOJTtXqS_Jqk9KJitU/edit)
## Brief change log
Use JUnit5 and AssertJ in tests instead of JUnit4 and Hamcrest
## Verifying this change
This change is a code cleanup without any test coverage.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): ( no)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: ( no)
- The serializers: (no)
- The runtime per-record code paths (performance sensitive): (no)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
- The S3 file system connector: ( no)
## Documentation
- Does this pull request introduce a new feature? ( no)
- If yes, how is the feature documented? (not applicable)
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] snuyanzin commented on pull request #19811: [FLINK-27758][tests][table] Migrate flink-table-runtime to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on PR #19811:
URL: https://github.com/apache/flink/pull/19811#issuecomment-1136687056
@flinkbot run azure
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] snuyanzin commented on pull request #19811: [FLINK-27758][tests][table] Migrate flink-table-runtime to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on PR #19811:
URL: https://github.com/apache/flink/pull/19811#issuecomment-1136752374
The PR is based on commit https://github.com/apache/flink/pull/19780 migrating `TypeSerializerUpgradeTestBase` to JUnit5
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-27758][tests][table] Migrate flink-table-runtime to JUnit5 [flink]
Posted by "Jiabao-Sun (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #19811: [FLINK-27758][tests][table] Migrate flink-table-runtime to JUnit5
Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #19811:
URL: https://github.com/apache/flink/pull/19811#issuecomment-1136400172
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "55083d6ce49a1e36922d059e19bf463d0a375914",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "55083d6ce49a1e36922d059e19bf463d0a375914",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 55083d6ce49a1e36922d059e19bf463d0a375914 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-27758][tests][table] Migrate flink-table-runtime to JUnit5 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin merged PR #19811:
URL: https://github.com/apache/flink/pull/19811
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-27758][tests][table] Migrate flink-table-runtime to JUnit5 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on PR #19811:
URL: https://github.com/apache/flink/pull/19811#issuecomment-2320184795
thanks a lot for taking a look
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-27758][tests][table] Migrate flink-table-runtime to JUnit5 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on PR #19811:
URL: https://github.com/apache/flink/pull/19811#issuecomment-2316095327
@Jiabao-Sun since you are one of those who is active in junit4-junit5 migration
may I ask you to have a look here?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-27758][tests][table] Migrate flink-table-runtime to JUnit5 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #19811:
URL: https://github.com/apache/flink/pull/19811#discussion_r1736030307
##########
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:
this is actually a good idea, thanks
moved
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org