Implement agent handler saveIndex test case #1731
Conversation
|
[CHATOPS:HELP] ChatOps commands.
|
github-actions
bot
added
area/agent/core
team/set
Codecov ReportBase: 30.29% // Head: 30.38% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1731 +/- ##
==========================================
+ Coverage 30.29% 30.38% +0.08%
==========================================
Files 370 370
Lines 33689 33689
==========================================
+ Hits 10206 10235 +29
+ Misses 23062 23040 -22
+ Partials 421 414 -7
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. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
vankichi
requested review from
a team and
datelier
and removed request for
a team
kevindiu
force-pushed
the
test/pkg/add-pkg-agent-handler-save-index-test
branch
from
2bc9812 to
be3becc
Compare
Deploying with
|
| Latest commit: |
36c57c9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://71687013.vald.pages.dev |
| Branch Preview URL: | https://test-pkg-add-pkg-agent-handl-f7mh.vald.pages.dev |
kevindiu
force-pushed
the
test/pkg/add-pkg-agent-handler-save-index-test
branch
from
6afc6b5 to
a3048a4
Compare
kevindiu
force-pushed
the
test/pkg/add-pkg-agent-handler-save-index-test
branch
from
9c8ae91 to
b92f0f8
Compare
| 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.
🚫 [golangci] reported by reviewdog 🐶
Function New->mktmp->initNGT should pass the context parameter (contextcheck)
kevindiu
force-pushed
the
test/pkg/add-pkg-agent-handler-save-index-test
branch
2 times, most recently
from
220e937 to
a5846ba
Compare
Signed-off-by: kevindiu <[email protected]>
Signed-off-by: kevindiu <[email protected]>
kevindiu
changed the title
| defaultAfterFunc := func(test test) { | ||
| os.RemoveAll(test.fields.indexPath) | ||
| } | ||
| checkBackupFolder := func(fields fields, ctx context.Context, wantVecs []*payload.Insert_Request) error { |
There was a problem hiding this comment.
could you please add the description when it is used.
And, i think it would be better to move it into the defaultCheckFunc.
There was a problem hiding this comment.
It cannot be moved to defaultCheckFunc because we need the wantVecs, which is created in the test function.
e.g. https://github.com/vdaas/vald/pull/1731/files#diff-86c3d3eae3293a28f5a4f5d3b45b9e0ec9c6654f36980af45bf9111f6a3fa22dR908
There was a problem hiding this comment.
Did you add the discription?
| @@ -508,106 +508,529 @@ func Test_server_CreateIndex(t *testing.T) { | |||
| func Test_server_SaveIndex(t *testing.T) { | |||
| t.Parallel() | |||
| type args struct { | |||
| ctx context.Context | |||
| in1 *payload.Empty | |||
There was a problem hiding this comment.
could you update this value to more understandable?
There was a problem hiding this comment.
I am sorry that I cant understand what you mean?
There was a problem hiding this comment.
fixed by rename it to in
| }, | ||
| } | ||
| }(), | ||
| { |
There was a problem hiding this comment.
please format coding style
There was a problem hiding this comment.
do you mean writing it as
func() test {
....
}(),
Signed-off-by: kevindiu <[email protected]>
Signed-off-by: kevindiu <[email protected]>
kevindiu
requested review from
a team and
hlts2
and removed request for
datelier and
a team
| timeout := time.After(10 * time.Second) | ||
| ticker := time.Tick(10 * time.Millisecond) | ||
| for { | ||
| select { | ||
| case <-timeout: | ||
| // execute save index twice | ||
| go s.SaveIndex(ctx, emptyPayload) | ||
|
|
||
| gotRes, err := s.SaveIndex(ctx, emptyPayload) | ||
| if err := defaultCheckFunc(t, ctx, s, n, w, gotRes, err); err != nil { | ||
| return err | ||
| } | ||
| case <-ticker: | ||
| // execute save index twice | ||
| go s.SaveIndex(ctx, emptyPayload) | ||
|
|
||
| gotRes, err := s.SaveIndex(ctx, emptyPayload) | ||
| if err := defaultCheckFunc(t, ctx, s, n, w, gotRes, err); err == nil { | ||
| return nil | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Can you please explain the background of this implementation? 🙏
There was a problem hiding this comment.
since we cannot guarantee the first saveIndex call on line 773 is executed asynchronously when we execute the actual saveIndex call on line 1041, I wanted to check it multiple times when it is not happen.
Signed-off-by: kevindiu <[email protected]>
Signed-off-by: kevindiu [email protected]
Description:
This PR implements saveIndex test cases.
It updates the
args,fields,wantand thedefaultCheckFuncfrom the generated test stub.contextfromargsand create it in test code to ensure that the context is handled correctly in test code.fieldsare changed to the functional options and configurations to initializeserverinstance.wantanddefaultCheckFuncare updated to check the error code only in the test case, otherwise it is very hard to ensure the correct error is produce by the function.Related Issue:
How Has This Been Tested?:
Environment:
Types of changes:
Changes to Core Features:
Checklist: