-
Notifications
You must be signed in to change notification settings - Fork 76
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
Implement pkg/agent/handler createAndSaveIndex test case #1794
Implement pkg/agent/handler createAndSaveIndex test case #1794
Conversation
Signed-off-by: kevindiu <kevindiujp@gmail.com>
[CHATOPS:HELP] ChatOps commands.
|
Codecov ReportBase: 50.23% // Head: 30.29% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1794 +/- ##
===========================================
- Coverage 50.23% 30.29% -19.95%
===========================================
Files 231 371 +140
Lines 13464 34019 +20555
===========================================
+ Hits 6764 10305 +3541
- Misses 6499 23293 +16794
- Partials 201 421 +220
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Deploying with Cloudflare Pages
|
LGTM (only test case plan) |
Signed-off-by: kevindiu <kevindiujp@gmail.com>
Signed-off-by: kevindiu <kevindiujp@gmail.com>
checkBackupFolder := func(fields fields, ctx context.Context, wantVecs []*payload.Insert_Request) error { | ||
// create another server instance to check if any vector is inserted and saved to the backup dir | ||
eg, _ := errgroup.New(ctx) | ||
ngt, err := service.New(fields.svcCfg, append(fields.svcOpts, |
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 🐶
Function New->mktmp->initNGT
should pass the context parameter (contextcheck)
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.
Please fix it.
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.
this warning means that checkBackupFolder
accept context parameter, and this warning warn us to pass the context to serviceNew()
.
it doesn't require the context on our implementation, so I cannot fix it and I think we can ignore it.
checkBackupFolder := func(fields fields, ctx context.Context, wantVecs []*payload.Insert_Request) error { | ||
// create another server instance to check if any vector is inserted and saved to the backup dir | ||
eg, _ := errgroup.New(ctx) | ||
ngt, err := service.New(fields.svcCfg, append(fields.svcOpts, |
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.
Please fix it.
Signed-off-by: kevindiu <kevindiujp@gmail.com>
Co-authored-by: Kiichiro YUKAWA <kyukawa315@gmail.com>
Signed-off-by: kevindiu <kevindiujp@gmail.com>
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
beforeFunc: func(t *testing.T, ctx context.Context, s Server, n service.NGT, test test) { | ||
t.Helper() | ||
var err error | ||
if ir, err = request.GenMultiInsertReq(request.Float, vector.Gaussian, insertCnt, dim, defaultInsertConfig); err != nil { | ||
t.Error(err) | ||
} | ||
if _, err := s.MultiInsert(ctx, ir); err != nil { | ||
t.Error(err) | ||
} | ||
}, |
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.
[ask]
When beforeFunc
fails, is Error ok? 🤔
I ask because I was wondering if we basically need to run tests on the target function when beforeFunc
fails!
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.
t.Error
fails the test so I think it is ok to use t.Error
.
t.Fatal
also fails the case, but it stops executing the test cases afterward, so I decided to use t.Error
here.
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.
OK, I got it! Thank you!
Signed-off-by: kevindiu kevindiujp@gmail.com
Description:
This PR implements pkg/agent/handler createAndSaveIndex test case, including the following changes:
fields
to initialize ngt and server instance through New() using functional option patternwant
to check error using errCodeRelated Issue:
How Has This Been Tested?:
Environment:
Types of changes:
Changes to Core Features:
Checklist: