Add test case for internal/errors/grpc.go #903

hlts2 · 2020-12-28T01:47:59Z

Description:

I added the test cases for internal/errors/grpc.go and comments.

And I modified the function name based on kevin-san's comment.

Add test case for internal/errors/grpc.go #903 (comment)

grammar check passed

Related Issue:

How Has This Been Tested?:

Environment:

Go Version: 1.15.2
Docker Version: 19.03.8
Kubernetes Version: 1.18.2
NGT Version: 1.12.1

Types of changes:

Bug fix [type/bug]
New feature [type/feature]
Add tests [type/test]
Security related changes [type/security]
Add documents [type/documentation]
Refactoring [type/refactoring]
Update dependencies [type/dependency]
Update benchmarks and performances [type/bench]
Update CI [type/ci]

Changes to Core Features:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your core changes, as applicable?
Have you successfully ran tests with your changes locally?

Checklist:

I have read the CONTRIBUTING document.
I have checked open Pull Requests for the similar feature or fixes?
I have added tests and benchmarks to cover my changes.
I have ensured all new and existing tests passed.
I have commented my code, particularly in hard-to-understand areas
I have updated the documentation accordingly.

vdaas-ci · 2020-12-28T01:48:15Z

[CHATOPS:HELP] ChatOps commands.

🙆‍♀️ /approve - approve
💌 /changelog - add changelog comment
🍱 /format - format codes and add licenses
✅ /gen-test - generate test codes
🏷️ /label - add labels
⏪ /rebase - rebase master

github-actions · 2020-12-28T01:54:08Z

internal/errors/grpc.go

@@ -33,19 +33,24 @@ var (
 		return Errorf("invalid gRPC client connection to %s", addr)
 	}

+	// ErrGRPCLookupIPAddrNotFound represents a function to generate an error that the vald internal gRPC couldn't find ip address.


[golangci] _{reported by reviewdog 🐶}
line is 128 characters (lll)

codecov · 2020-12-28T01:54:42Z

Codecov Report

Merging #903 (7c092fd) into master (5b8cfad) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #903      +/-   ##
==========================================
+ Coverage   19.16%   19.17%   +0.01%     
==========================================
  Files         423      422       -1     
  Lines       19602    19548      -54     
==========================================
- Hits         3756     3749       -7     
+ Misses      15628    15590      -38     
+ Partials      218      209       -9

Impacted Files	Coverage Δ
internal/errors/grpc.go	`100.00% <100.00%> (+100.00%)`	⬆️
internal/worker/worker.go	`82.29% <0.00%> (-1.05%)`	⬇️
internal/net/net.go
internal/db/storage/blob/s3/reader/reader.go	`97.70% <0.00%> (+32.18%)`	⬆️

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 fd20606...cc07abd. Read the comment docs.

internal/errors/grpc.go

kevindiu · 2021-01-05T01:16:27Z

internal/errors/grpc.go

