Skip to content

[Improvement]add UT for utils#13715

Closed
XiaochenNan wants to merge 7 commits intoapache:devfrom
XiaochenNan:ut_utils
Closed

[Improvement]add UT for utils#13715
XiaochenNan wants to merge 7 commits intoapache:devfrom
XiaochenNan:ut_utils

Conversation

@XiaochenNan
Copy link
Contributor

Purpose of the pull request

This pull request adds UT for some utils

Brief change log

ProcessUtilsTest.java
ArgsUtilsTest.java

Verify this pull request

This change added tests and can be verified as follows:
Added UT for ArgsUtils and ProcessUtils

@codecov-commenter
Copy link

codecov-commenter commented Mar 9, 2023

Codecov Report

Merging #13715 (3a54118) into dev (aef5524) will increase coverage by 0.03%.
The diff coverage is n/a.

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

@@             Coverage Diff              @@
##                dev   #13715      +/-   ##
============================================
+ Coverage     39.10%   39.13%   +0.03%     
- Complexity     4420     4430      +10     
============================================
  Files          1130     1130              
  Lines         42052    42052              
  Branches       4774     4774              
============================================
+ Hits          16445    16459      +14     
+ Misses        23792    23778      -14     
  Partials       1815     1815              

see 6 files with indirect coverage changes

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

@XiaochenNan XiaochenNan changed the title add UT for utils [Improvement]add UT for utils Mar 9, 2023
@XiaochenNan
Copy link
Contributor Author

issue is #13717

Comment on lines 35 to 36
TaskExecutionContext taskExecutionContext = new TaskExecutionContext();
Assertions.assertFalse(ProcessUtils.kill(taskExecutionContext));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This UT is meaningless.

Copy link
Contributor Author

@XiaochenNan XiaochenNan Mar 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ruanwenjun updated setAppIds step, please check again, thanks.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 5 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@EricGao888 EricGao888 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our purpose of adding UT cases is to:

  1. Cover code lines / logic branches as much as possible in order to reduce bugs.
  2. Work as documentations for the covered code so that developers know what the covered code does.

Therefore, you need to cover as many methods and lines as you can in the UT cases.

Copy link
Member

@EricGao888 EricGao888 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Aug 18, 2023
@github-actions
Copy link

This pull request has been closed because it has not had recent activity. You could reopen it if you try to continue your work, and anyone who are interested in it are encouraged to continue work on this pull request.

@github-actions github-actions bot closed this Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants