Jiabao-Sun commented on code in PR #23960:
URL: https://github.com/apache/flink/pull/23960#discussion_r1474031993
##########
flink-core/src/test/java/org/apache/flink/api/common/typeutils/SerializerTestBase.java:
##########
@@ -163,7 +154,7 @@ protected void testSnapshotConfigurationAndReconfigure()
throws Exception {
} else {
throw new AssertionError("Unable to restore serializer with " +
strategy);
}
- assertEquals(serializer.getClass(), restoreSerializer.getClass());
+
assertThat(restoreSerializer.getClass()).isSameAs(serializer.getClass());
Review Comment:
hasSameClassAs is better.
##########
flink-core/src/test/java/org/apache/flink/api/common/typeutils/CompositeTypeSerializerSnapshotTest.java:
##########
@@ -98,16 +98,16 @@ public void
testCompatibleWithReconfiguredSerializerPrecedence() throws IOExcept
// nested serializer at index 1 should strictly be a
ReconfiguredNestedSerializer
// nested serializer at index 0 and 2 is RestoredNestedSerializer
after checking
// compatibility
- Assert.assertSame(
- reconfiguredNestedSerializers[0].getClass(),
RestoredNestedSerializer.class);
- Assert.assertSame(
- reconfiguredNestedSerializers[1].getClass(),
ReconfiguredNestedSerializer.class);
- Assert.assertSame(
- reconfiguredNestedSerializers[2].getClass(),
RestoredNestedSerializer.class);
+ assertThat(reconfiguredNestedSerializers[0].getClass())
+ .isSameAs(RestoredNestedSerializer.class);
+ assertThat(reconfiguredNestedSerializers[1].getClass())
+ .isSameAs(ReconfiguredNestedSerializer.class);
+ assertThat(reconfiguredNestedSerializers[2].getClass())
+ .isSameAs(RestoredNestedSerializer.class);
Review Comment:
hasSameClassAs
##########
flink-core/src/test/java/org/apache/flink/api/common/operators/ResourceSpecTest.java:
##########
@@ -222,61 +227,60 @@ public void testSubtract() {
.build();
final ResourceSpec subtracted = rs1.subtract(rs2);
- assertEquals(new CPUResource(0.8), subtracted.getCpuCores());
- assertEquals(0, subtracted.getTaskHeapMemory().getMebiBytes());
- assertEquals(
- new ExternalResource(EXTERNAL_RESOURCE_NAME, 0.6),
- subtracted.getExtendedResource(EXTERNAL_RESOURCE_NAME).get());
+ 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));
}
- @Test(expected = IllegalArgumentException.class)
- public void testSubtractOtherHasLargerResources() {
+ @Test
+ void testSubtractOtherHasLargerResources() {
final ResourceSpec rs1 = ResourceSpec.newBuilder(1.0, 100).build();
final ResourceSpec rs2 = ResourceSpec.newBuilder(0.2, 200).build();
- rs1.subtract(rs2);
+ assertThatThrownBy(() ->
rs1.subtract(rs2)).isInstanceOf(IllegalArgumentException.class);
}
@Test
- public void testSubtractThisUnknown() {
+ void testSubtractThisUnknown() {
final ResourceSpec rs1 = ResourceSpec.UNKNOWN;
final ResourceSpec rs2 =
ResourceSpec.newBuilder(0.2, 100)
.setExtendedResource(new
ExternalResource(EXTERNAL_RESOURCE_NAME, 0.5))
.build();
final ResourceSpec subtracted = rs1.subtract(rs2);
- assertEquals(ResourceSpec.UNKNOWN, subtracted);
+ assertThat(subtracted).isEqualTo(ResourceSpec.UNKNOWN);
}
@Test
- public void testSubtractOtherUnknown() {
+ void testSubtractOtherUnknown() {
final ResourceSpec rs1 =
ResourceSpec.newBuilder(1.0, 100)
.setExtendedResource(new
ExternalResource(EXTERNAL_RESOURCE_NAME, 1.1))
.build();
final ResourceSpec rs2 = ResourceSpec.UNKNOWN;
final ResourceSpec subtracted = rs1.subtract(rs2);
- assertEquals(ResourceSpec.UNKNOWN, subtracted);
+ assertThat(subtracted).isEqualTo(ResourceSpec.UNKNOWN);
}
@Test
- public void testZeroExtendedResourceFromConstructor() {
+ void testZeroExtendedResourceFromConstructor() {
final ResourceSpec resourceSpec =
ResourceSpec.newBuilder(1.0, 100)
.setExtendedResource(new
ExternalResource(EXTERNAL_RESOURCE_NAME, 0))
.build();
- assertEquals(resourceSpec.getExtendedResources().size(), 0);
+ assertThat(0).isEqualTo(resourceSpec.getExtendedResources().size());
}
@Test
- public void testZeroExtendedResourceFromSubtract() {
+ void testZeroExtendedResourceFromSubtract() {
final ResourceSpec resourceSpec =
ResourceSpec.newBuilder(1.0, 100)
.setExtendedResource(new
ExternalResource(EXTERNAL_RESOURCE_NAME, 1.0))
.build();
-
assertEquals(resourceSpec.subtract(resourceSpec).getExtendedResources().size(),
0);
+
assertThat(0).isEqualTo(resourceSpec.subtract(resourceSpec).getExtendedResources().size());
Review Comment:
```suggestion
assertThat(resourceSpec.subtract(resourceSpec).getExtendedResources()).isEmpty();
```
##########
flink-core/src/test/java/org/apache/flink/api/common/state/ReducingStateDescriptorTest.java:
##########
@@ -72,21 +71,21 @@ public void testHashCodeEquals() throws Exception {
// test that hashCode() works on state descriptors with initialized
and uninitialized
// serializers
- assertEquals(original.hashCode(), same.hashCode());
- assertEquals(original.hashCode(), sameBySerializer.hashCode());
+ assertThat(same.hashCode()).isEqualTo(original.hashCode());
+ assertThat(sameBySerializer.hashCode()).isEqualTo(original.hashCode());
Review Comment:
hasSameHashCodeAs is better
##########
flink-core/src/test/java/org/apache/flink/api/common/state/ValueStateDescriptorTest.java:
##########
@@ -46,26 +45,26 @@ public void testHashCodeEquals() throws Exception {
// test that hashCode() works on state descriptors with initialized
and uninitialized
// serializers
- assertEquals(original.hashCode(), same.hashCode());
- assertEquals(original.hashCode(), sameBySerializer.hashCode());
+ assertThat(same.hashCode()).isEqualTo(original.hashCode());
+ assertThat(sameBySerializer.hashCode()).isEqualTo(original.hashCode());
Review Comment:
hasSameHashCodeAs is better
##########
flink-core/src/test/java/org/apache/flink/api/common/operators/ResourceSpecTest.java:
##########
@@ -222,61 +227,60 @@ public void testSubtract() {
.build();
final ResourceSpec subtracted = rs1.subtract(rs2);
- assertEquals(new CPUResource(0.8), subtracted.getCpuCores());
- assertEquals(0, subtracted.getTaskHeapMemory().getMebiBytes());
- assertEquals(
- new ExternalResource(EXTERNAL_RESOURCE_NAME, 0.6),
- subtracted.getExtendedResource(EXTERNAL_RESOURCE_NAME).get());
+ 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));
}
- @Test(expected = IllegalArgumentException.class)
- public void testSubtractOtherHasLargerResources() {
+ @Test
+ void testSubtractOtherHasLargerResources() {
final ResourceSpec rs1 = ResourceSpec.newBuilder(1.0, 100).build();
final ResourceSpec rs2 = ResourceSpec.newBuilder(0.2, 200).build();
- rs1.subtract(rs2);
+ assertThatThrownBy(() ->
rs1.subtract(rs2)).isInstanceOf(IllegalArgumentException.class);
}
@Test
- public void testSubtractThisUnknown() {
+ void testSubtractThisUnknown() {
final ResourceSpec rs1 = ResourceSpec.UNKNOWN;
final ResourceSpec rs2 =
ResourceSpec.newBuilder(0.2, 100)
.setExtendedResource(new
ExternalResource(EXTERNAL_RESOURCE_NAME, 0.5))
.build();
final ResourceSpec subtracted = rs1.subtract(rs2);
- assertEquals(ResourceSpec.UNKNOWN, subtracted);
+ assertThat(subtracted).isEqualTo(ResourceSpec.UNKNOWN);
}
@Test
- public void testSubtractOtherUnknown() {
+ void testSubtractOtherUnknown() {
final ResourceSpec rs1 =
ResourceSpec.newBuilder(1.0, 100)
.setExtendedResource(new
ExternalResource(EXTERNAL_RESOURCE_NAME, 1.1))
.build();
final ResourceSpec rs2 = ResourceSpec.UNKNOWN;
final ResourceSpec subtracted = rs1.subtract(rs2);
- assertEquals(ResourceSpec.UNKNOWN, subtracted);
+ assertThat(subtracted).isEqualTo(ResourceSpec.UNKNOWN);
}
@Test
- public void testZeroExtendedResourceFromConstructor() {
+ void testZeroExtendedResourceFromConstructor() {
final ResourceSpec resourceSpec =
ResourceSpec.newBuilder(1.0, 100)
.setExtendedResource(new
ExternalResource(EXTERNAL_RESOURCE_NAME, 0))
.build();
- assertEquals(resourceSpec.getExtendedResources().size(), 0);
+ assertThat(0).isEqualTo(resourceSpec.getExtendedResources().size());
Review Comment:
```suggestion
assertThat(resourceSpec.getExtendedResources()).isEmpty();
```
##########
flink-core/src/test/java/org/apache/flink/api/common/resources/ResourceTest.java:
##########
@@ -20,194 +20,198 @@
import org.apache.flink.util.TestLogger;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
import java.math.BigDecimal;
-import static org.hamcrest.Matchers.greaterThan;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.lessThan;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
/** Tests for {@link Resource}. */
-public class ResourceTest extends TestLogger {
+@SuppressWarnings("rawtypes")
+class ResourceTest extends TestLogger {
@Test
- public void testConstructorValid() {
+ void testConstructorValid() {
final Resource v1 = new TestResource(0.1);
assertTestResourceValueEquals(0.1, v1);
final Resource v2 = new TestResource(BigDecimal.valueOf(0.1));
assertTestResourceValueEquals(0.1, v2);
}
- @Test(expected = IllegalArgumentException.class)
- public void testConstructorInvalidValue() {
- new TestResource(-0.1);
+ @Test
+ void testConstructorInvalidValue() {
+ assertThatThrownBy(() -> new TestResource(-0.1))
+ .isInstanceOf(IllegalArgumentException.class);
}
@Test
- public void testEquals() {
+ void testEquals() {
final Resource v1 = new TestResource(0.1);
final Resource v2 = new TestResource(0.1);
final Resource v3 = new TestResource(0.2);
- assertTrue(v1.equals(v2));
- assertFalse(v1.equals(v3));
+ assertThat(v2).isEqualTo(v1);
+ assertThat(v3).isNotEqualTo(v1);
}
@Test
- public void testEqualsIgnoringScale() {
+ void testEqualsIgnoringScale() {
final Resource v1 = new TestResource(new BigDecimal("0.1"));
final Resource v2 = new TestResource(new BigDecimal("0.10"));
- assertTrue(v1.equals(v2));
+ assertThat(v2).isEqualTo(v1);
}
@Test
- public void testHashCodeIgnoringScale() {
+ void testHashCodeIgnoringScale() {
final Resource v1 = new TestResource(new BigDecimal("0.1"));
final Resource v2 = new TestResource(new BigDecimal("0.10"));
- assertTrue(v1.hashCode() == v2.hashCode());
+ assertThat(v2.hashCode()).isEqualTo(v1.hashCode());
Review Comment:
hasSameHashCodeAs 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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
