Skip to content

[BEAM-13870] [Playground] Increase test coverage for the cache package #16826

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.

Sign up for GitHub

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

Merged
pabloem merged 5 commits into from
Feb 24, 2022
Merged

[BEAM-13870] [Playground] Increase test coverage for the cache package #16826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
630e6b2
Increase test coverage for the cache package
pavel-avilov Feb 11, 2022
7d55c91
Add commentaries for several tests.
pavel-avilov Feb 14, 2022
b507694
Edit comments
pavel-avilov Feb 16, 2022
18e1f14
Edit comments
pavel-avilov Feb 16, 2022
398620f
Refactoring code
pavel-avilov Feb 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 103 additions & 12 deletions playground/backend/internal/cache/local/local_cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -76,7 +78,7 @@ func TestLocalCache_GetValue(t *testing.T) {
wantErr bool
}{
{
name: "Get exist value",
name: "Get existing value",
fields: fields{
cleanupInterval: cleanupInterval,
items: preparedItemsMap,
Expand All @@ -91,7 +93,7 @@ func TestLocalCache_GetValue(t *testing.T) {
wantErr: false,
},
{
name: "Get not exist value",
name: "Get not existing value",
fields: fields{
cleanupInterval: cleanupInterval,
items: make(map[uuid.UUID]map[cache.SubKey]interface{}),
Expand All @@ -103,6 +105,21 @@ func TestLocalCache_GetValue(t *testing.T) {
want: nil,
wantErr: true,
},
{
name: "Get an existing value that is expiring",
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) {
Expand Down Expand Up @@ -159,6 +176,21 @@ func TestLocalCache_SetValue(t *testing.T) {
},
wantErr: false,
},
{
name: "Set value for RunOutputIndex 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) {
Expand All @@ -180,6 +212,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{}
Expand All @@ -191,12 +226,27 @@ 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
}{
{
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,
},
{
name: "Set expiration time for not existing value in cache items",
fields: fields{
cleanupInterval: cleanupInterval,
items: make(map[uuid.UUID]map[cache.SubKey]interface{}),
Expand All @@ -207,6 +257,7 @@ func TestLocalCache_SetExpTime(t *testing.T) {
pipelineId: preparedId,
expTime: time.Minute,
},
wantErr: true,
},
}
for _, tt := range tests {
Expand All @@ -216,14 +267,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)
}
}
})
}
Expand Down Expand Up @@ -429,6 +479,16 @@ func TestLocalCache_startGC(t *testing.T) {
pipelinesExpiration: preparedExpMap,
},
},
{
// Test case with calling startGC method with nil cache items.
// As a result, items stay the same.
name: "Checking for deleting expired pipelines with nil cache items",
fields: fields{
cleanupInterval: time.Microsecond,
items: nil,
pipelinesExpiration: preparedExpMap,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -540,3 +600,34 @@ 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
}{
{
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)
}
})
}
}
68 changes: 54 additions & 14 deletions playground/backend/internal/cache/redis/redis_cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestRedisCache_GetValue(t *testing.T) {
wantErr bool
}{
{
name: "error during HGet operation",
name: "Error during HGet operation",
mocks: func() {
mock.ExpectHGet(pipelineId.String(), string(marshSubKey)).SetErr(fmt.Errorf("MOCK_ERROR"))
},
Expand All @@ -68,7 +68,7 @@ func TestRedisCache_GetValue(t *testing.T) {
wantErr: true,
},
{
name: "all success",
name: "Get existing value",
mocks: func() {
mock.ExpectHGet(pipelineId.String(), string(marshSubKey)).SetVal(string(marshValue))
},
Expand Down Expand Up @@ -122,7 +122,7 @@ func TestRedisCache_SetExpTime(t *testing.T) {
wantErr bool
}{
{
name: "error during Exists operation",
name: "Error during Exists operation",
mocks: func() {
mock.ExpectExists(pipelineId.String()).SetErr(fmt.Errorf("MOCK_ERROR"))
},
Expand All @@ -148,7 +148,7 @@ func TestRedisCache_SetExpTime(t *testing.T) {
wantErr: true,
},
{
name: "error during Expire operation",
name: "Set expiration time with error during Expire operation",
mocks: func() {
mock.ExpectExists(pipelineId.String()).SetVal(1)
mock.ExpectExpire(pipelineId.String(), expTime).SetErr(fmt.Errorf("MOCK_ERROR"))
Expand All @@ -162,7 +162,7 @@ func TestRedisCache_SetExpTime(t *testing.T) {
wantErr: true,
},
{
name: "all success",
name: "Set expiration time",
mocks: func() {
mock.ExpectExists(pipelineId.String()).SetVal(1)
mock.ExpectExpire(pipelineId.String(), expTime).SetVal(true)
Expand Down Expand Up @@ -215,7 +215,7 @@ func TestRedisCache_SetValue(t *testing.T) {
wantErr bool
}{
{
name: "error during HSet operation",
name: "Error during HSet operation",
mocks: func() {
mock.ExpectHSet(pipelineId.String(), marshSubKey, marshValue).SetErr(fmt.Errorf("MOCK_ERROR"))
},
Expand All @@ -229,7 +229,7 @@ func TestRedisCache_SetValue(t *testing.T) {
wantErr: true,
},
{
name: "all success",
name: "Set correct value",
mocks: func() {
mock.ExpectHSet(pipelineId.String(), marshSubKey, marshValue).SetVal(1)
mock.ExpectExpire(pipelineId.String(), time.Minute*15).SetVal(true)
Expand All @@ -243,6 +243,21 @@ func TestRedisCache_SetValue(t *testing.T) {
},
wantErr: false,
},
{
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) {
Expand Down Expand Up @@ -318,7 +333,7 @@ func TestCache_GetCatalog(t *testing.T) {
wantErr bool
}{
{
name: "Success",
name: "Get existing catalog",
mocks: func() {
mock.ExpectGet(cache.ExamplesCatalog).SetVal(string(catalogMarsh))
},
Expand Down Expand Up @@ -416,7 +431,7 @@ func TestCache_SetCatalog(t *testing.T) {
wantErr bool
}{
{
name: "Success",
name: "Set catalog",
mocks: func() {
mock.ExpectSet(cache.ExamplesCatalog, catalogMarsh, 0).SetVal("")
},
Expand Down Expand Up @@ -465,7 +480,7 @@ func Test_newRedisCache(t *testing.T) {
wantErr bool
}{
{
name: "error during Ping operation",
name: "Error during Ping operation",
args: args{
ctx: context.Background(),
addr: address,
Expand All @@ -487,6 +502,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
Expand All @@ -499,7 +517,7 @@ func Test_unmarshalBySubKey(t *testing.T) {
wantErr bool
}{
{
name: "status subKey",
name: "Status subKey",
args: args{
ctx: context.Background(),
subKey: cache.Status,
Expand All @@ -509,7 +527,7 @@ func Test_unmarshalBySubKey(t *testing.T) {
wantErr: false,
},
{
name: "runOutput subKey",
name: "RunOutput subKey",
args: args{
subKey: cache.RunOutput,
value: string(outputValue),
Expand All @@ -518,7 +536,7 @@ func Test_unmarshalBySubKey(t *testing.T) {
wantErr: false,
},
{
name: "compileOutput subKey",
name: "CompileOutput subKey",
args: args{
subKey: cache.CompileOutput,
value: string(outputValue),
Expand All @@ -527,14 +545,36 @@ func Test_unmarshalBySubKey(t *testing.T) {
wantErr: false,
},
{
name: "graph subKey",
name: "Graph subKey",
args: args{
subKey: cache.Graph,
value: string(outputValue),
},
want: output,
wantErr: false,
},
{
// Test case with calling unmarshalBySubKey method with Canceled subKey.
// As a result, want to receive false.
Copy link
Contributor

@daria-malkova daria-malkova Feb 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it will be false?

Copy link
Contributor Author

@pavel-avilov pavel-avilov Feb 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daria-malkova because canceledValue is JSON encoding of the false.

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 runOutputIndex.
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) {
Expand Down