Jiabao-Sun commented on code in PR #24603:
URL: https://github.com/apache/flink/pull/24603#discussion_r1552030213
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/AbstractGenericTypeComparatorTest.java:
##########
@@ -22,11 +22,12 @@
import org.apache.flink.api.common.typeutils.TypeComparator;
import org.apache.flink.api.common.typeutils.TypeSerializer;
-import org.junit.Ignore;
-import org.junit.Test;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
Review Comment:
The `@Test` methods visibility can be adjusted to package default.
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/PojoTypeInformationTest.java:
##########
@@ -21,11 +21,11 @@
import org.apache.flink.api.common.typeinfo.TypeInformation;
import org.apache.flink.api.common.typeutils.CompositeType;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
Review Comment:
I suggest using AssertJ to avoid adjusting assertions in the next JUnit
upgrade.
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/PojoSerializerSnapshotTest.java:
##########
@@ -146,47 +145,46 @@ public void testRestoreSerializerWithRemovedFields() {
mockRemovedField(HEIGHT_FIELD)));
final TypeSerializer<TestPojo> restoredSerializer =
testSnapshot.restoreSerializer();
- assertTrue(restoredSerializer.getClass() == PojoSerializer.class);
+ assertThat(restoredSerializer.getClass() ==
PojoSerializer.class).isTrue();
Review Comment:
instanceOf is better.
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/tuple/base/TuplePairComparatorTestBase.java:
##########
@@ -21,40 +21,31 @@
import org.apache.flink.api.common.typeutils.TypePairComparator;
import org.apache.flink.api.java.tuple.Tuple;
import org.apache.flink.api.java.tuple.Tuple2;
-import org.apache.flink.util.TestLogger;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.assertThat;
/**
* Abstract test base for TuplePairComparators.
*
* @param <T>
* @param <R>
*/
-public abstract class TuplePairComparatorTestBase<T extends Tuple, R extends
Tuple>
- extends TestLogger {
+public abstract class TuplePairComparatorTestBase<T extends Tuple, R extends
Tuple> {
protected abstract TypePairComparator<T, R> createComparator(boolean
ascending);
protected abstract Tuple2<T[], R[]> getSortedTestData();
@Test
public void testEqualityWithReference() {
Review Comment:
```suggestion
void testEqualityWithReference() {
```
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/PojoSerializerTest.java:
##########
@@ -173,11 +174,8 @@ public boolean equals(Object other) {
return false;
}
- if ((nestedClass == null && otherTUC.nestedClass != null)
- || (nestedClass != null &&
!nestedClass.equals(otherTUC.nestedClass))) {
- return false;
- }
- return true;
+ return (nestedClass != null || otherTUC.nestedClass == null)
+ && (nestedClass == null ||
nestedClass.equals(otherTUC.nestedClass));
Review Comment:
IMO, these changes are not necessary and unrelated to the JUnit migration.
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/PojoSerializerSnapshotTest.java:
##########
@@ -146,47 +145,46 @@ public void testRestoreSerializerWithRemovedFields() {
mockRemovedField(HEIGHT_FIELD)));
final TypeSerializer<TestPojo> restoredSerializer =
testSnapshot.restoreSerializer();
- assertTrue(restoredSerializer.getClass() == PojoSerializer.class);
+ assertThat(restoredSerializer.getClass() ==
PojoSerializer.class).isTrue();
final PojoSerializer<TestPojo> restoredPojoSerializer =
(PojoSerializer<TestPojo>) restoredSerializer;
final Field[] restoredFields = restoredPojoSerializer.getFields();
- assertArrayEquals(new Field[] {null, NAME_FIELD.field, null},
restoredFields);
+ assertThat(restoredFields).containsExactly(null, NAME_FIELD.field,
null);
final TypeSerializer<?>[] restoredFieldSerializers =
restoredPojoSerializer.getFieldSerializers();
- assertArrayEquals(
- new TypeSerializer[] {
- IntSerializer.INSTANCE, StringSerializer.INSTANCE,
DoubleSerializer.INSTANCE
- },
- restoredFieldSerializers);
+ assertThat(restoredFieldSerializers)
+ .containsExactly(
+ IntSerializer.INSTANCE,
+ StringSerializer.INSTANCE,
+ DoubleSerializer.INSTANCE);
}
@Test
- public void testRestoreSerializerWithNewFields() {
+ void testRestoreSerializerWithNewFields() {
final PojoSerializerSnapshot<TestPojo> testSnapshot =
buildTestSnapshot(Collections.singletonList(HEIGHT_FIELD));
final TypeSerializer<TestPojo> restoredSerializer =
testSnapshot.restoreSerializer();
- assertTrue(restoredSerializer.getClass() == PojoSerializer.class);
+ assertThat(restoredSerializer.getClass() ==
PojoSerializer.class).isTrue();
Review Comment:
instanceOf is better.
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/AbstractGenericTypeSerializerTest.java:
##########
@@ -22,7 +22,7 @@
import org.apache.flink.api.common.typeutils.TypeSerializer;
import org.apache.flink.util.StringUtils;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
Review Comment:
The `@Test` methods visibility can be adjusted to package default.
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/SubclassFromInterfaceSerializerTest.java:
##########
@@ -103,10 +103,7 @@ public boolean equals(Object other) {
if (dumm1 != otherTUC.dumm1) {
return false;
}
- if (!dumm2.equals(otherTUC.dumm2)) {
- return false;
- }
- return true;
+ return dumm2.equals(otherTUC.dumm2);
Review Comment:
We don't need these changes in this PR.
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/PojoSerializerUpgradeTestSpecifications.java:
##########
@@ -229,19 +228,23 @@ public TypeSerializer<StaticSchemaPojo>
createUpgradedSerializer() {
TypeSerializer<StaticSchemaPojo> serializer =
TypeExtractor.createTypeInfo(StaticSchemaPojo.class)
.createSerializer(new SerializerConfigImpl());
- assertSame(PojoSerializer.class, serializer.getClass());
+ assertThat(serializer.getClass()).isSameAs(PojoSerializer.class);
return serializer;
}
@Override
- public Matcher<StaticSchemaPojo> testDataMatcher() {
- return is(new StaticSchemaPojo("Gordon", 27,
StaticSchemaPojo.Color.BLUE, false));
+ public Condition<StaticSchemaPojo> testDataCondition() {
+ return new Condition<>(
+ value ->
+ new StaticSchemaPojo("Gordon", 27,
StaticSchemaPojo.Color.BLUE, false)
+ .equals(value),
+ "");
Review Comment:
Can we provide a meaningful description?
There are many similar cases in this class.
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/RowComparatorTest.java:
##########
@@ -159,7 +160,7 @@ public boolean equals(Object o) {
MyPojo myPojo = (MyPojo) o;
- return name != null ? name.equals(myPojo.name) : myPojo.name ==
null;
+ return Objects.equals(name, myPojo.name);
Review Comment:
We don't need these changes int this PR.
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/PojoSubclassSerializerTest.java:
##########
@@ -98,10 +98,7 @@ public boolean equals(Object other) {
if (dumm1 != otherTUC.dumm1) {
return false;
}
- if (!dumm2.equals(otherTUC.dumm2)) {
- return false;
- }
- return true;
+ return dumm2.equals(otherTUC.dumm2);
Review Comment:
We don't need these changes in this PR.
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/kryo/KryoSerializerSnapshotTest.java:
##########
@@ -32,32 +32,32 @@
import org.apache.flink.core.memory.DataOutputSerializer;
import org.apache.flink.testutils.ClassLoaderUtils;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import java.io.IOException;
import java.io.Serializable;
-import static
org.apache.flink.api.common.typeutils.TypeSerializerMatchers.isCompatibleAsIs;
-import static
org.apache.flink.api.common.typeutils.TypeSerializerMatchers.isCompatibleWithReconfiguredSerializer;
-import static
org.apache.flink.api.common.typeutils.TypeSerializerMatchers.isIncompatible;
-import static org.hamcrest.MatcherAssert.assertThat;
+import static
org.apache.flink.api.common.typeutils.TypeSerializerConditions.isCompatibleAsIs;
+import static
org.apache.flink.api.common.typeutils.TypeSerializerConditions.isCompatibleWithReconfiguredSerializer;
+import static
org.apache.flink.api.common.typeutils.TypeSerializerConditions.isIncompatible;
+import static org.assertj.core.api.Assertions.assertThat;
/** Tests for {@link KryoSerializerSnapshot}. */
public class KryoSerializerSnapshotTest {
private SerializerConfigImpl oldConfig;
private SerializerConfigImpl newConfig;
- @Before
- public void setup() {
+ @BeforeEach
+ void setup() {
oldConfig = new SerializerConfigImpl();
newConfig = new SerializerConfigImpl();
}
@Test
public void sanityTest() {
Review Comment:
```suggestion
void sanityTest() {
```
--
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]
