zentol commented on code in PR #19590:
URL: https://github.com/apache/flink/pull/19590#discussion_r862613016
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/JsonBatchFileSystemITCase.java:
##########
@@ -20,26 +20,31 @@
import org.apache.flink.table.api.TableResult;
import
org.apache.flink.table.planner.runtime.batch.sql.BatchFileSystemITCaseBase;
-import org.apache.flink.table.utils.LegacyRowResource;
+import org.apache.flink.table.utils.LegacyRowExtension;
import org.apache.flink.types.Row;
import org.apache.flink.util.FileUtils;
-import org.junit.Assert;
-import org.junit.Rule;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.api.io.TempDir;
import java.io.File;
import java.io.IOException;
import java.io.PrintWriter;
import java.net.URI;
+import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
+import static org.assertj.core.api.Assertions.assertThat;
+
/** ITCase to test json format for {@link JsonFormatFactory}. */
-public class JsonBatchFileSystemITCase extends BatchFileSystemITCaseBase {
+class JsonBatchFileSystemITCase extends BatchFileSystemITCaseBase {
+
+ @RegisterExtension LegacyRowExtension usesLegacyRows =
LegacyRowExtension.INSTANCE;
Review Comment:
```suggestion
@RegisterExtension private LegacyRowExtension usesLegacyRows =
LegacyRowExtension.INSTANCE;
```
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/ogg/OggJsonSerDeSchemaTest.java:
##########
@@ -242,7 +240,7 @@ private void testSerializationDeserialization(String
resourceFile) throws Except
"{\"before\":{\"id\":111,\"name\":\"scooter\",\"description\":\"Big 2-wheel
scooter \",\"weight\":5.18},\"after\":null,\"op_type\":\"D\"}",
"{\"before\":null,\"after\":{\"id\":111,\"name\":\"scooter\",\"description\":\"Big
2-wheel scooter \",\"weight\":5.17},\"op_type\":\"I\"}",
"{\"before\":{\"id\":111,\"name\":\"scooter\",\"description\":\"Big 2-wheel
scooter \",\"weight\":5.17},\"after\":null,\"op_type\":\"D\"}");
- assertEquals(expected, actual);
+ assertThat(actual).containsExactlyElementsOf(expected);
Review Comment:
```suggestion
assertThat(expected).containsExactlyElementsOf(actual);
```
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/JsonRowDataSerDeSchemaTest.java:
##########
@@ -501,11 +498,11 @@ public void testSerDeSQLTimestampFormat() throws
Exception {
byte[] serializedJson = objectMapper.writeValueAsBytes(root);
RowData rowData = deserializationSchema.deserialize(serializedJson);
byte[] actual = serializationSchema.serialize(rowData);
- assertEquals(new String(serializedJson), new String(actual));
+ assertThat(new String(actual)).isEqualTo(new String(serializedJson));
Review Comment:
again, compare on byte array
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/JsonRowDataSerDeSchemaTest.java:
##########
@@ -330,12 +328,12 @@ public void testSerDeMultiRows() throws Exception {
byte[] serializedJson = objectMapper.writeValueAsBytes(root);
RowData rowData =
deserializationSchema.deserialize(serializedJson);
byte[] actual = serializationSchema.serialize(rowData);
- assertEquals(new String(serializedJson), new String(actual));
+ assertThat(actual).isIn(serializedJson);
Review Comment:
this is not correct. It allows serializedJson to contain more stuff. You
should use containsExactly. Also, let's switch the assertion order.
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/debezium/DebeziumJsonSerDeSchemaTest.java:
##########
@@ -318,7 +307,7 @@ private void testDeserializationWithMetadata(
final SimpleCollector collector = new SimpleCollector();
deserializationSchema.deserialize(firstLine.getBytes(StandardCharsets.UTF_8),
collector);
- assertEquals(1, collector.list.size());
+ assertThat(collector.list.size()).isEqualTo(1);
testConsumer.accept(collector.list.get(0));
Review Comment:
assertThat(collector.list.get(0)).satisfies(testConsumer);
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/ogg/OggJsonSerDeSchemaTest.java:
##########
@@ -204,7 +202,7 @@ private void testSerializationDeserialization(String
resourceFile) throws Except
"-D(111,scooter,Big 2-wheel scooter ,5.17)");
List<String> actual =
collector.list.stream().map(Object::toString).collect(Collectors.toList());
- assertEquals(expected, actual);
+ assertThat(actual).containsExactlyElementsOf(expected);
Review Comment:
```suggestion
assertThat(expected).containsExactlyElementsOf(actual);
```
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/JsonBatchFileSystemITCase.java:
##########
@@ -50,11 +54,27 @@ public String[] formatProperties() {
}
@Test
- public void testParseError() throws Exception {
- String path = new URI(resultPath()).getPath();
+ void testParseError(@TempDir java.nio.file.Path temporaryFolder) throws
Exception {
+ String sql =
Review Comment:
That approach will just create a mess when the FileSystemITCaseBase has been
migrated, as no one will have a clue that something was copied into the tests
themselves.
If this test doesn't work as expected without the FileSystemITCaseBase etc.
being migrated, then this test should still use junit4 (but still use assertj).
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/ogg/OggJsonSerDeSchemaTest.java:
##########
@@ -94,7 +91,7 @@ public void testTombstoneMessages() throws Exception {
SimpleCollector collector = new SimpleCollector();
deserializationSchema.deserialize(null, collector);
deserializationSchema.deserialize(new byte[] {}, collector);
- assertTrue(collector.list.isEmpty());
+ assertThat(collector.list).isNullOrEmpty();
Review Comment:
```suggestion
assertThat(collector.list).isEmpty();
```
--
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]
