-
Notifications
You must be signed in to change notification settings - Fork 78
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 test case for internal/errors/net.go #969
Conversation
[CHATOPS:HELP] ChatOps commands.
|
Codecov Report
@@ Coverage Diff @@
## master #969 +/- ##
==========================================
- Coverage 14.35% 14.25% -0.11%
==========================================
Files 493 493
Lines 27898 27898
==========================================
- Hits 4006 3978 -28
- Misses 23647 23670 +23
- Partials 245 250 +5
Continue to review full report at Codecov.
|
internal/errors/net_test.go
Outdated
{ | ||
name: "return an ErrInvalidDNSConfig when dnsRefreshDur is 5 minute, dnsCacheExp is 4 minute", | ||
args: args{ | ||
dnsRefreshDur: 5 * time.Minute, | ||
dnsCacheExp: 4 * time.Minute, | ||
}, | ||
want: want{ | ||
want: New("dnsRefreshDuration > dnsCacheExp, 5m0s, 4m0s"), | ||
}, | ||
}, | ||
{ | ||
name: "return an ErrInvalidDNSConfig when dnsRefreshDur is 0, dnsCacheExp is 0", | ||
args: args{ | ||
dnsRefreshDur: 0, | ||
dnsCacheExp: 0, | ||
}, | ||
want: want{ | ||
want: New("dnsRefreshDuration > dnsCacheExp, 0s, 0s"), | ||
}, | ||
}, | ||
{ | ||
name: "return an ErrInvalidDNSConfig when dnsRefreshDur and dnsCacheExp are the minimum number of int64", | ||
args: args{ | ||
dnsRefreshDur: time.Duration(math.MinInt64), | ||
dnsCacheExp: time.Duration(math.MinInt64), | ||
}, | ||
want: want{ | ||
want: Errorf("dnsRefreshDuration > dnsCacheExp, %s, %s", time.Duration(math.MinInt64), time.Duration(math.MinInt64)), | ||
}, | ||
}, | ||
{ | ||
name: "return an ErrInvalidDNSConfig when dnsRefreshDur and dnsCacheExp are the maximum number of int64", | ||
args: args{ | ||
dnsRefreshDur: time.Duration(math.MaxInt64), | ||
dnsCacheExp: time.Duration(math.MaxInt64), | ||
}, | ||
want: want{ | ||
want: Errorf("dnsRefreshDuration > dnsCacheExp, %s, %s", time.Duration(math.MaxInt64), time.Duration(math.MaxInt64)), | ||
}, | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Could you add below case?
- when dnsRefreshDur is nil.
- when dnsCacheExp is nil.
- when dnsRefreshDur and dnsCacheExp is nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vankichi thank you for your comment.
time.Duration
is a primitive type (to be exact, an alias for primitive type), so I think we can not add a test case of nil pattern. 🤔
↓ An example of that nil is put in time.Duration
type.
https://play.golang.org/p/xdI5op3KTUu
From the above, instead of the nil case pattern, I added zero value pattern 🤔
what do you think?
if I made a mistake, I am sorry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks.
So I think we have to add below patterns
- when dnsRefreshDur is 0.
- when dnsCacheExp is 0.
and you already add the test when both of these value is 0, it'd be better renaming return an ErrInvalidDNSConfig when dnsRefreshDur is default value(or some better word), dnsCacheExp is default value (or some more better word)
from return an ErrInvalidDNSConfig when dnsRefreshDur is 0, dnsCacheExp is 0
.
also, it'd be better clearly if you remove these values definition from args{}
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/rebase |
[REBASE] Rebase triggered by kevindiu for branch: test/internal/add-newtest-for-errors-net |
Signed-off-by: hlts2 <[email protected]>
Signed-off-by: hlts2 <[email protected]>
Signed-off-by: hlts2 <[email protected]>
Co-authored-by: Kiichiro YUKAWA <[email protected]>
b2c6404
to
3f445f2
Compare
[FORMAT] Updating license headers and formatting go codes triggered by kevindiu. |
Signed-off-by: vdaas-ci <[email protected]>
Signed-off-by: hlts2 [email protected]
Description:
I added the test cases for internal/errors/net.go and comment.
And fixed code comments more clearly.
grammar check passed
Related Issue:
How Has This Been Tested?:
Environment:
Types of changes:
Changes to Core Features:
Checklist: