[Improvement][UT] Improve Worker registry coverage #15380

pegasas · 2023-12-29T03:06:00Z

Purpose of the pull request

part of #10573
try to resolve #15396
Improve worker registry coverage above 85%

Brief change log

Improve worker registry coverage above 85%

Verify this pull request

EricGao888

Basically LGTM

...va/org/apache/dolphinscheduler/server/worker/registry/WorkerConnectionStateListenerTest.java

...ker/src/test/java/org/apache/dolphinscheduler/server/worker/registry/WorkerStrategyTest.java

ruanwenjun

Good job, wait fix the spotless.

ruanwenjun · 2023-12-29T10:47:19Z

...c/test/java/org/apache/dolphinscheduler/server/worker/registry/WorkerRegistryClientTest.java


-import org.junit.jupiter.api.Assertions;
+import org.junit.Assert;


Please use Assertions.

fuchanghai · 2024-01-03T07:12:31Z

hi @pegasas should we create a new issue associated with this PR @EricGao888 and add this issue to #10573?

pegasas · 2024-01-03T07:42:10Z

hi @pegasas should we create a new issue associated with this PR @EricGao888 and add this issue to #10573?

This may be more reliable.
I've create an issue and attach it into these threads.

EricGao888 · 2024-01-11T02:45:48Z

As CI output said, you could run 'mvn spotless:apply' to fix these style violations.

pegasas · 2024-01-11T03:20:16Z

As CI output said, you could run 'mvn spotless:apply' to fix these style violations.

Thanks @EricGao888!
I've correct the style violation.

EricGao888

LGTM if CI passes

codecov-commenter · 2024-01-11T03:50:51Z

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (43a0652) 38.36% compared to head (780951c) 38.53%.

❗ Current head 780951c differs from pull request most recent head 8ad497c. Consider uploading reports for the commit 8ad497c to get more accurate results

Files	Patch %	Lines
.../server/worker/registry/WorkerWaitingStrategy.java	55.55%	0 Missing and 4 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##                dev   #15380      +/-   ##
============================================
+ Coverage     38.36%   38.53%   +0.16%     
- Complexity     4744     4770      +26     
============================================
  Files          1305     1305              
  Lines         44830    44838       +8     
  Branches       4803     4805       +2     
============================================
+ Hits          17200    17278      +78     
+ Misses        25759    25684      -75     
- Partials       1871     1876       +5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pegasas · 2024-01-11T04:36:02Z

Hi, @ruanwenjun , @SbloodyS , It seems other gated passed, would you like to take a look when anytime available?

fuchanghai · 2024-01-11T09:31:46Z

.../src/main/java/org/apache/dolphinscheduler/server/worker/registry/WorkerWaitingStrategy.java

