[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 java.util.Optional;

-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

    @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

sonarqubecloud · 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 <[email protected]> Co-authored-by: Eric Gao <[email protected]> Co-authored-by: Rick Cheng <[email protected]> (cherry picked from commit 69676b4)

pegasas requested review from SbloodyS, caishunfeng 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 EricGao888 and ruanwenjun 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

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

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

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

Uh oh!

Conversation

pegasas commented Dec 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of the pull request

Brief change log

Verify this pull request

Uh oh!

EricGao888 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ruanwenjun left a comment

Choose a reason for hiding this comment

Uh oh!

ruanwenjun Dec 29, 2023

Choose a reason for hiding this comment

Uh oh!

fuchanghai commented Jan 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pegasas commented Jan 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EricGao888 commented Jan 11, 2024

Uh oh!

pegasas commented Jan 11, 2024

Uh oh!

EricGao888 left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jan 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pegasas commented Jan 11, 2024

Uh oh!

fuchanghai Jan 11, 2024

Choose a reason for hiding this comment

Uh oh!

rickchengx commented Jan 23, 2024

Uh oh!

pegasas commented Jan 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rickchengx commented Jan 24, 2024

Uh oh!

pegasas commented Jan 29, 2024

Uh oh!

rickchengx left a comment

Choose a reason for hiding this comment

Uh oh!

rickchengx commented Feb 5, 2024

Uh oh!

EricGao888 commented Feb 5, 2024

Uh oh!

fuchanghai left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Feb 6, 2024

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

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