Add unit test cases for download_api_test.go&algorithm_test.go&atomiccount_test.go&httpuils_test.go #1272
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1272 +/- ##
==========================================
+ Coverage 48.75% 48.97% +0.21%
==========================================
Files 119 119
Lines 7763 7763
==========================================
+ Hits 3785 3802 +17
+ Misses 3663 3649 -14
+ Partials 315 312 -3
Continue to review full report at Codecov.
|
zouy414
changed the title
|
Thanks for your contribution. 🍻 @Hellcatlk |
zouy414
changed the title
zouy414
changed the title
dfget/util/algorithm_test.go
Outdated
| @@ -52,4 +52,10 @@ func (s *AlgorithmTestSuite) TestShuffle(c *check.C) { | |||
| Shuffle(n, func(i, j int) { isRun++ }) | |||
| c.Assert(isRun, check.Equals, n-1) | |||
| } | |||
|
|
|||
| Shuffle(1<<31, func(i, j int) { | |||
| if 31-1 <= i { | |||
There was a problem hiding this comment.
Could you please add some description here? I'm not very clear about its meaning of this.
There was a problem hiding this comment.
Because if i let the loop complete, it will time out.
So i need to make the loop stop early.
There was a problem hiding this comment.
sorry, this is my mistake. It should be 1<<31-1.
zouy414
changed the title
| @@ -39,6 +39,9 @@ func (suite *AtomicCountUtilSuite) TestAdd(c *check.C) { | |||
|
|
|||
| result := acount.Get() | |||
| c.Check(result, check.Equals, (int32)(12)) | |||
|
|
|||
| var nilAcount *AtomicInt | |||
| c.Check(nilAcount.Add(5), check.Equals, (int32)(0)) | |||
There was a problem hiding this comment.
It makes me confused why there is no panic when invoking the function of nil struct pointer. After a while, I understand: nilAcount.Add(5) equals (*AtomicInt).Add(nilAcount, 5) in GoLang, and the implementation of function AtomicInt#Add checks nil pointer before dereference it.
But is it a reasonable implementation?
- It's rare to check itself in a struct's own function. Why not let it panic directly?
- The behavior of the
AtomicIntfunctions are inconsistent:Setdoesn't check. - Any value plus zero value should be equal to itself, but
Addreturns 0.
@starnop WDYT
|
Could you rebase your code and combine your commits into one commit? |
zouy414
force-pushed
the
master
branch
2 times, most recently
from
31497b2 to
6da8759
Compare
|
@lowzj , Of course. Done. |
Signed-off-by: ZouYu <[email protected]>
Signed-off-by: ZouYu [email protected]
Ⅰ. Describe what this PR did
Add unit test case
Ⅱ. Does this pull request fix one issue?
NONE
Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)
NONE
Ⅳ. Describe how to verify it
NONE
Ⅴ. Special notes for reviews
NONE