@@ -52,6 +53,16 @@ public class WorkerWaitingStrategy implements WorkerConnectStrategy {
    @Autowired
    private WorkerTaskExecutorThreadPool workerManagerThread;

+    public WorkerWaitingStrategy(@NonNull WorkerConfig workerConfig,


hi @pegasas Through the @service annotation, I judge that this class is a singleton and injected into the spring container, so is this constructor necessary? Where is it used? Is there anything I haven't noticed?

rickchengx · 2024-01-23T02:38:50Z

Hi, @pegasas please resolve the conflicts

pegasas · 2024-01-23T06:10:31Z

Hi, @pegasas please resolve the conflicts

Thanks @rickchengx , I've update my PR with merging conflicts.

passing in my local environment.

rickchengx · 2024-01-24T07:39:52Z

Hi, @pegasas there is still an unresolved comment from @fuchanghai, please check

pegasas · 2024-01-29T05:07:06Z

Hi, @pegasas there is still an unresolved comment from @fuchanghai, please check

thanks @rickchengx
Hi, @fuchanghai, do we have any other solutions for the questions?
Shall we keep it that way?

rickchengx

LGTM

rickchengx · 2024-02-05T02:26:47Z

will merge tomorrow if no more comments

EricGao888 · 2024-02-05T07:38:28Z

CI failures seem not related to the changes. I've restarted it and let's see whether it could pass this time.

fuchanghai

LGTM

sonarcloud · 2024-02-06T02:42:08Z

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

10 New issues
0 Security Hotspots
76.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Co-authored-by: fuchanghai <changhaifu@apache.org> Co-authored-by: Eric Gao <ericgao.apache@gmail.com> Co-authored-by: Rick Cheng <rickchengx@gmail.com> (cherry picked from commit 69676b4)

pegasas requested review from caishunfeng, SbloodyS and ruanwenjun as code owners December 29, 2023 03:06

github-actions bot added the backend label Dec 29, 2023

pegasas force-pushed the worker-registry-ut branch 2 times, most recently from 2f1ae90 to 0ed1e8a Compare December 29, 2023 03:19

EricGao888 assigned pegasas Dec 29, 2023

EricGao888 added the improvement make more easy to user or prompt friendly label Dec 29, 2023

EricGao888 modified the milestones: 3.2.1, 4.0.0-alpha Dec 29, 2023

EricGao888 reviewed Dec 29, 2023

View reviewed changes

...va/org/apache/dolphinscheduler/server/worker/registry/WorkerConnectionStateListenerTest.java Show resolved Hide resolved

pegasas force-pushed the worker-registry-ut branch from 7ca4e2d to b56f25a Compare December 29, 2023 05:59

EricGao888 previously approved these changes Dec 29, 2023

View reviewed changes

EricGao888 added the ready-to-merge label Dec 29, 2023

EricGao888 reviewed Dec 29, 2023

View reviewed changes

...ker/src/test/java/org/apache/dolphinscheduler/server/worker/registry/WorkerStrategyTest.java Show resolved Hide resolved

pegasas dismissed EricGao888’s stale review via 55b2d16 December 29, 2023 07:08

pegasas force-pushed the worker-registry-ut branch from d1ffb08 to 55b2d16 Compare December 29, 2023 07:08

EricGao888 previously approved these changes Dec 29, 2023

View reviewed changes

ruanwenjun reviewed Dec 29, 2023

View reviewed changes

pegasas dismissed EricGao888’s stale review via a3e64df December 29, 2023 11:24

pegasas force-pushed the worker-registry-ut branch from 8617e0a to a3e64df Compare December 29, 2023 11:24

pegasas force-pushed the worker-registry-ut branch from a3e64df to 73ae07d Compare January 3, 2024 07:52

pegasas requested review from ruanwenjun and EricGao888 January 9, 2024 03:20

[Improvement][UT] Improve Worker registry coverage

c000aaf

pegasas force-pushed the worker-registry-ut branch from 51bc8dd to c000aaf Compare January 11, 2024 03:12

EricGao888 previously approved these changes Jan 11, 2024

View reviewed changes

fuchanghai reviewed Jan 11, 2024

View reviewed changes

fuchanghai and others added 3 commits January 12, 2024 17:05

Merge branch 'dev' into worker-registry-ut

d2e27c3

Merge branch 'dev' into worker-registry-ut

694e843

Merge branch 'dev' into worker-registry-ut

b426acb

Merge branch 'dev' into worker-registry-ut

11930a1

pegasas dismissed EricGao888’s stale review via 11930a1 January 23, 2024 06:09

rickchengx added 2 commits January 24, 2024 10:41

Merge branch 'dev' into worker-registry-ut

581d5ad

Merge branch 'dev' into worker-registry-ut

cb00b32

rickchengx approved these changes Feb 5, 2024

View reviewed changes

rickchengx added 2 commits February 5, 2024 10:05

Merge branch 'dev' into worker-registry-ut

b97bc30

Merge branch 'dev' into worker-registry-ut

6c839b2

EricGao888 added 2 commits February 5, 2024 11:50

Merge branch 'dev' into worker-registry-ut

df01a12

Merge branch 'dev' into worker-registry-ut

571393c

fuchanghai approved these changes Feb 6, 2024

View reviewed changes

fuchanghai and others added 2 commits February 6, 2024 10:00

Merge branch 'dev' into worker-registry-ut

aa8b021

Merge branch 'dev' into worker-registry-ut

8ad497c

rickchengx merged commit 69676b4 into apache:dev Feb 6, 2024
56 checks passed

pegasas deleted the worker-registry-ut branch February 6, 2024 02:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Improvement][UT] Improve Worker registry coverage #15380

[Improvement][UT] Improve Worker registry coverage #15380

pegasas commented Dec 29, 2023 •

edited

Loading

EricGao888 left a comment

ruanwenjun left a comment

ruanwenjun Dec 29, 2023

fuchanghai commented Jan 3, 2024 •

edited

Loading

pegasas commented Jan 3, 2024 •

edited

Loading

EricGao888 commented Jan 11, 2024

pegasas commented Jan 11, 2024

EricGao888 left a comment

codecov-commenter commented Jan 11, 2024 •

edited

Loading

pegasas commented Jan 11, 2024

fuchanghai Jan 11, 2024

rickchengx commented Jan 23, 2024

pegasas commented Jan 23, 2024 •

edited

Loading

rickchengx commented Jan 24, 2024

pegasas commented Jan 29, 2024

rickchengx left a comment

rickchengx commented Feb 5, 2024

EricGao888 commented Feb 5, 2024

fuchanghai left a comment

sonarcloud bot commented Feb 6, 2024


		import org.junit.jupiter.api.Assertions;
		import org.junit.Assert;

[Improvement][UT] Improve Worker registry coverage #15380

[Improvement][UT] Improve Worker registry coverage #15380

Conversation

pegasas commented Dec 29, 2023 • edited Loading

Purpose of the pull request

Brief change log

Verify this pull request

EricGao888 left a comment

Choose a reason for hiding this comment

ruanwenjun left a comment

Choose a reason for hiding this comment

ruanwenjun Dec 29, 2023

Choose a reason for hiding this comment

fuchanghai commented Jan 3, 2024 • edited Loading

pegasas commented Jan 3, 2024 • edited Loading

EricGao888 commented Jan 11, 2024

pegasas commented Jan 11, 2024

EricGao888 left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jan 11, 2024 • edited Loading

Codecov Report

pegasas commented Jan 11, 2024

fuchanghai Jan 11, 2024

Choose a reason for hiding this comment

rickchengx commented Jan 23, 2024

pegasas commented Jan 23, 2024 • edited Loading

rickchengx commented Jan 24, 2024

pegasas commented Jan 29, 2024

rickchengx left a comment

Choose a reason for hiding this comment

rickchengx commented Feb 5, 2024

EricGao888 commented Feb 5, 2024

fuchanghai left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Feb 6, 2024

Quality Gate passed

pegasas commented Dec 29, 2023 •

edited

Loading

fuchanghai commented Jan 3, 2024 •

edited

Loading

pegasas commented Jan 3, 2024 •

edited

Loading

codecov-commenter commented Jan 11, 2024 •

edited

Loading

pegasas commented Jan 23, 2024 •

edited

Loading