-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[BEAM-13870] [Playground] Increase test coverage for the cache package #16826
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,6 +58,8 @@ func TestLocalCache_GetValue(t *testing.T) { | |
preparedItemsMap[preparedId][preparedSubKey] = value | ||
preparedExpMap := make(map[uuid.UUID]time.Time) | ||
preparedExpMap[preparedId] = time.Now().Add(time.Millisecond) | ||
endedExpMap := make(map[uuid.UUID]time.Time) | ||
endedExpMap[preparedId] = time.Now().Add(-time.Millisecond) | ||
type fields struct { | ||
cleanupInterval time.Duration | ||
items map[uuid.UUID]map[cache.SubKey]interface{} | ||
|
@@ -103,6 +105,23 @@ func TestLocalCache_GetValue(t *testing.T) { | |
want: nil, | ||
wantErr: true, | ||
}, | ||
{ | ||
// Test case with calling GetValue method with exist value the expiration time of which has ended. | ||
// As a result, want to receive an error. | ||
name: "Get exist value the expiration time of which has ended", | ||
fields: fields{ | ||
cleanupInterval: cleanupInterval, | ||
items: preparedItemsMap, | ||
pipelinesExpiration: endedExpMap, | ||
}, | ||
args: args{ | ||
ctx: context.Background(), | ||
pipelineId: preparedId, | ||
subKey: preparedSubKey, | ||
}, | ||
want: nil, | ||
wantErr: true, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
|
@@ -159,6 +178,23 @@ func TestLocalCache_SetValue(t *testing.T) { | |
}, | ||
wantErr: false, | ||
}, | ||
{ | ||
// Test case with calling SetValue method with RunOutputIndex subKey. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
// As a result, want to save value to cache. | ||
name: "Set value for some index subKey", | ||
fields: fields{ | ||
cleanupInterval: cleanupInterval, | ||
items: make(map[uuid.UUID]map[cache.SubKey]interface{}), | ||
pipelinesExpiration: preparedExpMap, | ||
}, | ||
args: args{ | ||
ctx: context.Background(), | ||
pipelineId: preparedId, | ||
subKey: cache.RunOutputIndex, | ||
value: 5, | ||
}, | ||
wantErr: false, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
|
@@ -180,6 +216,9 @@ func TestLocalCache_SetValue(t *testing.T) { | |
|
||
func TestLocalCache_SetExpTime(t *testing.T) { | ||
preparedId, _ := uuid.NewUUID() | ||
preparedItems := make(map[uuid.UUID]map[cache.SubKey]interface{}) | ||
preparedItems[preparedId] = make(map[cache.SubKey]interface{}) | ||
preparedItems[preparedId][cache.Status] = 1 | ||
type fields struct { | ||
cleanupInterval time.Duration | ||
items map[uuid.UUID]map[cache.SubKey]interface{} | ||
|
@@ -191,12 +230,31 @@ func TestLocalCache_SetExpTime(t *testing.T) { | |
expTime time.Duration | ||
} | ||
tests := []struct { | ||
name string | ||
fields fields | ||
args args | ||
name string | ||
fields fields | ||
args args | ||
wantErr bool | ||
}{ | ||
{ | ||
// Test case with calling SetExpTime method with pipelineId and his expiration time. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Redundant There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
// As a result, want to set expiration time for this pipelineId. | ||
name: "Set expiration time", | ||
fields: fields{ | ||
cleanupInterval: cleanupInterval, | ||
items: preparedItems, | ||
pipelinesExpiration: make(map[uuid.UUID]time.Time), | ||
}, | ||
args: args{ | ||
ctx: context.Background(), | ||
pipelineId: preparedId, | ||
expTime: time.Minute, | ||
}, | ||
wantErr: false, | ||
}, | ||
{ | ||
// Test case with calling SetExpTime method with pipelineId which not set in cache and expiration time. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
// As a result, want to receive an error. | ||
name: "Set expiration time for not exist value", | ||
fields: fields{ | ||
cleanupInterval: cleanupInterval, | ||
items: make(map[uuid.UUID]map[cache.SubKey]interface{}), | ||
|
@@ -207,6 +265,7 @@ func TestLocalCache_SetExpTime(t *testing.T) { | |
pipelineId: preparedId, | ||
expTime: time.Minute, | ||
}, | ||
wantErr: true, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
|
@@ -216,14 +275,13 @@ func TestLocalCache_SetExpTime(t *testing.T) { | |
items: tt.fields.items, | ||
pipelinesExpiration: tt.fields.pipelinesExpiration, | ||
} | ||
_ = lc.SetValue(tt.args.ctx, preparedId, cache.Status, 1) | ||
err := lc.SetExpTime(tt.args.ctx, tt.args.pipelineId, tt.args.expTime) | ||
if err != nil { | ||
t.Error(err) | ||
} | ||
expTime, found := lc.pipelinesExpiration[tt.args.pipelineId] | ||
if expTime.Round(time.Second) != time.Now().Add(tt.args.expTime).Round(time.Second) || !found { | ||
t.Errorf("Expiration time of the pipeline: %s not set in cache.", tt.args.pipelineId) | ||
if (err != nil) != tt.wantErr { | ||
t.Errorf("SetExpTime() error = %v, wantErr %v", err, tt.wantErr) | ||
expTime, found := lc.pipelinesExpiration[tt.args.pipelineId] | ||
if expTime.Round(time.Second) != time.Now().Add(tt.args.expTime).Round(time.Second) || !found { | ||
t.Errorf("Expiration time of the pipeline: %s not set in cache.", tt.args.pipelineId) | ||
} | ||
} | ||
}) | ||
} | ||
|
@@ -429,6 +487,16 @@ func TestLocalCache_startGC(t *testing.T) { | |
pipelinesExpiration: preparedExpMap, | ||
}, | ||
}, | ||
{ | ||
// Test case with calling startGC method with nil items. | ||
// As a result, items stay the same. | ||
name: "Checking for deleting expired pipelines with nil items", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not clear There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
fields: fields{ | ||
cleanupInterval: time.Microsecond, | ||
items: nil, | ||
pipelinesExpiration: preparedExpMap, | ||
}, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
|
@@ -540,3 +608,36 @@ func TestLocalCache_clearItems(t *testing.T) { | |
}) | ||
} | ||
} | ||
|
||
func TestNew(t *testing.T) { | ||
items := make(map[uuid.UUID]map[cache.SubKey]interface{}) | ||
pipelinesExpiration := make(map[uuid.UUID]time.Time) | ||
type args struct { | ||
ctx context.Context | ||
} | ||
tests := []struct { | ||
name string | ||
args args | ||
want *Cache | ||
}{ | ||
{ | ||
// Test case with calling New method. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Redundant There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
// As a result, want to receive an expected Cache. | ||
name: "initialize local cache", | ||
args: args{ctx: context.Background()}, | ||
want: &Cache{ | ||
cleanupInterval: cleanupInterval, | ||
items: items, | ||
pipelinesExpiration: pipelinesExpiration, | ||
catalog: nil, | ||
}, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
if got := New(tt.args.ctx); !reflect.DeepEqual(got, tt.want) { | ||
t.Errorf("New() = %v, want %v", got, tt.want) | ||
} | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -243,6 +243,23 @@ func TestRedisCache_SetValue(t *testing.T) { | |
}, | ||
wantErr: false, | ||
}, | ||
{ | ||
// Test case with calling SetValue method with incorrect value. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Redundant There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
// As a result, want to receive an error during marshal value. | ||
name: "set incorrect value", | ||
mocks: func() { | ||
mock.ExpectHSet(pipelineId.String(), marshSubKey, marshValue).SetVal(1) | ||
mock.ExpectExpire(pipelineId.String(), time.Minute*15).SetVal(true) | ||
}, | ||
fields: fields{client}, | ||
args: args{ | ||
ctx: context.Background(), | ||
pipelineId: pipelineId, | ||
subKey: subKey, | ||
value: make(chan int), | ||
}, | ||
wantErr: true, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
|
@@ -487,6 +504,9 @@ func Test_unmarshalBySubKey(t *testing.T) { | |
statusValue, _ := json.Marshal(status) | ||
output := "MOCK_OUTPUT" | ||
outputValue, _ := json.Marshal(output) | ||
canceledValue, _ := json.Marshal(false) | ||
runOutputIndex := 0 | ||
runOutputIndexValue, _ := json.Marshal(runOutputIndex) | ||
type args struct { | ||
ctx context.Context | ||
subKey cache.SubKey | ||
|
@@ -535,6 +555,28 @@ func Test_unmarshalBySubKey(t *testing.T) { | |
want: output, | ||
wantErr: false, | ||
}, | ||
{ | ||
// Test case with calling unmarshalBySubKey method with Canceled subKey. | ||
// As a result, want to receive false. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why it will be false? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @daria-malkova because |
||
name: "Canceled subKey", | ||
args: args{ | ||
subKey: cache.Canceled, | ||
value: string(canceledValue), | ||
}, | ||
want: false, | ||
wantErr: false, | ||
}, | ||
{ | ||
// Test case with calling unmarshalBySubKey method with RunOutputIndex subKey. | ||
// As a result, want to receive expected runOutputIndexValue. | ||
name: "RunOutputIndex subKey", | ||
args: args{ | ||
subKey: cache.RunOutputIndex, | ||
value: string(runOutputIndexValue), | ||
}, | ||
want: float64(runOutputIndex), | ||
wantErr: false, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
|
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.
Thank you for adding these comments, could you please also add comments for other tests in these files? So every test in
local_cache_test.go
andredis_cache_test.go
has a corresponding comment😊