1996fanrui commented on code in PR #24732:
URL: https://github.com/apache/flink/pull/24732#discussion_r1582594242
##########
flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/BackgroundTaskTest.java:
##########
@@ -18,55 +18,52 @@
package org.apache.flink.runtime.scheduler.adaptive;
-import org.apache.flink.core.testutils.FlinkMatchers;
import org.apache.flink.core.testutils.OneShotLatch;
-import org.apache.flink.testutils.executor.TestExecutorResource;
-import org.apache.flink.util.TestLogger;
+import org.apache.flink.testutils.executor.TestExecutorExtension;
-import org.hamcrest.Matchers;
-import org.junit.ClassRule;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.apache.flink.core.testutils.FlinkAssertions.assertThatFuture;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
/** Tests for the {@link BackgroundTask}. */
-public class BackgroundTaskTest extends TestLogger {
+class BackgroundTaskTest {
- @ClassRule
- public static final TestExecutorResource<ExecutorService>
TEST_EXECUTOR_RESOURCE =
- new TestExecutorResource<>(() -> Executors.newFixedThreadPool(2));
+ @RegisterExtension
+ public static final TestExecutorExtension<ExecutorService>
TEST_EXECUTOR_EXTENSION =
+ new TestExecutorExtension<>(() -> Executors.newFixedThreadPool(2));
@Test
public void testFinishedBackgroundTaskIsTerminated() {
final BackgroundTask<Void> finishedBackgroundTask =
BackgroundTask.finishedBackgroundTask();
- assertTrue(finishedBackgroundTask.getTerminationFuture().isDone());
+
assertThatFuture(finishedBackgroundTask.getTerminationFuture()).isDone();
finishedBackgroundTask.getTerminationFuture().join();
}
@Test
public void testFinishedBackgroundTaskDoesNotContainAResult() {
final BackgroundTask<Void> finishedBackgroundTask =
BackgroundTask.finishedBackgroundTask();
-
assertTrue(finishedBackgroundTask.getResultFuture().isCompletedExceptionally());
+
assertThatFuture(finishedBackgroundTask.getResultFuture()).isCompletedExceptionally();
}
@Test
public void testNormalCompletionOfBackgroundTask() {
Review Comment:
All public can be removed.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SsgNetworkMemoryCalculationUtilsTest.java:
##########
@@ -182,21 +182,20 @@ private void
triggerComputeNumOfSubpartitions(IntermediateResult result) {
private void assertNetworkMemory(
List<SlotSharingGroup> slotSharingGroups, List<MemorySize>
networkMemory) {
- assertEquals(slotSharingGroups.size(), networkMemory.size());
+ assertThat(networkMemory.size()).isEqualTo(slotSharingGroups.size());
Review Comment:
hasSameSizeAs
##########
flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SsgNetworkMemoryCalculationUtilsTest.java:
##########
@@ -270,16 +269,16 @@ private void createExecutionGraphAndEnrichNetworkMemory(
createJobGraph(
slotSharingGroups, Arrays.asList(4, 5, 6),
resultPartitionType))
.setShuffleMaster(SHUFFLE_MASTER)
- .build(EXECUTOR_RESOURCE.getExecutor());
+ .build(EXECUTOR_EXTENSION.getExecutor());
}
private static JobGraph createJobGraph(
final List<SlotSharingGroup> slotSharingGroups,
List<Integer> parallelisms,
ResultPartitionType resultPartitionType) {
- assertThat(slotSharingGroups.size(), is(3));
- assertThat(parallelisms.size(), is(3));
+ assertThat(slotSharingGroups.size()).isEqualTo(3);
+ assertThat(parallelisms.size()).isEqualTo(3);
Review Comment:
hasSize
##########
flink-runtime/src/test/java/org/apache/flink/runtime/testutils/InternalMiniClusterExtension.java:
##########
@@ -109,4 +117,24 @@ public Object resolveParameter(
}
throw new ParameterResolutionException("Unsupported parameter");
}
+
+ @Override
+ public void beforeEach(ExtensionContext context) throws Exception {
+ miniClusterResource.before();
+ }
+
+ @Override
+ public void afterEach(ExtensionContext context) throws Exception {
+ miniClusterResource.after();
+ }
+
+ @Override
+ public void before(ExtensionContext context) throws Exception {
+ miniClusterResource.before();
+ }
+
+ @Override
+ public void after(ExtensionContext context) throws Exception {
+ miniClusterResource.after();
+ }
Review Comment:
I saw the mini cluster resource didn't close for each test before. I guess
one reason is performance.
Start a mini cluster needs some times, if we restart it frequently, test
will take a long time.
Is it necessary for this PR?
<img width="1043" alt="image"
src="https://github.com/apache/flink/assets/38427477/4343a850-9472-48ea-b331-5e6fd94ae9de";>
##########
flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/StateWithExecutionGraphTest.java:
##########
@@ -120,9 +123,8 @@ public void testOnGloballyTerminalStateCalled() throws
Exception {
context.close();
- assertThat(
- stateWithExecutionGraph.getGloballyTerminalStateFuture().get(),
- is(JobStatus.FINISHED));
+
assertThatFuture(stateWithExecutionGraph.getGloballyTerminalStateFuture())
+ .isCompletedWithValue(JobStatus.FINISHED);
}
@Test
Review Comment:
All public can be removed.
--
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]
