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 10:44:23 UTC

[GitHub] [flink] zentol commented on a diff in pull request #19467: [FLINK-25548][flink-sql-parser] Migrate tests to JUnit5

zentol commented on code in PR #19467:
URL: https://github.com/apache/flink/pull/19467#discussion_r853987636


##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/TableApiIdentifierParsingTest.java:
##########
@@ -26,49 +26,44 @@
 import org.apache.calcite.sql.SqlIdentifier;
 import org.apache.calcite.sql.parser.SqlAbstractParserImpl;
 import org.apache.calcite.util.SourceStringReader;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import java.util.List;
+import java.util.stream.Stream;
 
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.params.provider.Arguments.of;
 
 /** Tests for parsing a Table API specific SqlIdentifier. */
-@RunWith(Parameterized.class)
 public class TableApiIdentifierParsingTest {
 
     private static final String ANTHROPOS_IN_GREEK_IN_UNICODE =
             "#03B1#03BD#03B8#03C1#03C9#03C0#03BF#03C2";
     private static final String ANTHROPOS_IN_GREEK = "ανθρωπος";
 
-    @Parameterized.Parameters(name = "Parsing: {0}. Expected identifier: {1}")
-    public static Object[][] parameters() {
-        return new Object[][] {
-            new Object[] {"array", singletonList("array")},
-            new Object[] {"table", singletonList("table")},
-            new Object[] {"cat.db.array", asList("cat", "db", "array")},
-            new Object[] {"`cat.db`.table", asList("cat.db", "table")},
-            new Object[] {"db.table", asList("db", "table")},
-            new Object[] {"`ta``ble`", singletonList("ta`ble")},
-            new Object[] {"`c``at`.`d``b`.`ta``ble`", asList("c`at", "d`b", "ta`ble")},
-            new Object[] {
-                "db.U&\"" + ANTHROPOS_IN_GREEK_IN_UNICODE + "\" UESCAPE '#'",
-                asList("db", ANTHROPOS_IN_GREEK)
-            },
-            new Object[] {"db.ανθρωπος", asList("db", ANTHROPOS_IN_GREEK)}
-        };
+    public static Stream<Arguments> parameters() {

Review Comment:
   does this need to be public?



##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/TableApiIdentifierParsingTest.java:
##########
@@ -26,49 +26,44 @@
 import org.apache.calcite.sql.SqlIdentifier;
 import org.apache.calcite.sql.parser.SqlAbstractParserImpl;
 import org.apache.calcite.util.SourceStringReader;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import java.util.List;
+import java.util.stream.Stream;
 
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.params.provider.Arguments.of;
 
 /** Tests for parsing a Table API specific SqlIdentifier. */
-@RunWith(Parameterized.class)
 public class TableApiIdentifierParsingTest {
 
     private static final String ANTHROPOS_IN_GREEK_IN_UNICODE =
             "#03B1#03BD#03B8#03C1#03C9#03C0#03BF#03C2";
     private static final String ANTHROPOS_IN_GREEK = "ανθρωπος";
 
-    @Parameterized.Parameters(name = "Parsing: {0}. Expected identifier: {1}")
-    public static Object[][] parameters() {
-        return new Object[][] {
-            new Object[] {"array", singletonList("array")},
-            new Object[] {"table", singletonList("table")},
-            new Object[] {"cat.db.array", asList("cat", "db", "array")},
-            new Object[] {"`cat.db`.table", asList("cat.db", "table")},
-            new Object[] {"db.table", asList("db", "table")},
-            new Object[] {"`ta``ble`", singletonList("ta`ble")},
-            new Object[] {"`c``at`.`d``b`.`ta``ble`", asList("c`at", "d`b", "ta`ble")},
-            new Object[] {
-                "db.U&\"" + ANTHROPOS_IN_GREEK_IN_UNICODE + "\" UESCAPE '#'",
-                asList("db", ANTHROPOS_IN_GREEK)
-            },
-            new Object[] {"db.ανθρωπος", asList("db", ANTHROPOS_IN_GREEK)}
-        };
+    public static Stream<Arguments> parameters() {
+        return Stream.of(
+                of("array", singletonList("array")),
+                of("table", singletonList("table")),
+                of("cat.db.array", asList("cat", "db", "array")),
+                of("`cat.db`.table", asList("cat.db", "table")),
+                of("db.table", asList("db", "table")),
+                of("`ta``ble`", singletonList("ta`ble")),
+                of("`c``at`.`d``b`.`ta``ble`", asList("c`at", "d`b", "ta`ble")),
+                of(
+                        "db.U&\"" + ANTHROPOS_IN_GREEK_IN_UNICODE + "\" UESCAPE '#'",
+                        asList("db", ANTHROPOS_IN_GREEK)),
+                of("db.ανθρωπος", asList("db", ANTHROPOS_IN_GREEK)));
     }
 
-    @Parameterized.Parameter public String stringIdentifier;
-
-    @Parameterized.Parameter(1)
-    public List<String> expectedParsedIdentifier;
-
-    @Test
-    public void testTableApiIdentifierParsing() throws ParseException {
+    @ParameterizedTest

Review Comment:
   ```suggestion
       @ParameterizedTest(name = "Parsing: {0}. Expected identifier: {1}")
   ```
   ?



##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/TableApiIdentifierParsingTest.java:
##########
@@ -26,49 +26,44 @@
 import org.apache.calcite.sql.SqlIdentifier;
 import org.apache.calcite.sql.parser.SqlAbstractParserImpl;
 import org.apache.calcite.util.SourceStringReader;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import java.util.List;
+import java.util.stream.Stream;
 
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.params.provider.Arguments.of;
 
 /** Tests for parsing a Table API specific SqlIdentifier. */
-@RunWith(Parameterized.class)
 public class TableApiIdentifierParsingTest {

Review Comment:
   can be package-private



##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/FlinkDDLDataTypeTest.java:
##########
@@ -334,27 +344,28 @@ private static TestItem createTestItem(Object... args) {
         if (args.length == 3) {
             testItem.withExpectedUnparsed((String) args[2]);
         }
-        return testItem;
+        return of(testItem);
     }
 
-    @Parameterized.Parameter public TestItem testItem;
-
-    @Test
-    public void testDataTypeParsing() {
+    @ParameterizedTest

Review Comment:
   ```suggestion
       @ParameterizedTest(name = "{index}: {0}")
   ```



##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/FlinkDDLDataTypeTest.java:
##########
@@ -334,27 +344,28 @@ private static TestItem createTestItem(Object... args) {
         if (args.length == 3) {
             testItem.withExpectedUnparsed((String) args[2]);
         }
-        return testItem;
+        return of(testItem);
     }
 
-    @Parameterized.Parameter public TestItem testItem;
-
-    @Test
-    public void testDataTypeParsing() {
+    @ParameterizedTest
+    @MethodSource("testData")
+    void testDataTypeParsing(TestItem testItem) {
         if (testItem.expectedType != null) {
             checkType(testItem.testExpr, testItem.expectedType);
         }
     }
 
-    @Test
-    public void testThrowsError() {
+    @ParameterizedTest
+    @MethodSource("testData")
+    void testThrowsError(TestItem testItem) {
         if (testItem.expectedError != null) {
             checkFails(testItem.testExpr, testItem.expectedError);
         }
     }
 
-    @Test
-    public void testDataTypeUnparsing() {
+    @ParameterizedTest

Review Comment:
   ```suggestion
       @ParameterizedTest(name = "{index}: {0}")
   ```



##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/FlinkDDLDataTypeTest.java:
##########
@@ -334,27 +344,28 @@ private static TestItem createTestItem(Object... args) {
         if (args.length == 3) {
             testItem.withExpectedUnparsed((String) args[2]);
         }
-        return testItem;
+        return of(testItem);
     }
 
-    @Parameterized.Parameter public TestItem testItem;
-
-    @Test
-    public void testDataTypeParsing() {
+    @ParameterizedTest
+    @MethodSource("testData")
+    void testDataTypeParsing(TestItem testItem) {
         if (testItem.expectedType != null) {
             checkType(testItem.testExpr, testItem.expectedType);
         }
     }
 
-    @Test
-    public void testThrowsError() {
+    @ParameterizedTest

Review Comment:
   ```suggestion
       @ParameterizedTest(name = "{index}: {0}")
   ```



##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/FlinkDDLDataTypeTest.java:
##########
@@ -52,195 +52,203 @@
 import org.apache.calcite.test.catalog.MockCatalogReaderSimple;
 import org.apache.calcite.util.SourceStringReader;
 import org.apache.calcite.util.Util;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import javax.annotation.Nullable;
 
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
+import java.util.stream.Stream;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.params.provider.Arguments.of;
 
 /** Tests for all the supported Flink DDL data types. */
-@RunWith(Parameterized.class)
 public class FlinkDDLDataTypeTest {
     private static final Fixture FIXTURE = new Fixture(TestFactory.INSTANCE.getTypeFactory());
     private static final String DDL_FORMAT =
             "create table t1 (\n" + "  f0 %s\n" + ") with (\n" + "  'k1' = 'v1'\n" + ")";
 
-    @Parameterized.Parameters(name = "{index}: {0}")
-    public static List<TestItem> testData() {
-        return Arrays.asList(
-                createTestItem("CHAR", nullable(FIXTURE.char1Type), "CHAR"),
-                createTestItem("CHAR NOT NULL", FIXTURE.char1Type, "CHAR NOT NULL"),
-                createTestItem("CHAR   NOT \t\nNULL", FIXTURE.char1Type, "CHAR NOT NULL"),
-                createTestItem("char not null", FIXTURE.char1Type, "CHAR NOT NULL"),
-                createTestItem("CHAR NULL", nullable(FIXTURE.char1Type), "CHAR"),
-                createTestItem("CHAR(33)", nullable(FIXTURE.char33Type), "CHAR(33)"),
-                createTestItem("VARCHAR", nullable(FIXTURE.varcharType), "VARCHAR"),
-                createTestItem("VARCHAR(33)", nullable(FIXTURE.varchar33Type), "VARCHAR(33)"),
-                createTestItem(
+    public static Stream<Arguments> testData() {

Review Comment:
   does this need to be public?



##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/CreateTableLikeTest.java:
##########
@@ -48,10 +47,8 @@
 /** Tests for parsing and validating {@link SqlTableLike} clause. */
 public class CreateTableLikeTest {

Review Comment:
   can be package-private



##########
flink-table/flink-sql-parser-hive/src/test/java/org/apache/flink/sql/parser/hive/FlinkHiveSqlParserImplTest.java:
##########
@@ -22,8 +22,8 @@
 
 import org.apache.calcite.sql.parser.SqlParserImplFactory;
 import org.apache.calcite.sql.parser.SqlParserTest;
-import org.junit.Ignore;
-import org.junit.Test;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
 
 /** Tests for {@link FlinkHiveSqlParserImpl}. */
 public class FlinkHiveSqlParserImplTest extends SqlParserTest {

Review Comment:
   can be package-private



##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/FlinkDDLDataTypeTest.java:
##########
@@ -52,195 +52,203 @@
 import org.apache.calcite.test.catalog.MockCatalogReaderSimple;
 import org.apache.calcite.util.SourceStringReader;
 import org.apache.calcite.util.Util;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import javax.annotation.Nullable;
 
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
+import java.util.stream.Stream;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.params.provider.Arguments.of;
 
 /** Tests for all the supported Flink DDL data types. */
-@RunWith(Parameterized.class)
 public class FlinkDDLDataTypeTest {

Review Comment:
   can be package-private



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