Jiabao-Sun commented on code in PR #24603:
URL: https://github.com/apache/flink/pull/24603#discussion_r1547495311
##########
flink-core/src/test/java/org/apache/flink/api/common/operators/ResourceSpecTest.java:
##########
@@ -141,12 +141,12 @@ void testMerge() {
ResourceSpec rs3 = rs1.merge(rs2);
assertThat(rs3.getCpuCores()).isEqualTo(new CPUResource(2.0));
assertThat(rs3.getTaskHeapMemory().getMebiBytes()).isEqualTo(200);
- assertThat(rs3.getExtendedResource(EXTERNAL_RESOURCE_NAME).get())
- .isEqualTo(new ExternalResource(EXTERNAL_RESOURCE_NAME, 1.1));
+ assertThat(rs3.getExtendedResource(EXTERNAL_RESOURCE_NAME))
+ .contains(new ExternalResource(EXTERNAL_RESOURCE_NAME, 1.1));
Review Comment:
```suggestion
.hasValue(new ExternalResource(EXTERNAL_RESOURCE_NAME, 1.1));
```
##########
flink-core/src/test/java/org/apache/flink/api/common/operators/ExpressionKeysTest.java:
##########
@@ -58,8 +57,8 @@ void testGenericNonKeyType() {
// Fail: GenericType cannot be used as key
TypeInformation<GenericNonKeyType> genericType =
new GenericTypeInfo<>(GenericNonKeyType.class);
- assertThatExceptionOfType(InvalidProgramException.class)
- .isThrownBy(() -> new ExpressionKeys<>("*", genericType));
+ assertThatThrownBy(() -> new ExpressionKeys<>("*", genericType))
+ .isInstanceOf(InvalidProgramException.class);
Review Comment:
I don't think it's necessary to make any changes from
`assertThatExceptionOfType` to `assertThatThrownBy`.
##########
flink-core/src/test/java/org/apache/flink/api/java/ClosureCleanerTest.java:
##########
@@ -78,24 +85,29 @@ public void testNestedSerializable() throws Exception {
ClosureCleaner.ensureSerializable(map);
int result = map.map(3);
- Assert.assertEquals(result, 4);
+ assertThat(result).isEqualTo(4);
}
- @Test(expected = InvalidProgramException.class)
- public void testNestedNonSerializable() throws Exception {
- MapCreator creator = new NestedNonSerializableMapCreator(1);
- MapFunction<Integer, Integer> map = creator.getMap();
+ @Test
+ void testNestedNonSerializable() throws Exception {
+ assertThatThrownBy(
+ () -> {
+ MapCreator creator = new
NestedNonSerializableMapCreator(1);
+ MapFunction<Integer, Integer> map =
creator.getMap();
Review Comment:
Is the scope too large in the lambda?
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/PojoTypeExtractionTest.java:
##########
@@ -260,99 +262,98 @@ private void checkWCPojoAsserts(TypeInformation<?>
typeInfo) {
"complex.word.f2"
};
int[] positions = {9, 1, 0, 2, 3, 4, 5, 6, 7, 8};
- Assert.assertEquals(fields.length, positions.length);
+ assertThat(positions.length).isEqualTo(fields.length);
for (int i = 0; i < fields.length; i++) {
pojoType.getFlatFields(fields[i], 0, ffd);
- Assert.assertEquals("Too many keys returned", 1, ffd.size());
- Assert.assertEquals(
- "position of field " + fields[i] + " wrong",
- positions[i],
- ffd.get(0).getPosition());
+ assertThat(ffd.size()).as("Too many keys returned").isOne();
Review Comment:
```suggestion
assertThat(ffd).as("Too many keys returned").isEmpty();
```
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/PojoTypeExtractionTest.java:
##########
@@ -260,99 +262,98 @@ private void checkWCPojoAsserts(TypeInformation<?>
typeInfo) {
"complex.word.f2"
};
int[] positions = {9, 1, 0, 2, 3, 4, 5, 6, 7, 8};
- Assert.assertEquals(fields.length, positions.length);
+ assertThat(positions.length).isEqualTo(fields.length);
for (int i = 0; i < fields.length; i++) {
pojoType.getFlatFields(fields[i], 0, ffd);
- Assert.assertEquals("Too many keys returned", 1, ffd.size());
- Assert.assertEquals(
- "position of field " + fields[i] + " wrong",
- positions[i],
- ffd.get(0).getPosition());
+ assertThat(ffd.size()).as("Too many keys returned").isOne();
+ assertThat(ffd.get(0).getPosition())
+ .as("position of field " + fields[i] + " wrong")
+ .isEqualTo(positions[i]);
ffd.clear();
}
pojoType.getFlatFields("complex.word.*", 0, ffd);
- Assert.assertEquals(3, ffd.size());
+ assertThat(ffd).hasSize(3);
// check if it returns 5,6,7
for (FlatFieldDescriptor ffdE : ffd) {
final int pos = ffdE.getPosition();
- Assert.assertTrue(pos <= 8);
- Assert.assertTrue(6 <= pos);
+ assertThat(pos <= 8).isTrue();
+ assertThat(6 <= pos).isTrue();
if (pos == 6) {
- Assert.assertEquals(Long.class, ffdE.getType().getTypeClass());
+
assertThat(ffdE.getType().getTypeClass()).isEqualTo(Long.class);
}
if (pos == 7) {
- Assert.assertEquals(Long.class, ffdE.getType().getTypeClass());
+
assertThat(ffdE.getType().getTypeClass()).isEqualTo(Long.class);
}
if (pos == 8) {
- Assert.assertEquals(String.class,
ffdE.getType().getTypeClass());
+
assertThat(ffdE.getType().getTypeClass()).isEqualTo(String.class);
}
}
ffd.clear();
// scala style full tuple selection for pojos
pojoType.getFlatFields("complex.word._", 0, ffd);
- Assert.assertEquals(3, ffd.size());
+ assertThat(ffd).hasSize(3);
ffd.clear();
pojoType.getFlatFields("complex.*", 0, ffd);
- Assert.assertEquals(9, ffd.size());
+ assertThat(ffd).hasSize(9);
// check if it returns 0-7
for (FlatFieldDescriptor ffdE : ffd) {
final int pos = ffdE.getPosition();
- Assert.assertTrue(ffdE.getPosition() <= 8);
- Assert.assertTrue(0 <= ffdE.getPosition());
+ assertThat(ffdE.getPosition() <= 8).isTrue();
+ assertThat(0 <= ffdE.getPosition()).isTrue();
Review Comment:
`isLessThanOrEqualTo`
##########
flink-core/src/test/java/org/apache/flink/api/common/operators/ResourceSpecTest.java:
##########
@@ -141,12 +141,12 @@ void testMerge() {
ResourceSpec rs3 = rs1.merge(rs2);
assertThat(rs3.getCpuCores()).isEqualTo(new CPUResource(2.0));
assertThat(rs3.getTaskHeapMemory().getMebiBytes()).isEqualTo(200);
- assertThat(rs3.getExtendedResource(EXTERNAL_RESOURCE_NAME).get())
- .isEqualTo(new ExternalResource(EXTERNAL_RESOURCE_NAME, 1.1));
+ assertThat(rs3.getExtendedResource(EXTERNAL_RESOURCE_NAME))
+ .contains(new ExternalResource(EXTERNAL_RESOURCE_NAME, 1.1));
ResourceSpec rs4 = rs1.merge(rs3);
- assertThat(rs4.getExtendedResource(EXTERNAL_RESOURCE_NAME).get())
- .isEqualTo(new ExternalResource(EXTERNAL_RESOURCE_NAME, 2.2));
+ assertThat(rs4.getExtendedResource(EXTERNAL_RESOURCE_NAME))
+ .contains(new ExternalResource(EXTERNAL_RESOURCE_NAME, 2.2));
Review Comment:
```suggestion
.hasValue(new ExternalResource(EXTERNAL_RESOURCE_NAME, 2.2));
```
##########
flink-core/src/test/java/org/apache/flink/api/connector/source/lib/NumberSequenceSourceTest.java:
##########
@@ -94,6 +94,7 @@ private static void validateSequence(
private static void failSequence(final List<Long> sequence, final long
from, final long to) {
fail(
+ "",
Review Comment:
why we need this?
##########
flink-core/src/test/java/org/apache/flink/api/common/operators/ResourceSpecTest.java:
##########
@@ -228,8 +228,8 @@ void testSubtract() {
final ResourceSpec subtracted = rs1.subtract(rs2);
assertThat(subtracted.getCpuCores()).isEqualTo(new CPUResource(0.8));
assertThat(subtracted.getTaskHeapMemory().getMebiBytes()).isZero();
-
assertThat(subtracted.getExtendedResource(EXTERNAL_RESOURCE_NAME).get())
- .isEqualTo(new ExternalResource(EXTERNAL_RESOURCE_NAME, 0.6));
+ assertThat(subtracted.getExtendedResource(EXTERNAL_RESOURCE_NAME))
+ .contains(new ExternalResource(EXTERNAL_RESOURCE_NAME, 0.6));
Review Comment:
```suggestion
.hasValue(new ExternalResource(EXTERNAL_RESOURCE_NAME, 0.6));
```
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/PojoTypeExtractionTest.java:
##########
@@ -240,10 +242,10 @@ public void testPojoWC() {
@SuppressWarnings({"unchecked", "rawtypes"})
private void checkWCPojoAsserts(TypeInformation<?> typeInfo) {
- Assert.assertFalse(typeInfo.isBasicType());
- Assert.assertFalse(typeInfo.isTupleType());
- Assert.assertEquals(10, typeInfo.getTotalFields());
- Assert.assertTrue(typeInfo instanceof PojoTypeInfo);
+ assertThat(typeInfo.isBasicType()).isFalse();
+ assertThat(typeInfo.isTupleType()).isFalse();
+ assertThat(typeInfo.getTotalFields()).isEqualTo(10);
+ assertThat(typeInfo instanceof PojoTypeInfo).isTrue();
Review Comment:
`isInstanceOf()`
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/PojoTypeExtractionTest.java:
##########
@@ -260,99 +262,98 @@ private void checkWCPojoAsserts(TypeInformation<?>
typeInfo) {
"complex.word.f2"
};
int[] positions = {9, 1, 0, 2, 3, 4, 5, 6, 7, 8};
- Assert.assertEquals(fields.length, positions.length);
+ assertThat(positions.length).isEqualTo(fields.length);
for (int i = 0; i < fields.length; i++) {
pojoType.getFlatFields(fields[i], 0, ffd);
- Assert.assertEquals("Too many keys returned", 1, ffd.size());
- Assert.assertEquals(
- "position of field " + fields[i] + " wrong",
- positions[i],
- ffd.get(0).getPosition());
+ assertThat(ffd.size()).as("Too many keys returned").isOne();
+ assertThat(ffd.get(0).getPosition())
+ .as("position of field " + fields[i] + " wrong")
+ .isEqualTo(positions[i]);
ffd.clear();
}
pojoType.getFlatFields("complex.word.*", 0, ffd);
- Assert.assertEquals(3, ffd.size());
+ assertThat(ffd).hasSize(3);
// check if it returns 5,6,7
for (FlatFieldDescriptor ffdE : ffd) {
final int pos = ffdE.getPosition();
- Assert.assertTrue(pos <= 8);
- Assert.assertTrue(6 <= pos);
+ assertThat(pos <= 8).isTrue();
+ assertThat(6 <= pos).isTrue();
Review Comment:
`isLessThanOrEqualTo`
##########
flink-core/src/test/java/org/apache/flink/api/java/ClosureCleanerTest.java:
##########
@@ -24,52 +24,59 @@
import org.apache.flink.api.java.tuple.Tuple1;
import org.apache.flink.util.function.SerializableSupplier;
-import org.junit.Assert;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static
org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
+
/** Tests for {@link ClosureCleaner}. */
-public class ClosureCleanerTest {
+class ClosureCleanerTest {
- @Test(expected = InvalidProgramException.class)
- public void testNonSerializable() throws Exception {
- MapCreator creator = new NonSerializableMapCreator();
- MapFunction<Integer, Integer> map = creator.getMap();
+ @Test
+ void testNonSerializable() {
+ assertThatThrownBy(
+ () -> {
+ MapCreator creator = new
NonSerializableMapCreator();
+ MapFunction<Integer, Integer> map =
creator.getMap();
Review Comment:
Is the scope too large in the lambda?
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/PojoTypeExtractionTest.java:
##########
@@ -260,99 +262,98 @@ private void checkWCPojoAsserts(TypeInformation<?>
typeInfo) {
"complex.word.f2"
};
int[] positions = {9, 1, 0, 2, 3, 4, 5, 6, 7, 8};
- Assert.assertEquals(fields.length, positions.length);
+ assertThat(positions.length).isEqualTo(fields.length);
Review Comment:
```suggestion
assertThat(fields).hasSameSizeAs(positions);
```
##########
flink-core/src/test/java/org/apache/flink/api/java/typeutils/PojoTypeExtractionTest.java:
##########
@@ -71,9 +73,9 @@ public static class HasDuplicateField extends WC {
}
@Test
- public void testDuplicateFieldException() {
+ void testDuplicateFieldException() {
TypeInformation<?> ti =
TypeExtractor.createTypeInfo(HasDuplicateField.class);
- Assert.assertTrue(ti instanceof GenericTypeInfo<?>);
+ assertThat(ti instanceof GenericTypeInfo<?>).isTrue();
Review Comment:
I think `isInstanceOf()` is better.
--
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]
