JunRuiLee commented on code in PR #21634:
URL: https://github.com/apache/flink/pull/21634#discussion_r1115454833
##########
flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/benchmark/deploying/DeployingTasksInStreamingJobBenchmarkTest.java:
##########
@@ -22,7 +22,7 @@
import org.apache.flink.runtime.scheduler.benchmark.JobConfiguration;
import org.apache.flink.util.TestLogger;
Review Comment:
Can be removed. All the tests you want to migrate to Junit5 should remove it.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptivebatch/AdaptiveBatchSchedulerTest.java:
##########
@@ -400,4 +401,10 @@ private SchedulerBase createScheduler(
.setDefaultMaxParallelism(defaultMaxParallelism)
.buildAdaptiveBatchJobScheduler();
}
+
+ public static void initializeVerticesIfPossible(DefaultScheduler
scheduler) {
+ if (scheduler instanceof AdaptiveBatchScheduler) {
+ ((AdaptiveBatchScheduler)
scheduler).initializeVerticesIfPossible();
+ }
+ }
Review Comment:
This utility method should not be placed inside this class. And currently
only the SchedulerBenchmarkUtils class is used, you can move this method to
SchedulerBenchmarkUtils or create a new AdaptiveBatchSchedulerTestUtils.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/benchmark/deploying/DeployingDownstreamTasksInBatchJobBenchmarkTest.java:
##########
@@ -31,7 +31,7 @@
public class DeployingDownstreamTasksInBatchJobBenchmarkTest extends
TestLogger {
Review Comment:
If you need to migrate to Junit5, maybe you can to submit a new commit with
a title like [hotfix][runtime] Migrate some related tests to Juint5 and AssertJ.
And the TestLogger should be removed.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptivebatch/AdaptiveBatchSchedulerTest.java:
##########
@@ -73,7 +74,7 @@
import static org.assertj.core.api.Assertions.assertThat;
/** Test for {@link AdaptiveBatchScheduler}. */
-class AdaptiveBatchSchedulerTest {
+public class AdaptiveBatchSchedulerTest {
Review Comment:
This change is no longer needed after removing initializeVerticesIfPossible
--
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]
