[Improvement-12333][Test] Migrate all UT cases from jUnit 4 to jUnit 5 in microbench and e2e module

[rickchengx]

Purpose of the pull request

• close: [Migration][Test] Migrate all UT cases from jUnit 4 to jUnit 5 in microbench and e2e module #12333

Brief change log

Migrate all UT cases from jUnit 4 to jUnit 5 in microbench and e2e module

Verify this pull request

[codecov-commenter]

Codecov Report

Merging #12348 (3bef85f) into dev (ac84504) will increase coverage by 0.28%.
The diff coverage is 29.16%.

❗ Current head 3bef85f differs from pull request most recent head 4836073. Consider uploading reports for the commit 4836073 to get more accurate results

@@             Coverage Diff              @@
##                dev   #12348      +/-   ##
============================================
+ Coverage     38.93%   39.22%   +0.28%     
- Complexity     4177     4211      +34     
============================================
  Files          1040     1040              
  Lines         38797    38794       -3     
  Branches       4460     4460              
============================================
+ Hits          15105    15216     +111     
+ Misses        21923    21806     -117     
- Partials       1769     1772       +3     

Impacted Files	Coverage Δ
.../org/apache/dolphinscheduler/api/enums/Status.java	100.00% <ø> (ø)
...api/service/impl/ProcessDefinitionServiceImpl.java	34.83% <0.00%> (+0.11%)	⬆️
...inscheduler/service/expand/CuringGlobalParams.java	31.11% <0.00%> (ø)
...olphinscheduler/plugin/task/api/TaskConstants.java	0.00% <ø> (ø)
...lphinscheduler/plugin/task/api/utils/K8sUtils.java	3.50% <0.00%> (-0.34%)	⬇️
...dolphinscheduler/plugin/task/datax/DataxUtils.java	0.00% <0.00%> (ø)
.../dolphinscheduler/plugin/task/datax/DataxTask.java	36.77% <63.63%> (+36.77%)	⬆️
...org/apache/dolphinscheduler/remote/utils/Host.java	42.55% <0.00%> (-2.13%)	⬇️
...e/dolphinscheduler/remote/NettyRemotingClient.java	51.42% <0.00%> (-0.72%)	⬇️
...r/plugin/task/datax/DataxTaskExecutionContext.java	5.00% <0.00%> (+5.00%)	⬆️
... and 2 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[rickchengx]

Hi, @kezhenxu94 , could you please help me with this e2e problem?

This PR wants to remove the junit4 api in the e2e module. But in dolphinscheduler-e2e/dolphinscheduler-e2e-core/src/main/java/org/apache/dolphinscheduler/e2e/core/DolphinSchedulerExtension.java, we cannot just remove or replace the junit4 api since org.testcontainers.containers.Network is used.

It seems that Testcontainers is tightly coupled with the JUnit 4.x rule API, and here is the limitation of Testcontainers from the article below:

• https://www.testcontainers.org/test_framework_integration/junit_5/

If we want to migrate all UT cases from jUnit4 to Junit5 here, do we need to refactor the tests in e2e module.
Note: Testcontainers is also used in dolphinscheduler-api-test module.

cc @EricGao888

[kezhenxu94]

From the doc of testcontainers it seems impossible to remove junit4 from test classpath. But since we have spotless check to forbid usage of junit4 classes we can avoid direct use of junit4 in our codebase. Right?

Is your question about the failed tests, then?

[rickchengx]

@kezhenxu94 Thanks a lot for the comment, I agree with you. So we need to avoid the junit 4 api in our codebase.

As for the CI e2e failure, this is because I usenetwork = Network.newNetWork() instead of network = new NetWork() and override the methods of getId(), close() and apply().

The reason I made this change was to avoid the import of org.junit.runner.Description and org.junit.runners.model.Statement. But network = Network.newNetWork() now cannot return the networkId and cause the CI failure.

Now I have tried to use network = Network.builder().id(networkId).build();. I am not sure whether this could pass the CI.

Maybe we could wait for the CI results before deciding what to do next.

[rickchengx]

Hi @kezhenxu94 It seems that the CI cannot pass because of the network below, would you please help me with this CI failure?

[rickchengx]

I have a clue, WIP

[EricGao888]

@rickchengx FYI, we also have a junit import in microbench module, see:

dolphinscheduler/dolphinscheduler-microbench/src/main/java/org/apache/dolphinscheduler/microbench/base/AbstractBaseBenchmark.java

Line 23 in 3bef85f

import org.junit.Test;

[rickchengx]

@rickchengx FYI, we also have a junit import in microbench module, see:

dolphinscheduler/dolphinscheduler-microbench/src/main/java/org/apache/dolphinscheduler/microbench/base/AbstractBaseBenchmark.java

Line 23 in 3bef85f

import org.junit.Test;

Hi @EricGao888 Thanks for the comment. I've noticed this.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[rickchengx]

Hi, @EricGao888 , I have refactored DolphinschedulerExtension in e2e module to remove the junit4 api and It seems that the CI is passed, : )

cc @kezhenxu94

[EricGao888]

Seems good to me. But I'm not familiar with this module. Could u plz take a look when available? @kezhenxu94 @SbloodyS ? Thanks~

[zhongjiajie]

LGTM, BTW can you also fix the number format error?

[EricGao888]

LGTM, BTW can you also fix the number format error?

@zhongjiajie Thanks for pointing this out. I will submit a follow-up PR to this issue to fix this, as well as remove junit4 check exclusions for these two modules.

dolphinscheduler/pom.xml

Lines 642 to 647 in a11892a

	<java>
	<!--TODO: Remove the following excludes section when junit4 removed from e2e and microbench module-->
	<excludes>
	<exclude>**/e2e/**/*.java</exclude>
	<exclude>**/microbench/**/*.java</exclude>
	</excludes>

[EricGao888]

LGTM, BTW can you also fix the number format error?

@zhongjiajie I've done it in #12450, please take a look when available, thanks : )