This is an automated email from the ASF dual-hosted git repository.

pabloem pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/beam.git


The following commit(s) were added to refs/heads/master by this push:
     new 2a45a5b  Merge pull request #16826 from [BEAM-13870] [Playground] 
Increase test coverage for the cache package
2a45a5b is described below

commit 2a45a5b6c34141dcc595c5cda3d28264e542bdff
Author: Pavel Avilov <avilovpav...@gmail.com>
AuthorDate: Thu Feb 24 08:27:02 2022 +0300

    Merge pull request #16826 from [BEAM-13870] [Playground] Increase test 
coverage for the cache package
    
    * Increase test coverage for the cache package
    
    * Add commentaries for several tests.
    
    * Edit comments
    
    * Edit comments
    
    * Refactoring code
---
 .../internal/cache/local/local_cache_test.go       | 115 ++++++++++++++++++---
 .../internal/cache/redis/redis_cache_test.go       |  68 +++++++++---
 2 files changed, 157 insertions(+), 26 deletions(-)

diff --git a/playground/backend/internal/cache/local/local_cache_test.go 
b/playground/backend/internal/cache/local/local_cache_test.go
index c4d5a58..34b2654 100644
--- a/playground/backend/internal/cache/local/local_cache_test.go
+++ b/playground/backend/internal/cache/local/local_cache_test.go
@@ -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{}
@@ -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,
@@ -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{}),
@@ -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) {
@@ -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) {
@@ -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{}
@@ -191,14 +226,29 @@ 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{}),
                                pipelinesExpiration: 
make(map[uuid.UUID]time.Time),
                        },
@@ -207,6 +257,7 @@ func TestLocalCache_SetExpTime(t *testing.T) {
                                pipelineId: preparedId,
                                expTime:    time.Minute,
                        },
+                       wantErr: true,
                },
        }
        for _, tt := range tests {
@@ -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)
+                               }
                        }
                })
        }
@@ -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) {
@@ -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)
+                       }
+               })
+       }
+}
diff --git a/playground/backend/internal/cache/redis/redis_cache_test.go 
b/playground/backend/internal/cache/redis/redis_cache_test.go
index 02dec43..60a0ecf 100644
--- a/playground/backend/internal/cache/redis/redis_cache_test.go
+++ b/playground/backend/internal/cache/redis/redis_cache_test.go
@@ -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"))
                        },
@@ -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))
                        },
@@ -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"))
                        },
@@ -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"))
@@ -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)
@@ -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"))
                        },
@@ -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)
@@ -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) {
@@ -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))
                        },
@@ -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("")
                        },
@@ -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,
@@ -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
@@ -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,
@@ -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),
@@ -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),
@@ -527,7 +545,7 @@ func Test_unmarshalBySubKey(t *testing.T) {
                        wantErr: false,
                },
                {
-                       name: "graph subKey",
+                       name: "Graph subKey",
                        args: args{
                                subKey: cache.Graph,
                                value:  string(outputValue),
@@ -535,6 +553,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.
+                       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) {

Reply via email to