@@ -19,33 +19,39 @@ package errors

 var (

-	// gRPC
-
+	// ErrgRPCClientConnectionClose represents a function to generate an error that the gRPC connection couldn't close.
 	ErrgRPCClientConnectionClose = func(name string, err error) error {


I think it should be ErrGRPCClientConnectionClose

I could not understand your opinion ...
I'm sorry I don't understand.:bow:
so could you please tell me about this more clearly?? 🙇

Does this mean I need to modify the function name? 🤔
I think your opinion's function name(ErrgRPCClientConnectionClose ) and the function name (ErrgRPCClientConnectionClose ) in the original code are the same 🤔
If I made a mistake, I am sorry

sorry it was my mistake. I have modified the comment above.

thank you. 🙏
But I can not see the modified your comment above...:thinking:
It may not have been modified 🤔
so could you please re-modify it ?? 🙇

Sorry I should describe here.
Here is the suggestion.

Suggested change

ErrgRPCClientConnectionClose = func(name string, err error) error {

ErrGRPCClientConnectionClose = func(name string, err error) error {

thank you. I get it 👍
I will fix it soon 🙏

Does this change affect current code?
do you already check?

@vankichi
thank you for your comment.

Does this change affect current code?

I think this change does not affect to current code. 🤔

do you already check?

Yes, I did.
I searched for a usage points before, but I couldn't find it.
The following is the searched result 🙇
https://github.com/vdaas/vald/search?q=ErrgRPCClientConnectionClose

vankichi

@hlts2 leave comment

internal/errors/grpc_test.go

vankichi · 2021-01-05T06:16:30Z

internal/errors/grpc_test.go

+	type args struct {
+		addr string
+		host string
+		port uint16


I thinks it'd be better add case with boundary value of uint16

Thank you for your comment. 🙇
I agree with you. 👍

I selected 0 and maxUint16and 8080 as port value. 🤔
Originally, maxUint16 + 1, -1 should be also included as boundary value, but it could not be included due to a compile error. ...
So could you tell me which value should be selected as the boundary value of uint16?
sorry 🙏

For example, uint16 range is from 0 to maxUint16 🤔
so, is the boundary value below?

0

1

maxUint16-1

maxUint16

plz refer to https://golang.org/pkg/math/#pkg-constants

@vankichi
thank you

internal/errors/grpc.go

internal/errors/grpc_test.go

vankichi

LGTM

kevindiu · 2021-01-07T01:46:17Z

/rebase
/format
/approve

vdaas-ci · 2021-01-07T01:46:57Z

[REBASE] Rebase triggered by kevindiu for branch: test/internal/add-errors-grpc-test

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

Co-authored-by: Kiichiro YUKAWA <kyukawa315@gmail.com>

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

vdaas-ci · 2021-01-07T01:47:52Z

[FORMAT] Updating license headers and formatting go codes triggered by kevindiu.

Signed-off-by: vdaas-ci <ci@vdaas.org>

kevindiu · 2021-01-08T01:55:38Z

/rebase
/format
/approve

vdaas-ci · 2021-01-08T01:56:45Z

[REBASE] Rebase triggered by kevindiu for branch: test/internal/add-errors-grpc-test

vdaas-ci · 2021-01-08T01:57:36Z

[FORMAT] Updating license headers and formatting go codes triggered by kevindiu.

hlts2 requested a review from kevindiu December 28, 2020 01:47

hlts2 changed the title ~~Add test case for internal/errors/grpc.go~~ [WIP] Add test case for internal/errors/grpc.go Dec 28, 2020

github-actions bot added area/internal team/set SET team size/L type/test Adding tests priority/low labels Dec 28, 2020

github-actions bot reviewed Dec 28, 2020

View reviewed changes

hlts2 changed the title ~~[WIP] Add test case for internal/errors/grpc.go~~ Add test case for internal/errors/grpc.go Dec 28, 2020

github-actions bot added size/XL and removed size/L labels Dec 28, 2020

hlts2 commented Dec 28, 2020

View reviewed changes

internal/errors/grpc.go Outdated Show resolved Hide resolved

internal/errors/grpc.go Outdated Show resolved Hide resolved

kevindiu reviewed Jan 5, 2021

View reviewed changes

hlts2 requested a review from vankichi January 5, 2021 02:24

vankichi reviewed Jan 5, 2021

View reviewed changes

github-actions bot reviewed Jan 6, 2021

View reviewed changes

vankichi previously approved these changes Jan 6, 2021

View reviewed changes

hlts2 and others added 8 commits January 7, 2021 01:47

fix: test case name for grpc error

9f2819c

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix: add comment & test case name more clearly

686bdf3

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix: add comment & test case for ErrInvalidGRPCPort method

e80b48d

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix: apply suggestion

81b50b5

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

Update internal/errors/grpc.go

ab895da

Update internal/errors/grpc.go

45a12e6

Apply suggestions from code review

b425cfe

Co-authored-by: Kiichiro YUKAWA <kyukawa315@gmail.com>

fix: apply suggestion about boundary value

aa50a19

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

github-actions bot dismissed vankichi’s stale review via aa50a19 January 7, 2021 01:47

github-actions bot force-pushed the test/internal/add-errors-grpc-test branch from b793f54 to aa50a19 Compare January 7, 2021 01:47

🤖 Update license headers / Format go codes and yaml files

cc07abd

Signed-off-by: vdaas-ci <ci@vdaas.org>

kevindiu approved these changes Jan 8, 2021

View reviewed changes

kevindiu merged commit 0e93f6b into master Jan 8, 2021

kevindiu deleted the test/internal/add-errors-grpc-test branch January 8, 2021 02:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add test case for internal/errors/grpc.go #903

Add test case for internal/errors/grpc.go #903

hlts2 commented Dec 28, 2020 •

edited

Loading

vdaas-ci commented Dec 28, 2020

github-actions bot Dec 28, 2020

codecov bot commented Dec 28, 2020 •

edited

Loading

kevindiu Jan 5, 2021 •

edited

Loading

hlts2 Jan 5, 2021 •

edited

Loading

kevindiu Jan 5, 2021

hlts2 Jan 5, 2021 •

edited

Loading

kevindiu Jan 5, 2021

hlts2 Jan 5, 2021

vankichi Jan 5, 2021

hlts2 Jan 5, 2021 •

edited

Loading

vankichi left a comment

vankichi Jan 5, 2021

hlts2 Jan 5, 2021

hlts2 Jan 6, 2021

vankichi Jan 6, 2021

vankichi Jan 6, 2021

hlts2 Jan 6, 2021

vankichi left a comment

kevindiu commented Jan 7, 2021

vdaas-ci commented Jan 7, 2021

vdaas-ci commented Jan 7, 2021

kevindiu commented Jan 8, 2021

vdaas-ci commented Jan 8, 2021

vdaas-ci commented Jan 8, 2021

	ErrgRPCClientConnectionClose = func(name string, err error) error {
	ErrGRPCClientConnectionClose = func(name string, err error) error {

Add test case for internal/errors/grpc.go #903

Add test case for internal/errors/grpc.go #903

Conversation

hlts2 commented Dec 28, 2020 • edited Loading

Description:

Related Issue:

How Has This Been Tested?:

Environment:

Types of changes:

Changes to Core Features:

Checklist:

vdaas-ci commented Dec 28, 2020

github-actions bot Dec 28, 2020

Choose a reason for hiding this comment

codecov bot commented Dec 28, 2020 • edited Loading

Codecov Report

kevindiu Jan 5, 2021 • edited Loading

Choose a reason for hiding this comment

hlts2 Jan 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hlts2 Jan 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hlts2 Jan 5, 2021 • edited Loading

Choose a reason for hiding this comment

vankichi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vankichi left a comment

Choose a reason for hiding this comment

kevindiu commented Jan 7, 2021

vdaas-ci commented Jan 7, 2021

vdaas-ci commented Jan 7, 2021

kevindiu commented Jan 8, 2021

vdaas-ci commented Jan 8, 2021

vdaas-ci commented Jan 8, 2021

hlts2 commented Dec 28, 2020 •

edited

Loading

codecov bot commented Dec 28, 2020 •

edited

Loading

kevindiu Jan 5, 2021 •

edited

Loading

hlts2 Jan 5, 2021 •

edited

Loading

hlts2 Jan 5, 2021 •

edited

Loading

hlts2 Jan 5, 2021 •

edited

Loading