Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unit tests for com.baidu.hugegraph.util.StringUtil #485

Merged
merged 1 commit into from
May 15, 2019

Conversation

EricHetti
Copy link
Contributor

@EricHetti EricHetti commented Apr 26, 2019

I've analysed your codebase and noticed that com.baidu.hugegraph.util.StringUtil is not fully tested.
I've written some tests for the methods in this class with the help of Diffblue Cover.

Hopefully, these tests will help you detect any regressions caused by future code changes. If you would find it useful to have additional tests written for this repository, I would be more than happy to look at other classes that you consider important.

@codecov
Copy link

codecov bot commented Apr 26, 2019

Codecov Report

Merging #485 into master will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #485      +/-   ##
============================================
+ Coverage     68.99%   69.02%   +0.03%     
- Complexity     3280     3283       +3     
============================================
  Files           208      208              
  Lines         16193    16193              
  Branches       2334     2334              
============================================
+ Hits          11172    11178       +6     
+ Misses         3774     3765       -9     
- Partials       1247     1250       +3
Impacted Files Coverage Δ Complexity Δ
...a/com/baidu/hugegraph/backend/query/Condition.java 62.43% <0%> (-0.53%) 20% <0%> (ø)
...main/java/com/baidu/hugegraph/util/StringUtil.java 66.66% <0%> (+23.33%) 9% <0%> (+3%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bed8a6e...84f0ecd. Read the comment docs.

@javeme
Copy link
Contributor

javeme commented Apr 26, 2019

@Braavos96 Thank you very much for your code contribution. Yes it's very useful to improve the tests,
and look forward to your further contributions.

Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

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

Looks great! there are still some comments and please don't forget to sign CLA

@EricHetti EricHetti closed this May 1, 2019
@EricHetti EricHetti force-pushed the add-diffblue-tests branch from ab49895 to cf80faf Compare May 1, 2019 09:29
@EricHetti EricHetti reopened this May 1, 2019
@EricHetti
Copy link
Contributor Author

Hi @javeme, I have amended the test cases to include your comments on it and also signed the CLA.
Apologies I accidentally closed it while pushing the changes :)

@javeme
Copy link
Contributor

javeme commented May 6, 2019

@Braavos96 Thank you very much, there are some minor comments to be addressed.

@EricHetti EricHetti force-pushed the add-diffblue-tests branch 2 times, most recently from 0368547 to b6217e1 Compare May 13, 2019 09:43
Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

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

Still some comments, thanks very much

@EricHetti EricHetti force-pushed the add-diffblue-tests branch from b6217e1 to a1232ff Compare May 14, 2019 07:53
These tests were written using Diffblue Cover.
@EricHetti EricHetti force-pushed the add-diffblue-tests branch from a1232ff to 84f0ecd Compare May 14, 2019 15:37
@javeme javeme dismissed zhoney’s stale review May 15, 2019 06:08

exemption for first-time contributor

@javeme javeme merged commit 6fbcb49 into apache:master May 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants