1996fanrui commented on code in PR #23265:
URL: https://github.com/apache/flink/pull/23265#discussion_r1322913640
##########
flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java:
##########
@@ -216,18 +214,18 @@ void startTokensUpdate() {
scheduler.triggerAll();
delegationTokenManager.stopTokensUpdate();
- assertEquals(3, startTokensUpdateCallCount.get());
+ assertThat(startTokensUpdateCallCount.get()).isEqualTo(3);
Review Comment:
```suggestion
assertThat(startTokensUpdateCallCount).hasValue(3);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/security/token/DefaultDelegationTokenManagerTest.java:
##########
@@ -40,154 +40,152 @@
import static java.time.Instant.ofEpochMilli;
import static
org.apache.flink.configuration.SecurityOptions.DELEGATION_TOKENS_RENEWAL_TIME_RATIO;
import static
org.apache.flink.core.security.token.DelegationTokenProvider.CONFIG_PREFIX;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
/** Test for {@link DelegationTokenManager}. */
-public class DefaultDelegationTokenManagerTest {
+class DefaultDelegationTokenManagerTest {
@BeforeEach
- public void beforeEach() {
+ void beforeEach() {
ExceptionThrowingDelegationTokenProvider.reset();
ExceptionThrowingDelegationTokenReceiver.reset();
}
@AfterEach
- public void afterEach() {
+ void afterEach() {
ExceptionThrowingDelegationTokenProvider.reset();
ExceptionThrowingDelegationTokenReceiver.reset();
}
@Test
- public void isProviderEnabledMustGiveBackTrueByDefault() {
+ void isProviderEnabledMustGiveBackTrueByDefault() {
Configuration configuration = new Configuration();
-
assertTrue(DefaultDelegationTokenManager.isProviderEnabled(configuration,
"test"));
+
assertThat(DefaultDelegationTokenManager.isProviderEnabled(configuration,
"test")).isTrue();
}
@Test
- public void isProviderEnabledMustGiveBackFalseWhenDisabled() {
+ void isProviderEnabledMustGiveBackFalseWhenDisabled() {
Configuration configuration = new Configuration();
configuration.setBoolean(CONFIG_PREFIX + ".test.enabled", false);
-
assertFalse(DefaultDelegationTokenManager.isProviderEnabled(configuration,
"test"));
+
assertThat(DefaultDelegationTokenManager.isProviderEnabled(configuration,
"test"))
+ .isFalse();
}
@Test
- public void configurationIsNullMustFailFast() {
- assertThrows(
- Exception.class, () -> new DefaultDelegationTokenManager(null,
null, null, null));
+ void configurationIsNullMustFailFast() {
+ assertThatThrownBy(() -> new DefaultDelegationTokenManager(null, null,
null, null))
+ .isInstanceOf(Exception.class);
}
@Test
- public void oneProviderThrowsExceptionMustFailFast() {
- assertThrows(
- Exception.class,
- () -> {
-
ExceptionThrowingDelegationTokenProvider.throwInInit.set(true);
- new DefaultDelegationTokenManager(new Configuration(),
null, null, null);
- });
+ void oneProviderThrowsExceptionMustFailFast() {
+ assertThatThrownBy(
+ () -> {
+
ExceptionThrowingDelegationTokenProvider.throwInInit.set(true);
+ new DefaultDelegationTokenManager(
+ new Configuration(), null, null, null);
+ })
+ .isInstanceOf(Exception.class);
}
@Test
- public void testAllProvidersLoaded() {
+ void testAllProvidersLoaded() {
Configuration configuration = new Configuration();
configuration.setBoolean(CONFIG_PREFIX + ".throw.enabled", false);
DefaultDelegationTokenManager delegationTokenManager =
new DefaultDelegationTokenManager(configuration, null, null,
null);
- assertEquals(3,
delegationTokenManager.delegationTokenProviders.size());
+ assertThat(delegationTokenManager.delegationTokenProviders).hasSize(3);
- assertTrue(delegationTokenManager.isProviderLoaded("hadoopfs"));
- assertTrue(delegationTokenManager.isReceiverLoaded("hadoopfs"));
+
assertThat(delegationTokenManager.isProviderLoaded("hadoopfs")).isTrue();
+
assertThat(delegationTokenManager.isReceiverLoaded("hadoopfs")).isTrue();
- assertTrue(delegationTokenManager.isProviderLoaded("hbase"));
- assertTrue(delegationTokenManager.isReceiverLoaded("hbase"));
+ assertThat(delegationTokenManager.isProviderLoaded("hbase")).isTrue();
+ assertThat(delegationTokenManager.isReceiverLoaded("hbase")).isTrue();
- assertTrue(delegationTokenManager.isProviderLoaded("test"));
- assertTrue(delegationTokenManager.isReceiverLoaded("test"));
+ assertThat(delegationTokenManager.isProviderLoaded("test")).isTrue();
+ assertThat(delegationTokenManager.isReceiverLoaded("test")).isTrue();
- assertTrue(ExceptionThrowingDelegationTokenProvider.constructed.get());
- assertTrue(ExceptionThrowingDelegationTokenReceiver.constructed.get());
- assertFalse(delegationTokenManager.isProviderLoaded("throw"));
- assertFalse(delegationTokenManager.isReceiverLoaded("throw"));
+
assertThat(ExceptionThrowingDelegationTokenProvider.constructed.get()).isTrue();
+
assertThat(ExceptionThrowingDelegationTokenReceiver.constructed.get()).isTrue();
+ assertThat(delegationTokenManager.isProviderLoaded("throw")).isFalse();
+ assertThat(delegationTokenManager.isReceiverLoaded("throw")).isFalse();
}
@Test
- public void
checkProviderAndReceiverConsistencyShouldNotThrowWhenNothingLoaded() {
+ void checkProviderAndReceiverConsistencyShouldNotThrowWhenNothingLoaded() {
DefaultDelegationTokenManager.checkProviderAndReceiverConsistency(
Collections.emptyMap(), Collections.emptyMap());
}
@Test
- public void
checkProviderAndReceiverConsistencyShouldThrowWhenMissingReceiver() {
+ void checkProviderAndReceiverConsistencyShouldThrowWhenMissingReceiver() {
Map<String, DelegationTokenProvider> providers = new HashMap<>();
providers.put("test", new TestDelegationTokenProvider());
- IllegalStateException e =
- assertThrows(
- IllegalStateException.class,
+ assertThatThrownBy(
() ->
DefaultDelegationTokenManager.checkProviderAndReceiverConsistency(
- providers, Collections.emptyMap()));
- assertTrue(e.getMessage().contains("Missing receivers: test"));
+ providers, Collections.emptyMap()))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessageContaining("Missing receivers: test");
}
@Test
- public void
checkProviderAndReceiverConsistencyShouldThrowWhenMissingProvider() {
+ void checkProviderAndReceiverConsistencyShouldThrowWhenMissingProvider() {
Map<String, DelegationTokenReceiver> receivers = new HashMap<>();
receivers.put("test", new TestDelegationTokenReceiver());
- IllegalStateException e =
- assertThrows(
- IllegalStateException.class,
+ assertThatThrownBy(
() ->
DefaultDelegationTokenManager.checkProviderAndReceiverConsistency(
- Collections.emptyMap(), receivers));
- assertTrue(e.getMessage().contains("Missing providers: test"));
+ Collections.emptyMap(), receivers))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessageContaining("Missing providers: test");
}
@Test
- public void
checkProviderAndReceiverConsistencyShouldNotThrowWhenBothLoaded() {
+ void checkProviderAndReceiverConsistencyShouldNotThrowWhenBothLoaded() {
Map<String, DelegationTokenProvider> providers = new HashMap<>();
providers.put("test", new TestDelegationTokenProvider());
Map<String, DelegationTokenReceiver> receivers = new HashMap<>();
receivers.put("test", new TestDelegationTokenReceiver());
DefaultDelegationTokenManager.checkProviderAndReceiverConsistency(providers,
receivers);
- assertEquals(1, providers.size());
- assertTrue(providers.containsKey("test"));
- assertEquals(1, receivers.size());
- assertTrue(receivers.containsKey("test"));
+ assertThat(providers).hasSize(1);
+ assertThat(providers).containsKey("test");
+ assertThat(receivers).hasSize(1);
+ assertThat(receivers).containsKey("test");
}
@Test
- public void
checkSamePrefixedProvidersShouldNotGiveErrorsWhenNoSamePrefix() {
+ void checkSamePrefixedProvidersShouldNotGiveErrorsWhenNoSamePrefix() {
Map<String, DelegationTokenProvider> providers = new HashMap<>();
providers.put("s3-hadoop", new TestDelegationTokenProvider());
Set<String> warnings = new HashSet<>();
DefaultDelegationTokenManager.checkSamePrefixedProviders(providers,
warnings);
- assertTrue(warnings.isEmpty());
+ assertThat(warnings.isEmpty()).isTrue();
Review Comment:
```suggestion
assertThat(warnings).isEmpty();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/taskmanager/TaskExecutionStateTest.java:
##########
@@ -48,16 +48,16 @@ public void testEqualsHashCode() {
TaskExecutionState s1 = new TaskExecutionState(executionId, state,
error);
TaskExecutionState s2 = new TaskExecutionState(executionId, state,
error);
- assertEquals(s1.hashCode(), s2.hashCode());
- assertEquals(s1, s2);
+ assertThat(s2.hashCode()).isEqualTo(s1.hashCode());
+ assertThat(s2).isEqualTo(s1);
Review Comment:
```suggestion
assertThat(s2).hasSameHashCodeAs(s1).isEqualTo(s1);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/source/coordinator/SplitAssignmentTrackerTest.java:
##########
@@ -60,24 +57,24 @@ public void testOnCheckpoint() throws Exception {
tracker.onCheckpoint(checkpointId);
// Verify the uncheckpointed assignments.
- assertTrue(tracker.uncheckpointedAssignments().isEmpty());
+ assertThat(tracker.uncheckpointedAssignments().isEmpty()).isTrue();
Review Comment:
```suggestion
assertThat(tracker.uncheckpointedAssignments()).isEmpty();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/taskmanager/TaskTest.java:
##########
@@ -397,11 +385,11 @@ public void testExecutionFailsInInvoke() throws Exception
{
task.run();
- assertEquals(ExecutionState.FAILED, task.getExecutionState());
- assertTrue(task.isCanceledOrFailed());
- assertNotNull(task.getFailureCause());
- assertNotNull(task.getFailureCause().getMessage());
- assertTrue(task.getFailureCause().getMessage().contains("test"));
+ assertThat(task.getExecutionState()).isEqualTo(ExecutionState.FAILED);
+ assertThat(task.isCanceledOrFailed()).isTrue();
+ assertThat(task.getFailureCause()).isNotNull();
+ assertThat(task.getFailureCause().getMessage()).isNotNull();
+ assertThat(task.getFailureCause().getMessage()).contains("test");
Review Comment:
```suggestion
assertThat(task.getFailureCause().getMessage()).isNotNull().contains("test");
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/security/token/hadoop/HadoopFSDelegationTokenProviderITCase.java:
##########
@@ -167,52 +163,58 @@ public void
getTokenRenewalDateShouldReturnMinWhenMultipleTokens() {
credentials.addToken(
tokenService2, new TestDelegationToken(tokenService2,
tokenIdentifier2));
- assertEquals(
- Optional.of(NOW + 1),
provider.getTokenRenewalDate(constantClock, credentials, 1));
+ assertThat(provider.getTokenRenewalDate(constantClock, credentials, 1))
+ .isEqualTo(Optional.of(NOW + 1));
Review Comment:
```suggestion
.hasValue(NOW + 1);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/source/coordinator/ExecutorNotifierTest.java:
##########
@@ -79,24 +78,24 @@ public void testBasic() throws InterruptedException {
latch.countDown();
});
latch.await();
- assertEquals(1234, result.get());
+ assertThat(result.get()).isEqualTo(1234);
Review Comment:
```suggestion
assertThat(result).hasValue(1234);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/taskmanager/TaskManagerLocationTest.java:
##########
@@ -116,120 +120,127 @@ void testEqualsHashAndCompareToWithDifferentNodeId()
throws Exception {
assertThat(one.hashCode()).isNotEqualTo(two.hashCode());
assertThat(two.hashCode()).isNotEqualTo(three.hashCode());
- assertThat(one.compareTo(three)).isEqualTo(0);
- assertThat(one.compareTo(two)).isNotEqualTo(0);
- assertThat(two.compareTo(three)).isNotEqualTo(0);
+ assertThat(one.compareTo(three)).isZero();
+ assertThat(one.compareTo(two)).isNotZero();
+ assertThat(two.compareTo(three)).isNotZero();
Review Comment:
```suggestion
assertThat(one).isEqualByComparingTo(three);
assertThat(one).isNotEqualByComparingTo(two);
assertThat(two).isNotEqualByComparingTo(three);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/taskmanager/TaskManagerLocationTest.java:
##########
@@ -38,57 +38,61 @@ class TaskManagerLocationTest {
@Test
void testEqualsHashAndCompareTo() {
- try {
- ResourceID resourceID1 = new ResourceID("a");
- ResourceID resourceID2 = new ResourceID("b");
- ResourceID resourceID3 = new ResourceID("c");
-
- // we mock the addresses to save the times of the reverse name
lookups
- InetAddress address1 = mock(InetAddress.class);
- when(address1.getCanonicalHostName()).thenReturn("localhost");
- when(address1.getHostName()).thenReturn("localhost");
- when(address1.getHostAddress()).thenReturn("127.0.0.1");
- when(address1.getAddress()).thenReturn(new byte[] {127, 0, 0, 1});
-
- InetAddress address2 = mock(InetAddress.class);
- when(address2.getCanonicalHostName()).thenReturn("testhost1");
- when(address2.getHostName()).thenReturn("testhost1");
- when(address2.getHostAddress()).thenReturn("0.0.0.0");
- when(address2.getAddress()).thenReturn(new byte[] {0, 0, 0, 0});
-
- InetAddress address3 = mock(InetAddress.class);
- when(address3.getCanonicalHostName()).thenReturn("testhost2");
- when(address3.getHostName()).thenReturn("testhost2");
- when(address3.getHostAddress()).thenReturn("192.168.0.1");
- when(address3.getAddress()).thenReturn(new byte[] {(byte) 192,
(byte) 168, 0, 1});
-
- // one == four != two != three
- TaskManagerLocation one = new TaskManagerLocation(resourceID1,
address1, 19871);
- TaskManagerLocation two = new TaskManagerLocation(resourceID2,
address2, 19871);
- TaskManagerLocation three = new TaskManagerLocation(resourceID3,
address3, 10871);
- TaskManagerLocation four = new TaskManagerLocation(resourceID1,
address1, 19871);
-
- assertThat(one).isEqualTo(four);
- assertThat(one).isNotEqualTo(two);
- assertThat(one).isNotEqualTo(three);
- assertThat(two).isNotEqualTo(three);
- assertThat(three).isNotEqualTo(four);
-
- assertThat(one.compareTo(four)).isEqualTo(0);
- assertThat(four.compareTo(one)).isEqualTo(0);
- assertThat(one.compareTo(two)).isNotEqualTo(0);
- assertThat(one.compareTo(three)).isNotEqualTo(0);
- assertThat(two.compareTo(three)).isNotEqualTo(0);
- assertThat(three.compareTo(four)).isNotEqualTo(0);
-
- {
- int val = one.compareTo(two);
- assertThat(two.compareTo(one)).isEqualTo(-val);
- }
- } catch (Exception e) {
- e.printStackTrace();
- fail(e.getMessage());
- }
+ assertThatCode(
+ () -> {
+ ResourceID resourceID1 = new ResourceID("a");
+ ResourceID resourceID2 = new ResourceID("b");
+ ResourceID resourceID3 = new ResourceID("c");
+
+ // we mock the addresses to save the times of the
reverse name lookups
+ InetAddress address1 = mock(InetAddress.class);
+
when(address1.getCanonicalHostName()).thenReturn("localhost");
+
when(address1.getHostName()).thenReturn("localhost");
+
when(address1.getHostAddress()).thenReturn("127.0.0.1");
+ when(address1.getAddress()).thenReturn(new byte[]
{127, 0, 0, 1});
+
+ InetAddress address2 = mock(InetAddress.class);
+
when(address2.getCanonicalHostName()).thenReturn("testhost1");
+
when(address2.getHostName()).thenReturn("testhost1");
+
when(address2.getHostAddress()).thenReturn("0.0.0.0");
+ when(address2.getAddress()).thenReturn(new byte[]
{0, 0, 0, 0});
+
+ InetAddress address3 = mock(InetAddress.class);
+
when(address3.getCanonicalHostName()).thenReturn("testhost2");
+
when(address3.getHostName()).thenReturn("testhost2");
+
when(address3.getHostAddress()).thenReturn("192.168.0.1");
+ when(address3.getAddress())
+ .thenReturn(new byte[] {(byte) 192, (byte)
168, 0, 1});
+
+ // one == four != two != three
+ TaskManagerLocation one =
+ new TaskManagerLocation(resourceID1,
address1, 19871);
+ TaskManagerLocation two =
+ new TaskManagerLocation(resourceID2,
address2, 19871);
+ TaskManagerLocation three =
+ new TaskManagerLocation(resourceID3,
address3, 10871);
+ TaskManagerLocation four =
+ new TaskManagerLocation(resourceID1,
address1, 19871);
+
+ assertThat(one).isEqualTo(four);
+ assertThat(one).isNotEqualTo(two);
+ assertThat(one).isNotEqualTo(three);
+ assertThat(two).isNotEqualTo(three);
+ assertThat(three).isNotEqualTo(four);
+
+ assertThat(one.compareTo(four)).isZero();
+ assertThat(four.compareTo(one)).isZero();
+ assertThat(one.compareTo(two)).isNotZero();
+ assertThat(one.compareTo(three)).isNotZero();
+ assertThat(two.compareTo(three)).isNotZero();
+ assertThat(three.compareTo(four)).isNotZero();
+
+ {
+ int val = one.compareTo(two);
+ assertThat(two.compareTo(one)).isEqualTo(-val);
+ }
+ })
+ .doesNotThrowAnyException();
Review Comment:
If one test throw a exception, the test will fail directly, so I don't think
the `doesNotThrowAnyException` is necessary.
The original catch wants to print some information after the test throw any
exception. The `doesNotThrowAnyException` just fail this test, it's same to
removing it.
- If these information is useful, we should keep `e.printStackTrace();` and
`fail(e.getMessage());`
- If it isn't useful, we can remove it directly.
Please correct me if I'm wrong, thanks
##########
flink-runtime/src/test/java/org/apache/flink/runtime/taskmanager/TaskTest.java:
##########
@@ -1117,18 +1104,18 @@ public void
testTerminationFutureCompletesOnImmediateCancellation() throws Excep
task.cancelExecution();
- assertFalse(task.getTerminationFuture().isDone());
+ assertThat(task.getTerminationFuture().isDone()).isFalse();
Review Comment:
```suggestion
assertThat(task.getTerminationFuture()).isNotDone();
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/taskmanager/TaskExecutionStateTest.java:
##########
@@ -70,23 +70,23 @@ public void testSerialization() {
TaskExecutionState javaSerCopy2 =
CommonTestUtils.createCopySerializable(original2);
// equalities
- assertEquals(original1, javaSerCopy1);
- assertEquals(javaSerCopy1, original1);
+ assertThat(javaSerCopy1).isEqualTo(original1);
+ assertThat(original1).isEqualTo(javaSerCopy1);
- assertEquals(original2, javaSerCopy2);
- assertEquals(javaSerCopy2, original2);
+ assertThat(javaSerCopy2).isEqualTo(original2);
+ assertThat(original2).isEqualTo(javaSerCopy2);
// hash codes
- assertEquals(original1.hashCode(), javaSerCopy1.hashCode());
- assertEquals(original2.hashCode(), javaSerCopy2.hashCode());
+
assertThat(javaSerCopy1.hashCode()).isEqualTo(original1.hashCode());
+
assertThat(javaSerCopy2.hashCode()).isEqualTo(original2.hashCode());
Review Comment:
hasSameHashCodeAs
##########
flink-runtime/src/test/java/org/apache/flink/runtime/taskmanager/TaskTest.java:
##########
@@ -375,19 +363,19 @@ public void testExecutionFailsInRestore() throws
Exception {
task.run();
- assertEquals(ExecutionState.FAILED, task.getExecutionState());
- assertTrue(task.isCanceledOrFailed());
- assertNotNull(task.getFailureCause());
- assertNotNull(task.getFailureCause().getMessage());
- assertThat(task.getFailureCause().getMessage(),
containsString(RESTORE_EXCEPTION_MSG));
+ assertThat(task.getExecutionState()).isEqualTo(ExecutionState.FAILED);
+ assertThat(task.isCanceledOrFailed()).isTrue();
+ assertThat(task.getFailureCause()).isNotNull();
+ assertThat(task.getFailureCause().getMessage()).isNotNull();
+
assertThat(task.getFailureCause().getMessage()).contains(RESTORE_EXCEPTION_MSG);
Review Comment:
```suggestion
assertThat(task.getFailureCause().getMessage()).isNotNull().contains(RESTORE_EXCEPTION_MSG);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/taskmanager/TaskTest.java:
##########
@@ -1098,17 +1085,17 @@ public void
testTerminationFutureCompletesOnNormalExecution() throws Exception {
// wait till the task is in invoke
awaitInvokableLatch(task);
- assertFalse(task.getTerminationFuture().isDone());
+ assertThat(task.getTerminationFuture().isDone()).isFalse();
Review Comment:
```suggestion
assertThat(task.getTerminationFuture()).isNotDone();
```
--
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]
