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/17 21:43:55 UTC
[GitHub] [flink] snuyanzin opened a new pull request, #19753: [FLINK-27672][tests][table] Migrate flink-table-common to JUnit5
snuyanzin opened a new pull request, #19753:
URL: https://github.com/apache/flink/pull/19753
## What is the purpose of the change
Update the flink-table-common 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] flinkbot commented on pull request #19753: [FLINK-27672][tests][table] Migrate flink-table-common to JUnit5
Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #19753:
URL: https://github.com/apache/flink/pull/19753#issuecomment-1129347186
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "af3374e392677cc6ab2161dbd25185e0215999d5",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "af3374e392677cc6ab2161dbd25185e0215999d5",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* af3374e392677cc6ab2161dbd25185e0215999d5 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
[GitHub] [flink] zentol commented on a diff in pull request #19753: [FLINK-27672][tests][table] Migrate flink-table-common to JUnit5
Posted by GitBox <gi...@apache.org>.
zentol commented on code in PR #19753:
URL: https://github.com/apache/flink/pull/19753#discussion_r876855264
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/LogicalTypeDuplicatorTest.java:
##########
@@ -34,53 +34,46 @@
import org.apache.flink.table.types.logical.VarCharType;
import org.apache.flink.table.types.logical.utils.LogicalTypeDuplicator;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameter;
-import org.junit.runners.Parameterized.Parameters;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import java.util.Arrays;
import java.util.Collections;
-import java.util.List;
+import java.util.stream.Stream;
import static org.assertj.core.api.Assertions.assertThat;
/** Tests for {@link LogicalTypeDuplicator}. */
-@RunWith(Parameterized.class)
-public class LogicalTypeDuplicatorTest {
+class LogicalTypeDuplicatorTest {
private static final LogicalTypeDuplicator DUPLICATOR = new LogicalTypeDuplicator();
private static final LogicalTypeDuplicator INT_REPLACER = new IntReplacer();
- @Parameters(name = "{index}: {0}")
- public static List<Object[]> testData() {
- return Arrays.asList(
- new Object[][] {
- {new CharType(2), new CharType(2)},
- {createMultisetType(new IntType()), createMultisetType(new BigIntType())},
- {createArrayType(new IntType()), createArrayType(new BigIntType())},
- {createMapType(new IntType()), createMapType(new BigIntType())},
- {createRowType(new IntType()), createRowType(new BigIntType())},
- {createDistinctType(new IntType()), createDistinctType(new BigIntType())},
- {createUserType(new IntType()), createUserType(new BigIntType())},
- {createHumanType(), createHumanType()}
- });
+ static Stream<Arguments> testData() {
Review Comment:
private
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTestBase.java:
##########
@@ -45,39 +43,39 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;
/** Base class for testing {@link InputTypeStrategy}. */
-@RunWith(Parameterized.class)
public abstract class InputTypeStrategiesTestBase {
- @Parameterized.Parameter public TestSpec testSpec;
-
- @Test
- public void testStrategy() {
+ @ParameterizedTest
Review Comment:
```suggestion
```
Since the method is called from the sub-classes.
Although I'm curious; what would happen if this method had a `@MethodSource`, but the method only exists in sub-classes?
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTestBase.java:
##########
@@ -45,39 +43,39 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;
/** Base class for testing {@link InputTypeStrategy}. */
-@RunWith(Parameterized.class)
public abstract class InputTypeStrategiesTestBase {
- @Parameterized.Parameter public TestSpec testSpec;
-
- @Test
- public void testStrategy() {
+ @ParameterizedTest
Review Comment:
What we could also try is using Lifecycle.PER_CLASS and an abstract `testData()` method that sub-classes implement.
--
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 a diff in pull request #19753: [FLINK-27672][tests][table] Migrate flink-table-common to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19753:
URL: https://github.com/apache/flink/pull/19753#discussion_r875285230
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/descriptors/DescriptorTestBase.java:
##########
@@ -1,85 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.flink.table.descriptors;
-
-import org.apache.flink.util.Preconditions;
-
-import org.junit.Test;
-
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-
-import static org.assertj.core.api.Assertions.assertThat;
-
-/** Test base for testing {@link Descriptor} together with {@link DescriptorValidator}. */
-public abstract class DescriptorTestBase {
Review Comment:
Removed as it is abstract, no successors, no usage
--
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 a diff in pull request #19753: [FLINK-27672][tests][table] Migrate flink-table-common to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19753:
URL: https://github.com/apache/flink/pull/19753#discussion_r876422680
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/ComparableInputTypeStrategyTest.java:
##########
@@ -33,24 +33,30 @@
import org.apache.flink.table.types.logical.StructuredType;
import org.apache.flink.table.types.logical.StructuredType.StructuredComparison;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
import javax.annotation.Nonnull;
import java.lang.reflect.Field;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
+import java.util.stream.Stream;
-import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;
/** Tests for {@link ComparableTypeStrategy}. */
-public class ComparableInputTypeStrategyTest extends InputTypeStrategiesTestBase {
+class ComparableInputTypeStrategyTest extends InputTypeStrategiesTestBase {
- @Parameterized.Parameters(name = "{index}: {0}")
- public static List<TestSpec> testData() {
- return asList(
+ @ParameterizedTest(name = "{index}: {0}")
+ @MethodSource("testData")
+ protected void testStrategy(TestSpec testSpec) {
+ super.testStrategy(testSpec);
+ }
Review Comment:
This allows to run parameterized tests with parameters generated in successor
--
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 a diff in pull request #19753: [FLINK-27672][tests][table] Migrate flink-table-common to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19753:
URL: https://github.com/apache/flink/pull/19753#discussion_r876347233
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTestBase.java:
##########
@@ -45,39 +44,49 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;
/** Base class for testing {@link InputTypeStrategy}. */
-@RunWith(Parameterized.class)
public abstract class InputTypeStrategiesTestBase {
- @Parameterized.Parameter public TestSpec testSpec;
-
- @Test
- public void testStrategy() {
+ @ParameterizedTest(name = "{index}: {0}")
+ @MethodSource(
+ value = {
+ "org.apache.flink.table.types.inference.ComparableInputTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.CurrentWatermarkInputTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.InputTypeStrategiesTest#testData",
+ "org.apache.flink.table.types.inference.strategies.RepeatingSequenceInputTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.SubsequenceInputTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.SymbolArgumentTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.TypeLiteralArgumentTypeStrategyTest#testData"
+ })
Review Comment:
It leads to cartesian of tests and input....
need to think how to cope with this
--
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 a diff in pull request #19753: [FLINK-27672][tests][table] Migrate flink-table-common to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19753:
URL: https://github.com/apache/flink/pull/19753#discussion_r876970913
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/strategies/CurrentWatermarkTypeStrategyTest.java:
##########
@@ -26,21 +26,12 @@
import org.apache.flink.table.types.logical.TimestampType;
import org.apache.flink.table.types.utils.TypeConversions;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.MethodSource;
-
import java.util.stream.Stream;
/** Tests for {@link CurrentWatermarkTypeStrategy}. */
class CurrentWatermarkTypeStrategyTest extends TypeStrategiesTestBase {
- @ParameterizedTest(name = "{index}: {0}")
- @MethodSource("testData")
- protected void testTypeStrategy(TestSpec testSpec) {
- super.testTypeStrategy(testSpec);
- }
-
- static Stream<TestSpec> testData() {
+ protected Stream<TestSpec> testData() {
Review Comment:
yes, you're right, thanks
added missing `@Override`
--
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 a diff in pull request #19753: [FLINK-27672][tests][table] Migrate flink-table-common to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19753:
URL: https://github.com/apache/flink/pull/19753#discussion_r876967473
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTestBase.java:
##########
@@ -45,39 +43,39 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;
/** Base class for testing {@link InputTypeStrategy}. */
-@RunWith(Parameterized.class)
public abstract class InputTypeStrategiesTestBase {
- @Parameterized.Parameter public TestSpec testSpec;
-
- @Test
- public void testStrategy() {
+ @ParameterizedTest
Review Comment:
Thanks for the hint
it seems working
--
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 #19753: [FLINK-27672][tests][table] Migrate flink-table-common to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on PR #19753:
URL: https://github.com/apache/flink/pull/19753#issuecomment-1129436869
@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 a diff in pull request #19753: [FLINK-27672][tests][table] Migrate flink-table-common to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19753:
URL: https://github.com/apache/flink/pull/19753#discussion_r876347664
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/TypeStrategiesTestBase.java:
##########
@@ -42,26 +40,36 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;
/** Base class for tests of {@link TypeStrategies}. */
-@RunWith(Parameterized.class)
public abstract class TypeStrategiesTestBase {
- @Parameter public TestSpec testSpec;
-
- @Test
- public void testTypeStrategy() {
+ @ParameterizedTest(name = "{index}: {0}")
+ @MethodSource(
+ value = {
+ "org.apache.flink.table.types.inference.strategies.ArrayTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.CurrentWatermarkTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.DecimalTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.GetTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.MapTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.MappingTypeStrategiesTest#testData",
+ "org.apache.flink.table.types.inference.strategies.RowTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.StringConcatTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.TypeStrategiesTest#testData"
+ })
Review Comment:
It leads to cartesian of tests and input....
need to think how to cope with this
--
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 #19753: [FLINK-27672][tests][table] Migrate flink-table-common to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on PR #19753:
URL: https://github.com/apache/flink/pull/19753#issuecomment-1130513388
I converted since to draft since I noticed an issue with tests `InputTypeStrategiesTestBase` and `TypeStrategiesTestBase` mentioned above
--
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 a diff in pull request #19753: [FLINK-27672][tests][table] Migrate flink-table-common to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19753:
URL: https://github.com/apache/flink/pull/19753#discussion_r876419257
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/TypeStrategiesTestBase.java:
##########
@@ -42,26 +40,36 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;
/** Base class for tests of {@link TypeStrategies}. */
-@RunWith(Parameterized.class)
public abstract class TypeStrategiesTestBase {
- @Parameter public TestSpec testSpec;
-
- @Test
- public void testTypeStrategy() {
+ @ParameterizedTest(name = "{index}: {0}")
+ @MethodSource(
+ value = {
+ "org.apache.flink.table.types.inference.strategies.ArrayTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.CurrentWatermarkTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.DecimalTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.GetTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.MapTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.MappingTypeStrategiesTest#testData",
+ "org.apache.flink.table.types.inference.strategies.RowTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.StringConcatTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.TypeStrategiesTest#testData"
+ })
Review Comment:
seems resolved with test overriding
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTestBase.java:
##########
@@ -45,39 +44,49 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;
/** Base class for testing {@link InputTypeStrategy}. */
-@RunWith(Parameterized.class)
public abstract class InputTypeStrategiesTestBase {
- @Parameterized.Parameter public TestSpec testSpec;
-
- @Test
- public void testStrategy() {
+ @ParameterizedTest(name = "{index}: {0}")
+ @MethodSource(
+ value = {
+ "org.apache.flink.table.types.inference.ComparableInputTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.CurrentWatermarkInputTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.InputTypeStrategiesTest#testData",
+ "org.apache.flink.table.types.inference.strategies.RepeatingSequenceInputTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.SubsequenceInputTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.SymbolArgumentTypeStrategyTest#testData",
+ "org.apache.flink.table.types.inference.strategies.TypeLiteralArgumentTypeStrategyTest#testData"
+ })
Review Comment:
seems resolved with test overriding
--
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] zentol commented on a diff in pull request #19753: [FLINK-27672][tests][table] Migrate flink-table-common to JUnit5
Posted by GitBox <gi...@apache.org>.
zentol commented on code in PR #19753:
URL: https://github.com/apache/flink/pull/19753#discussion_r876938657
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/strategies/CurrentWatermarkTypeStrategyTest.java:
##########
@@ -26,21 +26,12 @@
import org.apache.flink.table.types.logical.TimestampType;
import org.apache.flink.table.types.utils.TypeConversions;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.MethodSource;
-
import java.util.stream.Stream;
/** Tests for {@link CurrentWatermarkTypeStrategy}. */
class CurrentWatermarkTypeStrategyTest extends TypeStrategiesTestBase {
- @ParameterizedTest(name = "{index}: {0}")
- @MethodSource("testData")
- protected void testTypeStrategy(TestSpec testSpec) {
- super.testTypeStrategy(testSpec);
- }
-
- static Stream<TestSpec> testData() {
+ protected Stream<TestSpec> testData() {
Review Comment:
missing `@Override`; same in other classes ;)
--
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] zentol merged pull request #19753: [FLINK-27672][tests][table] Migrate flink-table-common to JUnit5
Posted by GitBox <gi...@apache.org>.
zentol merged PR #19753:
URL: https://github.com/apache/flink/pull/19753
--
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