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/22 15:06:35 UTC
[GitHub] [flink] snuyanzin commented on a diff in pull request #19552: [FLINK-27185][connectors][formats] Convert formats and connectors modules to assertj
snuyanzin commented on code in PR #19552:
URL: https://github.com/apache/flink/pull/19552#discussion_r856240226
##########
flink-connectors/flink-connector-base/src/test/java/org/apache/flink/connector/base/source/reader/fetcher/SplitFetcherManagerTest.java:
##########
@@ -100,7 +93,7 @@ private final void testExceptionPropagation(
fetcher.checkErrors();
fail("expected exception");
} catch (Exception e) {
- assertSame(testingException, e.getCause().getCause());
+ assertThat(e.getCause().getCause()).isSameAs(testingException);
Review Comment:
Shouldn't it be replaced with `assertThatThrownBy(() -> ... )` ?
##########
flink-connectors/flink-connector-cassandra/src/test/java/org/apache/flink/streaming/connectors/cassandra/CassandraSinkBaseTest.java:
##########
@@ -159,9 +156,9 @@ public void testThrowErrorOnSnapshot() throws Exception {
try {
testHarness.snapshot(123L, 123L);
- Assert.fail();
+ fail("unknown failure");
} catch (Exception e) {
- Assert.assertTrue(e.getCause() instanceof IOException);
+ assertThat(e.getCause()).isInstanceOf(IOException.class);
}
Review Comment:
Could it be replaced with
```java
assertThatThrownBy(() -> testHarness.snapshot(123L, 123L))
.hasCauseInstanceOf(IOException.class);
```
?
##########
flink-connectors/flink-connector-jdbc/src/test/java/org/apache/flink/connector/jdbc/internal/connection/SimpleJdbcConnectionProviderTest.java:
##########
@@ -140,19 +130,17 @@ public void testInvalidDriverUrl() throws Exception {
provider.getOrEstablishConnection();
fail("expect exception");
} catch (SQLException ex) {
- assertThat(
Review Comment:
Probably `assertThatThrownBy` could be used like
```java
assertThatThrownBy(provider::getOrEstablishConnection)
.isInstanceOf(SQLException.class)
.hasMessageContaining("No suitable driver found for " + FakeDBUtils.TEST_DB_INVALID_URL);
```
##########
flink-connectors/flink-connector-jdbc/src/test/java/org/apache/flink/connector/jdbc/utils/JdbcTypeUtilTest.java:
##########
@@ -25,23 +25,23 @@
import java.sql.Types;
import static org.apache.flink.connector.jdbc.utils.JdbcTypeUtil.logicalTypeToSqlType;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
/** Testing the type conversions from Flink to SQL types. */
public class JdbcTypeUtilTest {
@Test
public void testTypeConversions() {
- assertEquals(Types.INTEGER, logicalTypeToSqlType(LogicalTypeRoot.INTEGER));
+ assertThat(logicalTypeToSqlType(LogicalTypeRoot.INTEGER)).isEqualTo(Types.INTEGER);
testUnsupportedType(LogicalTypeRoot.RAW);
testUnsupportedType(LogicalTypeRoot.MAP);
}
private static void testUnsupportedType(LogicalTypeRoot logicalTypeRoot) {
try {
logicalTypeToSqlType(logicalTypeRoot);
- fail();
+ fail("unknown failure");
} catch (IllegalArgumentException ignored) {
}
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-connectors/flink-connector-kafka/src/test/java/org/apache/flink/streaming/connectors/kafka/KafkaShortRetentionTestBase.java:
##########
@@ -286,7 +286,7 @@ public void runFailOnAutoOffsetResetNoneEager() throws Exception {
fail("should fail with an exception");
} catch (IllegalArgumentException e) {
// expected
- assertTrue(e.getMessage().contains("none"));
+ assertThat(e.getMessage()).contains("none");
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-connectors/flink-connector-kafka/src/test/java/org/apache/flink/streaming/connectors/kafka/table/KafkaConnectorOptionsUtilTest.java:
##########
@@ -118,14 +120,15 @@ public void testInvalidKeyFormatPrefixProjection() {
try {
createKeyFormatProjection(config, dataType);
- fail();
+ fail("unknown failure");
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-formats/flink-csv/src/test/java/org/apache/flink/formats/csv/CsvRowDataSerDeSchemaTest.java:
##########
@@ -196,11 +194,11 @@ public void testSerializeDeserializeCustomizedProperties() throws Exception {
try {
testFieldDeserialization(
TIME(precision), "12:12:12.45", LocalTime.parse("12:12:12"), deserConfig, ";");
- fail();
+ fail("unknown failure");
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-formats/flink-csv/src/test/java/org/apache/flink/formats/csv/CsvRowDataSerDeSchemaTest.java:
##########
@@ -362,7 +357,7 @@ public void testSerializationWithTypesMismatch() {
serialize(serSchemaBuilder, rowData);
fail("expecting exception message:" + errorMessage);
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-formats/flink-csv/src/test/java/org/apache/flink/formats/csv/CsvRowDataSerDeSchemaTest.java:
##########
@@ -378,7 +373,7 @@ public void testDeserializationWithTypesMismatch() {
deserialize(deserSchemaBuilder, data);
fail("expecting exception message:" + errorMessage);
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/debezium/DebeziumJsonFormatFactoryTest.java:
##########
@@ -140,12 +140,12 @@ public void testSchemaIncludeOption() {
// should fail
sinkMock.valueFormat.createRuntimeEncoder(
new SinkRuntimeProviderContext(false), PHYSICAL_DATA_TYPE);
- fail();
+ fail("unknown failure");
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-connectors/flink-connector-jdbc/src/test/java/org/apache/flink/connector/jdbc/table/JdbcOutputFormatTest.java:
##########
@@ -146,7 +144,7 @@ public void testInvalidURL() {
outputFormat.open(0, 1);
fail("Expected exception is not thrown.");
} catch (Exception e) {
- assertTrue(findThrowable(e, IllegalStateException.class).isPresent());
+ assertThat(findThrowable(e, IllegalStateException.class)).isPresent();
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-connectors/flink-connector-jdbc/src/test/java/org/apache/flink/connector/jdbc/table/JdbcOutputFormatTest.java:
##########
@@ -183,7 +181,7 @@ public void testIncompatibleTypes() {
outputFormat.close();
fail("Expected exception is not thrown.");
} catch (Exception e) {
- assertTrue(findThrowable(e, ClassCastException.class).isPresent());
+ assertThat(findThrowable(e, ClassCastException.class)).isPresent();
}
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-connectors/flink-connector-jdbc/src/test/java/org/apache/flink/connector/jdbc/table/JdbcOutputFormatTest.java:
##########
@@ -263,8 +261,8 @@ public void testExceptionOnClose() {
fail("Expected exception is not thrown.");
} catch (Exception e) {
- assertTrue(findThrowable(e, RuntimeException.class).isPresent());
- assertTrue(findThrowableWithMessage(e, expectedMsg).isPresent());
+ assertThat(findThrowable(e, RuntimeException.class)).isPresent();
+ assertThat(findThrowableWithMessage(e, expectedMsg)).isPresent();
}
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-connectors/flink-connector-hive/src/test/java/org/apache/flink/table/catalog/hive/factories/HiveCatalogFactoryTest.java:
##########
@@ -134,7 +132,7 @@ public void testCreateHiveCatalogWithIllegalHadoopConfDir() throws IOException {
options,
null,
Thread.currentThread().getContextClassLoader());
- Assert.fail();
+ fail("unknown failure");
} catch (ValidationException e) {
}
Review Comment:
Probably could be used `assertThatThrownBy` like
```java
assertThatThrownBy(() -> FactoryUtil.createCatalog(
catalogName,
options,
null,
Thread.currentThread().getContextClassLoader()))
.isInstanceOf(ValidationException.class);
```
##########
flink-connectors/flink-connector-jdbc/src/test/java/org/apache/flink/connector/jdbc/table/JdbcOutputFormatTest.java:
##########
@@ -115,8 +113,8 @@ public void testInvalidDriver() {
outputFormat.open(0, 1);
fail("Expected exception is not thrown.");
} catch (Exception e) {
- assertTrue(findThrowable(e, IOException.class).isPresent());
- assertTrue(findThrowableWithMessage(e, expectedMsg).isPresent());
+ assertThat(findThrowable(e, IOException.class)).isPresent();
+ assertThat(findThrowableWithMessage(e, expectedMsg)).isPresent();
}
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-connectors/flink-connector-kafka/src/test/java/org/apache/flink/streaming/connectors/kafka/table/KafkaConnectorOptionsUtilTest.java:
##########
@@ -92,17 +93,18 @@ public void testInvalidKeyFormatFieldProjection() {
try {
createKeyFormatProjection(config, dataType);
- fail();
+ fail("unknown failure");
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/JsonRowDataSerDeSchemaTest.java:
##########
@@ -562,18 +560,18 @@ public void testSerializationMapNullKey() throws Exception {
try {
// throw exception when mapNullKey Mode is fail
serializationSchema1.serialize(rowData);
- Assert.fail("expecting exception message: " + errorMessage1);
+ fail("expecting exception message: " + errorMessage1);
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-connectors/flink-connector-kafka/src/test/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducerITCase.java:
##########
@@ -193,7 +189,7 @@ public void testFlinkKafkaProducerFailBeforeNotify() throws Exception {
try {
testHarness.processElement(44, 4);
testHarness.snapshot(2, 5);
- fail();
+ fail("unknown failure");
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-connectors/flink-connector-kafka/src/test/java/org/apache/flink/streaming/connectors/kafka/table/KafkaConnectorOptionsUtilTest.java:
##########
@@ -71,14 +71,15 @@ public void testMissingKeyFormatProjection() {
try {
createKeyFormatProjection(config, dataType);
- fail();
+ fail("unknown failure");
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-connectors/flink-connector-jdbc/src/test/java/org/apache/flink/connector/jdbc/table/JdbcOutputFormatTest.java:
##########
@@ -220,7 +218,7 @@ public void testExceptionOnInvalidType() {
outputFormat.close();
fail("Expected exception is not thrown.");
} catch (Exception e) {
- assertTrue(findThrowable(e, ClassCastException.class).isPresent());
+ assertThat(findThrowable(e, ClassCastException.class)).isPresent();
}
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/JsonRowDataSerDeSchemaTest.java:
##########
@@ -443,25 +441,25 @@ public void testDeserializationMissingField() throws Exception {
deserializationSchema.deserialize(serializedJson);
fail("expecting exception message: " + errorMessage);
} catch (Throwable t) {
- assertEquals(errorMessage, t.getMessage());
+ assertThat(t).hasMessage(errorMessage);
}
// ignore on parse error
deserializationSchema =
new JsonRowDataDeserializationSchema(
schema, InternalTypeInfo.of(schema), false, true, TimestampFormat.ISO_8601);
actual = convertToExternal(deserializationSchema.deserialize(serializedJson), dataType);
- assertEquals(expected, actual);
+ assertThat(actual).isEqualTo(expected);
errorMessage =
"JSON format doesn't support failOnMissingField and ignoreParseErrors are both enabled.";
try {
// failOnMissingField and ignoreParseErrors both enabled
new JsonRowDataDeserializationSchema(
schema, InternalTypeInfo.of(schema), true, true, TimestampFormat.ISO_8601);
- Assert.fail("expecting exception message: " + errorMessage);
+ fail("expecting exception message: " + errorMessage);
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/JsonRowDataSerDeSchemaTest.java:
##########
@@ -703,7 +701,7 @@ private void testParseErrors(TestSpec spec) throws Exception {
failingSchema.deserialize(spec.json.getBytes());
fail("expecting exception " + spec.errorMessage);
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-connectors/flink-connector-kafka/src/test/java/org/apache/flink/streaming/connectors/kafka/table/KafkaConnectorOptionsUtilTest.java:
##########
@@ -140,14 +143,15 @@ public void testInvalidValueFormatProjection() {
try {
createValueFormatProjection(config, dataType);
- fail();
+ fail("unknown failure");
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/JsonRowDataSerDeSchemaTest.java:
##########
@@ -443,25 +441,25 @@ public void testDeserializationMissingField() throws Exception {
deserializationSchema.deserialize(serializedJson);
fail("expecting exception message: " + errorMessage);
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/JsonRowDataSerDeSchemaTest.java:
##########
@@ -649,7 +647,7 @@ public void testSerializationWithTypesMismatch() {
serializationSchema.serialize(genericRowData);
fail("expecting exception message: " + errorMessage);
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
##########
flink-formats/flink-json/src/test/java/org/apache/flink/formats/json/JsonRowDataSerDeSchemaTest.java:
##########
@@ -665,7 +663,7 @@ public void testDeserializationWithTypesMismatch() {
deserializationSchema.deserialize(json.getBytes());
fail("expecting exception message: " + errorMessage);
Review Comment:
Probably `try ... catch` could be replaced with `assertThatThrownBy`
--
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