1996fanrui commented on code in PR #23233:
URL: https://github.com/apache/flink/pull/23233#discussion_r1306383828
##########
flink-runtime/src/test/java/org/apache/flink/runtime/operators/sort/ExternalSortLargeRecordsITCase.java:
##########
@@ -402,11 +400,11 @@ public Tuple2<Long, SmallOrMediumOrLargeValue> next() {
for (int i = 0; i < NUM_RECORDS; i++) {
val = iterator.next(val);
- assertTrue(val.f0 <= prevKey);
- assertTrue(val.f0.intValue() == val.f1.val());
+ assertThat(val.f0).isLessThanOrEqualTo(prevKey);
+ assertThat(val.f0.intValue()).isEqualTo(val.f1.val());
Review Comment:
```suggestion
assertThat(val.f0).isLessThanOrEqualTo(prevKey).isEqualTo(val.f1.val());
```
Other `intValue()` of this class may be removed as well.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/operators/sort/LargeRecordHandlerITCase.java:
##########
@@ -267,8 +258,8 @@ public void fileTest() {
Tuple3<Long, SomeVeryLongValue, Byte> next =
serializer.deserialize(in);
// key and value must be equal
- assertTrue(next.f0.intValue() == next.f1.val());
- assertTrue(next.f0.byteValue() == next.f2);
+ assertThat(next.f0.intValue()).isEqualTo(next.f1.val());
+ assertThat(next.f0.byteValue()).isEqualTo(next.f2);
Review Comment:
Same
##########
flink-runtime/src/test/java/org/apache/flink/runtime/operators/drivers/GroupReduceDriverTest.java:
##########
@@ -235,22 +237,16 @@ public void
testAllReduceDriverIncorrectlyAccumulatingMutable() {
Object[] res = result.getList().toArray();
Object[] expected =
DriverTestData.createReduceMutableDataGroupedResult().toArray();
- try {
- DriverTestData.compareTupleArrays(expected, res);
- Assert.fail(
- "Accumulationg mutable objects is expected to result
in incorrect values.");
- } catch (AssertionError e) {
- // expected
- }
+ DriverTestData.compareTupleArrays(expected, res);
Review Comment:
Sounds make sense, I'm unsure is there a possible
`DriverTestData.compareTupleArrays(expected, res)` throws an `AssertionError`.
I wonder why FLINK-1005 added the catch. If
`DriverTestData.compareTupleArrays(expected, res)` never throw
`AssertionError`, this change makes sense to me.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/operators/sort/LargeRecordHandlerITCase.java:
##########
@@ -100,61 +98,54 @@ public void testRecordHandlerCompositeKey() {
128,
owner.getExecutionConfig());
- assertFalse(handler.hasData());
+ assertThat(handler.hasData()).isFalse();
// add the test data
Random rnd = new Random();
for (int i = 0; i < NUM_RECORDS; i++) {
long val = rnd.nextLong();
- handler.addRecord(
- new Tuple3<Long, SomeVeryLongValue, Byte>(
- val, new SomeVeryLongValue((int) val), (byte)
val));
- assertTrue(handler.hasData());
+ handler.addRecord(new Tuple3<>(val, new
SomeVeryLongValue((int) val), (byte) val));
+ assertThat(handler.hasData()).isTrue();
}
MutableObjectIterator<Tuple3<Long, SomeVeryLongValue, Byte>>
sorted =
handler.finishWriteAndSortKeys(sortMemory);
- try {
- handler.addRecord(new Tuple3<Long, SomeVeryLongValue,
Byte>(92L, null, (byte) 1));
- fail("should throw an exception");
- } catch (IllegalStateException e) {
- // expected
- }
+ assertThatThrownBy(() -> handler.addRecord(new Tuple3<>(92L, null,
(byte) 1)))
+ .withFailMessage("should throw an exception")
+ .isInstanceOf(IllegalStateException.class);
Tuple3<Long, SomeVeryLongValue, Byte> previous = null;
Tuple3<Long, SomeVeryLongValue, Byte> next;
while ((next = sorted.next(null)) != null) {
// key and value must be equal
- assertTrue(next.f0.intValue() == next.f1.val());
- assertTrue(next.f0.byteValue() == next.f2);
+ assertThat(next.f0.intValue()).isEqualTo(next.f1.val());
+ assertThat(next.f0.byteValue()).isEqualTo(next.f2);
Review Comment:
Same to the last comment.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/operators/hash/HashTableTest.java:
##########
@@ -144,10 +144,9 @@ public void testBufferMissingForProbing() {
while (matches.next() != null) ;
}
} catch (RuntimeException e) {
- if (!e.getMessage().contains("exceeded maximum number of
recursions")) {
- e.printStackTrace();
- fail("Test failed with unexpected exception");
- }
+ assertThat(e)
+ .withFailMessage("Test failed with unexpected
exception")
+ .hasMessageContaining("exceeded maximum number of
recursions");
Review Comment:
`assertThrownXXX` may be better here.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/operators/sort/MergeIteratorTest.java:
##########
@@ -104,15 +105,15 @@ public void testMergeOfTwoStreams() throws Exception {
int pos = 1;
- Assert.assertTrue((rec1 = iterator.next(rec1)) != null);
- Assert.assertEquals(expected[0], rec1.f0.intValue());
+ assertThat(rec1 = iterator.next(rec1)).isNotNull();
+ assertThat(rec1.f0.intValue()).isEqualTo(expected[0]);
Review Comment:
You can help search similar code globally in `operators` package, and I
won't comment them, thanks~
--
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
