Jiabao-Sun commented on code in PR #24670: URL: https://github.com/apache/flink/pull/24670#discussion_r1592131889
########## flink-core/src/test/java/org/apache/flink/util/CloseableIteratorTest.java: ########## @@ -17,48 +17,48 @@ package org.apache.flink.util; -import org.junit.Test; +import org.junit.jupiter.api.Test; import java.util.ArrayList; import java.util.List; import static java.util.Arrays.asList; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertFalse; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** {@link CloseableIterator} test. */ @SuppressWarnings("unchecked") -public class CloseableIteratorTest { +class CloseableIteratorTest { private static final String[] ELEMENTS = new String[] {"element-1", "element-2"}; @Test - public void testFlattenEmpty() throws Exception { + void testFlattenEmpty() throws Exception { List<CloseableIterator<?>> iterators = asList( CloseableIterator.flatten(), CloseableIterator.flatten(CloseableIterator.empty()), CloseableIterator.flatten(CloseableIterator.flatten())); for (CloseableIterator<?> i : iterators) { - assertFalse(i.hasNext()); + assertThat(i.hasNext()).isFalse(); Review Comment: ```suggestion assertThat(i).isExhausted(); ``` ########## flink-core/src/test/java/org/apache/flink/util/AbstractIDTest.java: ########## @@ -20,44 +20,40 @@ import org.apache.flink.core.testutils.CommonTestUtils; -import org.junit.Test; +import org.junit.jupiter.api.Test; import java.io.InputStream; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; /** This class contains tests for the {@link org.apache.flink.util.AbstractID} class. */ -public class AbstractIDTest extends TestLogger { +class AbstractIDTest { /** Tests the serialization/deserialization of an abstract ID. */ @Test - public void testSerialization() throws Exception { + void testSerialization() throws Exception { final AbstractID origID = new AbstractID(); final AbstractID copyID = CommonTestUtils.createCopySerializable(origID); - assertEquals(origID.hashCode(), copyID.hashCode()); - assertEquals(origID, copyID); + assertThat(copyID.hashCode()).isEqualTo(origID.hashCode()); Review Comment: ```suggestion assertThat(copyID).hasSameHashCodeAs(origID); ``` ########## flink-core/src/test/java/org/apache/flink/util/InstantiationUtilTest.java: ########## @@ -99,39 +97,40 @@ private String createProxyDefinition(String proxyName) { } @Test - public void testInstantiationOfStringValue() { + void testInstantiationOfStringValue() { StringValue stringValue = InstantiationUtil.instantiate(StringValue.class, null); - assertNotNull(stringValue); + assertThat(Optional.of(stringValue)).isNotNull(); Review Comment: ```java Object stringValue = InstantiationUtil.instantiate(StringValue.class, null); assertThat(stringValue).isNotNull(); ``` ########## flink-core/src/test/java/org/apache/flink/util/CollectionUtilTest.java: ########## @@ -138,15 +129,12 @@ public void testCheckedSubTypeCast() { list.add(null); Collection<B> castSuccess = CollectionUtil.checkedSubTypeCast(list, B.class); Iterator<B> iterator = castSuccess.iterator(); - Assertions.assertEquals(b, iterator.next()); - Assertions.assertEquals(c, iterator.next()); - Assertions.assertNull(iterator.next()); - Assertions.assertFalse(iterator.hasNext()); - try { - Collection<C> castFail = CollectionUtil.checkedSubTypeCast(list, C.class); - fail("Expected ClassCastException"); - } catch (ClassCastException expected) { - } + assertThat(iterator.next()).isEqualTo(b); + assertThat(iterator.next()).isEqualTo(c); + assertThat(iterator.next()).isNull(); + assertThat(iterator.hasNext()).isFalse(); Review Comment: ```suggestion assertThat(iterator).isExhausted(); ``` ########## flink-core/src/test/java/org/apache/flink/util/FileUtilsTest.java: ########## @@ -572,8 +566,6 @@ public void testExpandDirWithForbiddenEscape() { * @param outputFile the path of the output file * @param length the size of content to generate * @return MD5 of the output file - * @throws IOException - * @throws NoSuchAlgorithmException */ private static String generateTestFile(String outputFile, int length) Review Comment: We don't have to delete it just because there is no description provided for when the exception is thrown. ########## flink-core/src/test/java/org/apache/flink/util/CompressedSerializedValueTest.java: ########## @@ -20,56 +20,57 @@ import org.apache.flink.core.testutils.CommonTestUtils; -import org.junit.Test; +import org.junit.jupiter.api.Test; import java.util.Arrays; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNotNull; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** Tests for {@link CompressedSerializedValue}. */ -public class CompressedSerializedValueTest { +class CompressedSerializedValueTest { @Test - public void testSimpleValue() throws Exception { + void testSimpleValue() throws Exception { final String value = "teststring"; CompressedSerializedValue<String> v = CompressedSerializedValue.fromObject(value); CompressedSerializedValue<String> copy = CommonTestUtils.createCopySerializable(v); - assertEquals(value, v.deserializeValue(getClass().getClassLoader())); - assertEquals(value, copy.deserializeValue(getClass().getClassLoader())); + assertThat(v.deserializeValue(getClass().getClassLoader())).isEqualTo(value); + assertThat(copy.deserializeValue(getClass().getClassLoader())).isEqualTo(value); - assertEquals(v, copy); - assertEquals(v.hashCode(), copy.hashCode()); + assertThat(copy).isEqualTo(v); + assertThat(copy.hashCode()).isEqualTo(v.hashCode()); Review Comment: hasSameHashCodeAs ########## flink-core/src/test/java/org/apache/flink/util/ExceptionUtilsTest.java: ########## @@ -18,99 +18,89 @@ package org.apache.flink.util; -import org.junit.Test; - -import static org.hamcrest.CoreMatchers.sameInstance; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** Tests for the utility methods in {@link ExceptionUtils}. */ -public class ExceptionUtilsTest extends TestLogger { +class ExceptionUtilsTest { @Test - public void testStringifyNullException() { - assertNotNull(ExceptionUtils.STRINGIFIED_NULL_EXCEPTION); - assertEquals( - ExceptionUtils.STRINGIFIED_NULL_EXCEPTION, ExceptionUtils.stringifyException(null)); + void testStringifyNullException() { + assertThat(ExceptionUtils.STRINGIFIED_NULL_EXCEPTION).isNotNull(); + assertThat(ExceptionUtils.STRINGIFIED_NULL_EXCEPTION) + .isEqualTo(ExceptionUtils.stringifyException(null)); Review Comment: ```suggestion assertThat(ExceptionUtils.STRINGIFIED_NULL_EXCEPTION) .isNotNull() .isEqualTo(ExceptionUtils.stringifyException(null)); ``` ########## flink-core/src/test/java/org/apache/flink/util/LinkedOptionalMapTest.java: ########## @@ -124,42 +118,50 @@ public void mergingToEmpty() { first.putAll(second); - assertThat(first.keyNames(), contains("a", "b", "c", "d")); + assertThat(first.keyNames()).contains("a", "b", "c", "d"); } - @Test(expected = IllegalStateException.class) - public void unwrapOptionalsWithMissingValueThrows() { - LinkedOptionalMap<Class<?>, String> map = new LinkedOptionalMap<>(); + @Test + void unwrapOptionalsWithMissingValueThrows() { + assertThatThrownBy( + () -> { + LinkedOptionalMap<Class<?>, String> map = new LinkedOptionalMap<>(); - map.put("a", String.class, null); + map.put("a", String.class, null); - map.unwrapOptionals(); + map.unwrapOptionals(); + }) + .isInstanceOf(IllegalStateException.class); } - @Test(expected = IllegalStateException.class) - public void unwrapOptionalsWithMissingKeyThrows() { - LinkedOptionalMap<Class<?>, String> map = new LinkedOptionalMap<>(); + @Test + void unwrapOptionalsWithMissingKeyThrows() { + assertThatThrownBy( + () -> { + LinkedOptionalMap<Class<?>, String> map = new LinkedOptionalMap<>(); - map.put("a", null, "blabla"); + map.put("a", null, "blabla"); - map.unwrapOptionals(); + map.unwrapOptionals(); + }) + .isInstanceOf(IllegalStateException.class); } @Test - public void unwrapOptionalsPreservesOrder() { + void unwrapOptionalsPreservesOrder() { LinkedOptionalMap<Class<?>, String> map = new LinkedOptionalMap<>(); map.put("a", String.class, "aaa"); map.put("b", Boolean.class, "bbb"); LinkedHashMap<Class<?>, String> m = map.unwrapOptionals(); - assertThat(m.keySet(), contains(String.class, Boolean.class)); - assertThat(m.values(), contains("aaa", "bbb")); + assertThat(m).containsKey(String.class); + assertThat(m).containsValue("aaa"); Review Comment: Boolean.class and "bbb" is missing? ########## flink-core/src/test/java/org/apache/flink/util/LongValueSequenceIteratorTest.java: ########## @@ -57,15 +56,15 @@ private static void testSplitting( org.apache.flink.util.LongValueSequenceIterator iter, int numSplits) { org.apache.flink.util.LongValueSequenceIterator[] splits = iter.split(numSplits); - assertEquals(numSplits, splits.length); + assertThat(splits.length).isEqualTo(numSplits); Review Comment: ```suggestion assertThat(splits).hasSize(numSplits); ``` ########## flink-core/src/test/java/org/apache/flink/util/InstantiationUtilTest.java: ########## @@ -99,39 +97,40 @@ private String createProxyDefinition(String proxyName) { } @Test - public void testInstantiationOfStringValue() { + void testInstantiationOfStringValue() { StringValue stringValue = InstantiationUtil.instantiate(StringValue.class, null); - assertNotNull(stringValue); + assertThat(Optional.of(stringValue)).isNotNull(); } @Test - public void testInstantiationOfStringValueAndCastToValue() { + void testInstantiationOfStringValueAndCastToValue() { StringValue stringValue = InstantiationUtil.instantiate(StringValue.class, Value.class); - assertNotNull(stringValue); + assertThat(Optional.of(stringValue)).isNotNull(); Review Comment: same as above -- 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