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/04/20 15:41:48 UTC
[GitHub] [flink] snuyanzin opened a new pull request, #19538: [FLINK-27323][flink-table-api-java] Migrate tests to JUnit5
snuyanzin opened a new pull request, #19538:
URL: https://github.com/apache/flink/pull/19538
## What is the purpose of the change
Update the flink-table/flink-table-api-java module to AssertJ and JUnit 5 following the [JUnit 5 Migration Guide](https://docs.google.com/document/d/1514Wa_aNB9bJUen4xm5uiuXOooOJTtXqS_Jqk9KJitU/edit)
Also it touches tests extending from _org.apache.flink.table.catalog.CatalogTest_
## 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] zentol commented on a diff in pull request #19538: [FLINK-27323][flink-table-api-java][tests] Migrate tests to JUnit5
Posted by GitBox <gi...@apache.org>.
zentol commented on code in PR #19538:
URL: https://github.com/apache/flink/pull/19538#discussion_r866661713
##########
flink-table/flink-table-api-java/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension:
##########
@@ -0,0 +1,16 @@
+# 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.
+
+org.apache.flink.util.TestLoggerExtension
Review Comment:
Deleting it is not what I meant :)
We do want this file to be there, but not be bundled in the test-jar by changing the maven-jar-plugin config. It then works for this module, but doesn't affect downstream modles.
--
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 #19538: [FLINK-27323][flink-table-api-java][tests] Migrate tests to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19538:
URL: https://github.com/apache/flink/pull/19538#discussion_r866753467
##########
flink-table/flink-table-api-java/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension:
##########
@@ -0,0 +1,16 @@
+# 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.
+
+org.apache.flink.util.TestLoggerExtension
Review Comment:
🙈, sorry probably didn't understand you.
I rolled back `TestLoggerExtension` and changed maven-jar-plugin config
--
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 #19538: [FLINK-27323][flink-table-api-java][tests] Migrate tests to JUnit5
Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #19538:
URL: https://github.com/apache/flink/pull/19538#issuecomment-1104089222
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5f48c9c1bb7c958c63ca00d245a312c4c9bdcc9a",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "5f48c9c1bb7c958c63ca00d245a312c4c9bdcc9a",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5f48c9c1bb7c958c63ca00d245a312c4c9bdcc9a 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 merged pull request #19538: [FLINK-27323][flink-table-api-java][tests] Migrate tests to JUnit5
Posted by GitBox <gi...@apache.org>.
zentol merged PR #19538:
URL: https://github.com/apache/flink/pull/19538
--
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 #19538: [FLINK-27323][flink-table-api-java][tests] Migrate tests to JUnit5
Posted by GitBox <gi...@apache.org>.
zentol commented on code in PR #19538:
URL: https://github.com/apache/flink/pull/19538#discussion_r865611214
##########
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/typeutils/FieldInfoUtilsTest.java:
##########
@@ -25,55 +25,63 @@
import org.apache.flink.api.java.typeutils.RowTypeInfo;
import org.apache.flink.table.expressions.Expression;
-import org.junit.Test;
-import org.junit.experimental.runners.Enclosed;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import java.sql.Timestamp;
import java.util.Arrays;
-import java.util.Collection;
+import java.util.stream.Stream;
import static org.apache.flink.table.api.Expressions.$;
import static org.assertj.core.api.Assertions.assertThat;
/** Test suite for {@link FieldInfoUtils}. */
-@RunWith(Enclosed.class)
-public class FieldInfoUtilsTest {
+class FieldInfoUtilsTest {
/** Test for ByNameMode. */
- @RunWith(Parameterized.class)
- public static final class TestByNameMode {
-
- @Parameterized.Parameters(name = "{0}")
- public static Collection<TypeInformation> parameters() throws Exception {
- return Arrays.asList(
- new RowTypeInfo(
- new TypeInformation[] {Types.INT, Types.LONG, Types.SQL_TIMESTAMP},
- new String[] {"f0", "f1", "f2"}),
- new PojoTypeInfo(
- MyPojo.class,
- Arrays.asList(
- new PojoField(MyPojo.class.getDeclaredField("f0"), Types.INT),
- new PojoField(MyPojo.class.getDeclaredField("f1"), Types.LONG),
- new PojoField(
- MyPojo.class.getDeclaredField("f2"),
- Types.SQL_TIMESTAMP))));
+ @Nested
+ @TestInstance(TestInstance.Lifecycle.PER_CLASS)
+ class TestByNameMode {
Review Comment:
Are these nested tests connected in any way? I'm wondering if we could just split them into 2 test cases.
##########
flink-table/flink-table-api-java/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension:
##########
@@ -0,0 +1,16 @@
+# 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.
+
+org.apache.flink.util.TestLoggerExtension
Review Comment:
this needs to be excluded from the test-jar as it otherwise leaks into several other JUnit4 modules.
##########
flink-table/flink-table-api-scala-bridge/src/test/scala/org/apache/flink/table/types/TypeInfoDataTypeConverterScalaTest.scala:
##########
@@ -23,54 +23,55 @@ import org.apache.flink.table.types.TypeInfoDataTypeConverterTest.TestSpec
import org.apache.flink.table.types.utils.DataTypeFactoryMock.dummyRaw
import org.apache.flink.table.types.utils.TypeInfoDataTypeConverter
-import org.hamcrest.CoreMatchers.equalTo
-import org.junit.Assert.assertThat
-import org.junit.Test
-import org.junit.runner.RunWith
-import org.junit.runners.Parameterized
-import org.junit.runners.Parameterized.Parameters
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.jupiter.params.ParameterizedTest
+import org.junit.jupiter.params.provider.{Arguments, MethodSource}
+
+import java.util.stream
/** Scala tests for [[TypeInfoDataTypeConverter]]. */
-@RunWith(classOf[Parameterized])
-class TypeInfoDataTypeConverterScalaTest(testSpec: TypeInfoDataTypeConverterTest.TestSpec) {
+class TypeInfoDataTypeConverterScalaTest {
- @Test
- def testScalaConversion(): Unit = {
+ @ParameterizedTest
+ @MethodSource(Array("testData"))
+ def testScalaConversion(testSpec: TestSpec): Unit = {
val dataType = TypeInfoDataTypeConverter.toDataType(testSpec.typeFactory, testSpec.typeInfo)
- assertThat(dataType, equalTo(testSpec.expectedDataType))
+ assertThat(dataType).isEqualTo(testSpec.expectedDataType)
}
+
}
object TypeInfoDataTypeConverterScalaTest {
-
- @Parameters
- def testData: Array[TestSpec] = Array(
- TestSpec
- .forType(createTypeInformation[ScalaCaseClass])
- .expectDataType(
- DataTypes
- .STRUCTURED(
- classOf[ScalaCaseClass],
- DataTypes.FIELD(
- "primitiveIntField",
- DataTypes.INT().notNull().bridgedTo(java.lang.Integer.TYPE)),
- DataTypes.FIELD("doubleField", DataTypes.DOUBLE().notNull()),
- DataTypes.FIELD("stringField", DataTypes.STRING().nullable())
- )
- .notNull()),
- TestSpec
- .forType(createTypeInformation[(java.lang.Double, java.lang.Float)])
- .expectDataType(
- DataTypes
- .STRUCTURED(
- classOf[(_, _)],
- DataTypes.FIELD("_1", DataTypes.DOUBLE().notNull()),
- DataTypes.FIELD("_2", DataTypes.FLOAT().notNull()))
- .notNull()),
- TestSpec
- .forType(createTypeInformation[Unit])
- .lookupExpects(classOf[Unit])
- .expectDataType(dummyRaw(classOf[Unit]))
+ def testData(): stream.Stream[Arguments] = java.util.stream.Stream.of(
Review Comment:
You don't need to wrap things in `Arguments` if there is only a single parameter.
##########
flink-connectors/flink-connector-hive/src/test/java/org/apache/flink/table/catalog/hive/HiveCatalogHiveMetadataTest.java:
##########
@@ -270,13 +267,18 @@ public void testCreateTableWithConstraints() throws Exception {
new CatalogTableImpl(builder.build(), getBatchTableProperties(), null),
false);
CatalogTable catalogTable = (CatalogTable) hiveCatalog.getTable(path1);
- assertTrue("PK not present", catalogTable.getSchema().getPrimaryKey().isPresent());
+ assertThat(catalogTable.getSchema().getPrimaryKey().isPresent())
+ .as("PK not present")
+ .isTrue();
Review Comment:
```suggestion
assertThat(catalogTable.getSchema().getPrimaryKey())
.as("PK not present")
.isPresent();
```
##########
flink-connectors/flink-connector-hive/src/test/java/org/apache/flink/table/catalog/hive/HiveCatalogHiveMetadataTest.java:
##########
@@ -303,13 +305,13 @@ public void testAlterPartition() throws Exception {
catalog.alterPartition(path1, createPartitionSpec(), another, false);
- assertEquals(
- Collections.singletonList(createPartitionSpec()), catalog.listPartitions(path1));
+ assertThat(catalog.listPartitions(path1))
+ .isEqualTo(Collections.singletonList(createPartitionSpec()));
Review Comment:
containsExactly?
##########
flink-connectors/flink-connector-hive/src/test/java/org/apache/flink/table/catalog/hive/HiveCatalogHiveMetadataTest.java:
##########
@@ -92,23 +89,23 @@ public void testCreateTable_Streaming() throws Exception {}
@Test
// verifies that input/output formats and SerDe are set for Hive tables
- public void testCreateTable_StorageFormatSet() throws Exception {
+ void testCreateTable_StorageFormatSet() throws Exception {
catalog.createDatabase(db1, createDb(), false);
catalog.createTable(path1, createTable(), false);
Table hiveTable = ((HiveCatalog) catalog).getHiveTable(path1);
String inputFormat = hiveTable.getSd().getInputFormat();
String outputFormat = hiveTable.getSd().getOutputFormat();
String serde = hiveTable.getSd().getSerdeInfo().getSerializationLib();
- assertFalse(StringUtils.isNullOrWhitespaceOnly(inputFormat));
- assertFalse(StringUtils.isNullOrWhitespaceOnly(outputFormat));
- assertFalse(StringUtils.isNullOrWhitespaceOnly(serde));
+ assertThat(StringUtils.isNullOrWhitespaceOnly(inputFormat)).isFalse();
+ assertThat(StringUtils.isNullOrWhitespaceOnly(outputFormat)).isFalse();
+ assertThat(StringUtils.isNullOrWhitespaceOnly(serde)).isFalse();
Review Comment:
```suggestion
assertThat(inputFormat).isNotBlank();
assertThat(outputFormat).isNotBlank();
assertThat(serde).isNotBlank();
```
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/catalog/CatalogTest.java:
##########
@@ -101,7 +98,7 @@ public void cleanup() throws Exception {
}
}
- @AfterClass
+ @AfterAll
public static void closeup() {
Review Comment:
```suggestion
static void closeup() {
```
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/catalog/CatalogTest.java:
##########
@@ -50,6 +48,7 @@
import java.util.Optional;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
/** Class for unit tests to run on catalogs. */
public abstract class CatalogTest {
Review Comment:
```suggestion
/** Class for unit tests to run on catalogs. */
@ExtendWith(TestLoggerExtension.class)
public abstract class CatalogTest {
```
##########
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/typeutils/FieldInfoUtilsTest.java:
##########
@@ -83,20 +91,21 @@ public void testByNameModeReorderAndRename() {
assertThat(schema.getFieldNames()).isEqualTo(new String[] {"aa", "bb", "cc"});
}
+ }
- /** Test Pojo class. */
- public static class MyPojo {
- public int f0;
- public long f1;
- public Timestamp f2;
+ /** Test Pojo class. */
+ public static class MyPojo {
+ public int f0;
+ public long f1;
+ public Timestamp f2;
- public MyPojo() {}
- }
+ public MyPojo() {}
}
/** Test for ByPositionMode. */
- public static final class TestByPositionMode {
- private static final RowTypeInfo rowTypeInfo =
+ @Nested
+ public final class TestByPositionMode {
Review Comment:
```suggestion
final class TestByPositionMode {
```
?
##########
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/typeutils/FieldInfoUtilsTest.java:
##########
@@ -25,55 +25,63 @@
import org.apache.flink.api.java.typeutils.RowTypeInfo;
import org.apache.flink.table.expressions.Expression;
-import org.junit.Test;
-import org.junit.experimental.runners.Enclosed;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import java.sql.Timestamp;
import java.util.Arrays;
-import java.util.Collection;
+import java.util.stream.Stream;
import static org.apache.flink.table.api.Expressions.$;
import static org.assertj.core.api.Assertions.assertThat;
/** Test suite for {@link FieldInfoUtils}. */
-@RunWith(Enclosed.class)
-public class FieldInfoUtilsTest {
+class FieldInfoUtilsTest {
/** Test for ByNameMode. */
- @RunWith(Parameterized.class)
- public static final class TestByNameMode {
-
- @Parameterized.Parameters(name = "{0}")
- public static Collection<TypeInformation> parameters() throws Exception {
- return Arrays.asList(
- new RowTypeInfo(
- new TypeInformation[] {Types.INT, Types.LONG, Types.SQL_TIMESTAMP},
- new String[] {"f0", "f1", "f2"}),
- new PojoTypeInfo(
- MyPojo.class,
- Arrays.asList(
- new PojoField(MyPojo.class.getDeclaredField("f0"), Types.INT),
- new PojoField(MyPojo.class.getDeclaredField("f1"), Types.LONG),
- new PojoField(
- MyPojo.class.getDeclaredField("f2"),
- Types.SQL_TIMESTAMP))));
+ @Nested
+ @TestInstance(TestInstance.Lifecycle.PER_CLASS)
+ class TestByNameMode {
+
+ Stream<Arguments> parameters() throws Exception {
+ return Stream.of(
+ Arguments.of(
Review Comment:
unnecessary wrapping
##########
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/typeutils/FieldInfoUtilsTest.java:
##########
@@ -25,55 +25,63 @@
import org.apache.flink.api.java.typeutils.RowTypeInfo;
import org.apache.flink.table.expressions.Expression;
-import org.junit.Test;
-import org.junit.experimental.runners.Enclosed;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import java.sql.Timestamp;
import java.util.Arrays;
-import java.util.Collection;
+import java.util.stream.Stream;
import static org.apache.flink.table.api.Expressions.$;
import static org.assertj.core.api.Assertions.assertThat;
/** Test suite for {@link FieldInfoUtils}. */
-@RunWith(Enclosed.class)
-public class FieldInfoUtilsTest {
+class FieldInfoUtilsTest {
/** Test for ByNameMode. */
- @RunWith(Parameterized.class)
- public static final class TestByNameMode {
-
- @Parameterized.Parameters(name = "{0}")
- public static Collection<TypeInformation> parameters() throws Exception {
- return Arrays.asList(
- new RowTypeInfo(
- new TypeInformation[] {Types.INT, Types.LONG, Types.SQL_TIMESTAMP},
- new String[] {"f0", "f1", "f2"}),
- new PojoTypeInfo(
- MyPojo.class,
- Arrays.asList(
- new PojoField(MyPojo.class.getDeclaredField("f0"), Types.INT),
- new PojoField(MyPojo.class.getDeclaredField("f1"), Types.LONG),
- new PojoField(
- MyPojo.class.getDeclaredField("f2"),
- Types.SQL_TIMESTAMP))));
+ @Nested
+ @TestInstance(TestInstance.Lifecycle.PER_CLASS)
+ class TestByNameMode {
+
+ Stream<Arguments> parameters() throws Exception {
Review Comment:
I think these can be private.
##########
flink-connectors/flink-connector-hive/src/test/java/org/apache/flink/table/catalog/hive/HiveCatalogGenericMetadataTest.java:
##########
@@ -332,38 +331,38 @@ public void testGenericTableWithoutConnectorProp() throws Exception {
CatalogTable catalogTable = new CatalogTableImpl(tableSchema, Collections.emptyMap(), null);
catalog.createTable(path1, catalogTable, false);
CatalogTable retrievedTable = (CatalogTable) catalog.getTable(path1);
- assertEquals(tableSchema, retrievedTable.getSchema());
- assertEquals(Collections.emptyMap(), retrievedTable.getOptions());
+ assertThat(retrievedTable.getSchema()).isEqualTo(tableSchema);
+ assertThat(retrievedTable.getOptions()).isEqualTo(Collections.emptyMap());
Review Comment:
```suggestion
assertThat(retrievedTable.getOptions()).isEmpty();
```
##########
flink-connectors/flink-connector-hive/src/test/java/org/apache/flink/table/catalog/hive/HiveCatalogHiveMetadataTest.java:
##########
@@ -288,11 +290,11 @@ public void testAlterPartition() throws Exception {
catalog.createTable(path1, createPartitionedTable(), false);
catalog.createPartition(path1, createPartitionSpec(), createPartition(), false);
- assertEquals(
- Collections.singletonList(createPartitionSpec()), catalog.listPartitions(path1));
+ assertThat(catalog.listPartitions(path1))
+ .isEqualTo(Collections.singletonList(createPartitionSpec()));
Review Comment:
containsExactly?
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/catalog/CatalogTest.java:
##########
@@ -1130,8 +1151,9 @@ public void testListPartitionPartialSpec() throws Exception {
@Test
public void testGetTableStats_TableNotExistException() throws Exception {
catalog.createDatabase(db1, createDb(), false);
- exception.expect(org.apache.flink.table.catalog.exceptions.TableNotExistException.class);
- catalog.getTableStatistics(path1);
+ assertThatThrownBy(() -> catalog.getTableStatistics(path1))
+ .isInstanceOf(
+ org.apache.flink.table.catalog.exceptions.TableNotExistException.class);
Review Comment:
```suggestion
.isInstanceOf(TableNotExistException.class);
```
This seems to be the same class as in other tests.
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/catalog/CatalogTest.java:
##########
@@ -74,9 +73,7 @@ public abstract class CatalogTest {
protected static Catalog catalog;
- @Rule public ExpectedException exception = ExpectedException.none();
-
- @After
+ @AfterEach
public void cleanup() throws Exception {
Review Comment:
```suggestion
void cleanup() throws Exception {
```
--
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 #19538: [FLINK-27323][flink-table-api-java][tests] Migrate tests to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on PR #19538:
URL: https://github.com/apache/flink/pull/19538#issuecomment-1108422331
@zentol sorry for the poke.
Since you are one of the committers involved in migration to junit5/asserJ, could you please have a look here once you have time?
--
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 #19538: [FLINK-27323][flink-table-api-java][tests] Migrate tests to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on PR #19538:
URL: https://github.com/apache/flink/pull/19538#issuecomment-1121530098
merge conflicts resolved and rebased
--
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 #19538: [FLINK-27323][flink-table-api-java][tests] Migrate tests to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19538:
URL: https://github.com/apache/flink/pull/19538#discussion_r866297695
##########
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/typeutils/FieldInfoUtilsTest.java:
##########
@@ -25,55 +25,63 @@
import org.apache.flink.api.java.typeutils.RowTypeInfo;
import org.apache.flink.table.expressions.Expression;
-import org.junit.Test;
-import org.junit.experimental.runners.Enclosed;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import java.sql.Timestamp;
import java.util.Arrays;
-import java.util.Collection;
+import java.util.stream.Stream;
import static org.apache.flink.table.api.Expressions.$;
import static org.assertj.core.api.Assertions.assertThat;
/** Test suite for {@link FieldInfoUtils}. */
-@RunWith(Enclosed.class)
-public class FieldInfoUtilsTest {
+class FieldInfoUtilsTest {
/** Test for ByNameMode. */
- @RunWith(Parameterized.class)
- public static final class TestByNameMode {
-
- @Parameterized.Parameters(name = "{0}")
- public static Collection<TypeInformation> parameters() throws Exception {
- return Arrays.asList(
- new RowTypeInfo(
- new TypeInformation[] {Types.INT, Types.LONG, Types.SQL_TIMESTAMP},
- new String[] {"f0", "f1", "f2"}),
- new PojoTypeInfo(
- MyPojo.class,
- Arrays.asList(
- new PojoField(MyPojo.class.getDeclaredField("f0"), Types.INT),
- new PojoField(MyPojo.class.getDeclaredField("f1"), Types.LONG),
- new PojoField(
- MyPojo.class.getDeclaredField("f2"),
- Types.SQL_TIMESTAMP))));
+ @Nested
+ @TestInstance(TestInstance.Lifecycle.PER_CLASS)
+ class TestByNameMode {
Review Comment:
It seems nested are not required here since parametrized is on method level
--
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 #19538: [FLINK-27323][flink-table-api-java][tests] Migrate tests to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19538:
URL: https://github.com/apache/flink/pull/19538#discussion_r866296097
##########
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/typeutils/FieldInfoUtilsTest.java:
##########
@@ -25,55 +25,63 @@
import org.apache.flink.api.java.typeutils.RowTypeInfo;
import org.apache.flink.table.expressions.Expression;
-import org.junit.Test;
-import org.junit.experimental.runners.Enclosed;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import java.sql.Timestamp;
import java.util.Arrays;
-import java.util.Collection;
+import java.util.stream.Stream;
import static org.apache.flink.table.api.Expressions.$;
import static org.assertj.core.api.Assertions.assertThat;
/** Test suite for {@link FieldInfoUtils}. */
-@RunWith(Enclosed.class)
-public class FieldInfoUtilsTest {
+class FieldInfoUtilsTest {
/** Test for ByNameMode. */
- @RunWith(Parameterized.class)
- public static final class TestByNameMode {
-
- @Parameterized.Parameters(name = "{0}")
- public static Collection<TypeInformation> parameters() throws Exception {
- return Arrays.asList(
- new RowTypeInfo(
- new TypeInformation[] {Types.INT, Types.LONG, Types.SQL_TIMESTAMP},
- new String[] {"f0", "f1", "f2"}),
- new PojoTypeInfo(
- MyPojo.class,
- Arrays.asList(
- new PojoField(MyPojo.class.getDeclaredField("f0"), Types.INT),
- new PojoField(MyPojo.class.getDeclaredField("f1"), Types.LONG),
- new PojoField(
- MyPojo.class.getDeclaredField("f2"),
- Types.SQL_TIMESTAMP))));
+ @Nested
+ @TestInstance(TestInstance.Lifecycle.PER_CLASS)
+ class TestByNameMode {
+
+ Stream<Arguments> parameters() throws Exception {
Review Comment:
yes, you're right
--
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 #19538: [FLINK-27323][flink-table-api-java][tests] Migrate tests to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19538:
URL: https://github.com/apache/flink/pull/19538#discussion_r866298009
##########
flink-table/flink-table-api-java/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension:
##########
@@ -0,0 +1,16 @@
+# 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.
+
+org.apache.flink.util.TestLoggerExtension
Review Comment:
removed
--
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