-
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
Added test case for internal/info
pacakge.
#514
Conversation
Best reviewed: commit by commit
Optimal code review plan
|
/rebase |
[REBASE] Rebase triggered by hlts2 for branch: test/internal/info |
2b03782
to
1c4105d
Compare
Codecov Report
@@ Coverage Diff @@
## master #514 +/- ##
=========================================
+ Coverage 8.86% 9.03% +0.16%
=========================================
Files 402 402
Lines 20831 20831
=========================================
+ Hits 1847 1882 +35
+ Misses 18731 18696 -35
Partials 253 253
Continue to review full report at Codecov.
|
@hlts2 Seems some test case is not completed. |
How do you think add error case or default params case? |
/rebase |
[REBASE] Rebase triggered by hlts2 for branch: test/internal/info |
Please add comments to |
seems |
/rebase |
[REBASE] Rebase triggered by hlts2 for branch: test/internal/info |
25c51b8
to
4a1e910
Compare
internal/info/info.go
Outdated
@@ -45,6 +46,7 @@ type Detail struct { | |||
PrepOnce sync.Once `json:"-" yaml:"-"` | |||
} | |||
|
|||
// StackTrace represents stacktrace infomation about url, function name, file, line ..etc. |
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.
[golangci] reported by reviewdog 🐶
infomation
is a misspelling of information
(misspell)
/rebase |
LGTM but please fix #514 (comment) first :) |
[REBASE] Rebase triggered by kevindiu for branch: test/internal/info |
Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
ccb2691
to
571097f
Compare
Co-authored-by: Kevin Diu <kevin_diu@yahoo.com.hk>
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 vankichi for branch: test/internal/info |
[FORMAT] Updating license headers and formatting go codes triggered by vankichi. |
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.
[APPROVED] This PR is approved by vankichi.
Description:
I added a test case for the
internal/info
package.However, I'm not able to reach some code because I'm calling
runtime.Caller
directly.Therefore, I skipped the tests related to the parts other than initialization.
As a future move, I make
runtime.Callder
subordinate to another structure and change the implementation code to get the information via the interface. After making the changes, I implement the tests for that part.The following is target code.
https://github.com/vdaas/vald/blob/master/internal/info/info.go#L153-L160
Related Issue:
How Has This Been Tested?:
Environment:
Types of changes:
Changes to Core Features:
Checklist: