increase code coverage (#152)

* unit tests for transaction_registry_redis

* replacing mocks with miniredis

* add redis_cache tests

* increase code coverage

* increase coverage

* increase config code coverage

* fix from PR's comments

* add mistaken conf tests

* fix negative config tests

Co-authored-by: christophe.kalenzaga <[email protected]>

Author: mga-chka

cache/async_cache.go

@@ -1,10 +1,12 @@
 package cache
 
 import (
+	"fmt"
+	"time"
+
 	"github.com/contentsquare/chproxy/clients"
 	"github.com/contentsquare/chproxy/config"
 	"github.com/go-redis/redis/v8"
-	"time"
 )
 
 // AsyncCache is a transactional cache allowing the results from concurrent queries.
@@ -79,6 +81,8 @@ func NewAsyncCache(cfg config.Cache) (*AsyncCache, error) {
 		redisClient, err = clients.NewRedisClient(cfg.Redis)
 		cache = newRedisCache(redisClient, cfg)
 		transaction = newRedisTransactionRegistry(redisClient, time.Duration(cfg.GraceTime))
+	default:
+		return nil, fmt.Errorf("unknown config mode")
 	}
 
 	if err != nil {

cache/async_cache_test.go

@@ -1,10 +1,13 @@
 package cache
 
 import (
-	"github.com/contentsquare/chproxy/config"
+	"log"
 	"os"
 	"testing"
 	"time"
+
+	"github.com/alicebob/miniredis/v2"
+	"github.com/contentsquare/chproxy/config"
 )
 
 const asyncTestDir = "./async-test-data"
@@ -134,3 +137,86 @@ func newAsyncTestCache(t *testing.T, graceTime time.Duration) *AsyncCache {
 	}
 	return asyncC
 }
+
+func TestAsyncCache_FilesystemCache_instantiation(t *testing.T) {
+	const testDirAsync = "./test-data-async"
+	fileSystemCfg := config.Cache{
+		Name: "test",
+		Mode: "file_system",
+		FileSystem: config.FileSystemCacheConfig{
+			Dir:     asyncTestDir,
+			MaxSize: 8192,
+		},
+		Expire: config.Duration(time.Minute),
+	}
+	if err := os.RemoveAll(testDirAsync); err != nil {
+		log.Fatalf("cannot remove %q: %s", testDirAsync, err)
+	}
+	_, err := NewAsyncCache(fileSystemCfg)
+	if err != nil {
+		t.Fatalf("could not instanciate filsystem async cache because of the following error: %s", err)
+	}
+}
+
+func TestAsyncCache_FilesystemCache_wrong_instantiation(t *testing.T) {
+	fileSystemCfg := config.Cache{
+		Name: "test",
+		Mode: "file_system",
+		FileSystem: config.FileSystemCacheConfig{
+			Dir:     "",
+			MaxSize: 8192,
+		},
+		Expire: config.Duration(time.Minute),
+	}
+	_, err := NewAsyncCache(fileSystemCfg)
+	if err == nil {
+		t.Fatalf("the instanciate of filsystem async cache should have crashed")
+	}
+}
+
+func TestAsyncCache_RedisCache_instantiation(t *testing.T) {
+	s := miniredis.RunT(t)
+	var redisCfg = config.Cache{
+		Name: "test",
+		Mode: "redis",
+		Redis: config.RedisCacheConfig{
+			Addresses: []string{s.Addr()},
+		},
+		Expire: config.Duration(cacheTTL),
+	}
+
+	_, err := NewAsyncCache(redisCfg)
+	if err != nil {
+		t.Fatalf("could not instanciate redis async cache because of the following error: %s", err)
+	}
+}
+
+func TestAsyncCache_RedisCache_wrong_instantiation(t *testing.T) {
+	var redisCfg = config.Cache{
+		Name: "test",
+		Mode: "redis",
+		Redis: config.RedisCacheConfig{
+			// fake address
+			Addresses: []string{"127.12.0.10:1024"},
+		},
+	}
+
+	_, err := NewAsyncCache(redisCfg)
+	if err == nil {
+		t.Fatalf("the redis instanciation should have crashed")
+	}
+}
+
+func TestAsyncCache_Unkown_instantiation(t *testing.T) {
+	var redisCfg = config.Cache{
+		Name:   "test",
+		Mode:   "Unkown Mode",
+		Redis:  config.RedisCacheConfig{},
+		Expire: config.Duration(cacheTTL),
+	}
+
+	_, err := NewAsyncCache(redisCfg)
+	if err == nil {
+		t.Fatalf("The instanciation should have crash")
+	}
+}

cache/filesystem_cache_test.go

@@ -47,9 +47,16 @@ func TestCacheClose(t *testing.T) {
 	}
 }
 
-func TestCacheAddGet(t *testing.T) {
+func TestFilesystemCacheAddGet(t *testing.T) {
 	c := newTestCache(t)
 	defer c.Close()
+	c1 := newTestCache(t)
+	defer c1.Close()
+	cacheAddGetHelper(t, c, c1)
+}
+
+// metatest used for both filesystem and redis Cache
+func cacheAddGetHelper(t *testing.T, c Cache, c1 Cache) {
 
 	for i := 0; i < 10; i++ {
 		key := &Key{
@@ -104,9 +111,6 @@ func TestCacheAddGet(t *testing.T) {
 	}
 
 	// Verify the cache may be re-opened.
-	c1 := newTestCache(t)
-	defer c1.Close()
-
 	for i := 0; i < 10; i++ {
 		key := &Key{
 			Query: []byte(fmt.Sprintf("SELECT %d", i)),
@@ -141,10 +145,14 @@ func TestCacheAddGet(t *testing.T) {
 	}
 }
 
-func TestCacheMiss(t *testing.T) {
+func TestFilesystemCacheMiss(t *testing.T) {
 	c := newTestCache(t)
 	defer c.Close()
+	cacheMissHelper(t, c)
+}
 
+// metatest used for both filesystem and redis Cache
+func cacheMissHelper(t *testing.T, c Cache) {
 	for i := 0; i < 10; i++ {
 		key := &Key{
 			Query: []byte(fmt.Sprintf("SELECT %d cache miss", i)),

cache/redis_cache_test.go

@@ -0,0 +1,79 @@
+package cache
+
+import (
+	"testing"
+	"time"
+
+	"github.com/alicebob/miniredis/v2"
+	"github.com/go-redis/redis/v8"
+
+	"github.com/contentsquare/chproxy/config"
+)
+
+const cacheTTL = time.Duration(10 * time.Second)
+
+var redisConf = config.Cache{
+	Name: "foobar",
+	Redis: config.RedisCacheConfig{
+		Addresses: []string{"http://localhost:8080"},
+	},
+	Expire: config.Duration(cacheTTL),
+}
+
+func TestCacheSize(t *testing.T) {
+	redisCache := generateRedisClientAndServer(t)
+	nbKeys := redisCache.nbOfKeys()
+	if nbKeys > 0 {
+		t.Fatalf("the cache should be empty")
+	}
+
+	cacheSize := redisCache.nbOfBytes()
+	if cacheSize > 0 {
+		t.Fatalf("the cache should be empty")
+	}
+
+	trw := &testResponseWriter{}
+	crw := NewBufferedResponseWriter(trw)
+	buffer := crw.Reader()
+
+	if _, err := redisCache.Put(buffer, ContentMetadata{}, &Key{Query: []byte("SELECT 1")}); err != nil {
+		t.Fatalf("failed to put it to cache: %s", err)
+	}
+	if _, err := redisCache.Put(buffer, ContentMetadata{}, &Key{Query: []byte("SELECT 2")}); err != nil {
+		t.Fatalf("failed to put it to cache: %s", err)
+	}
+	if _, err := redisCache.Put(buffer, ContentMetadata{}, &Key{Query: []byte("SELECT 2")}); err != nil {
+		t.Fatalf("failed to put it to cache: %s", err)
+	}
+
+	stats := redisCache.Stats()
+	if stats.Items != 2 {
+		t.Fatalf("cache should contain 2 items")
+	}
+	// because of the use of miniredis to simulate a real redis server
+	// we can't check stats.Size because miniredis doesn't handle the memory usage of redis
+}
+
+func generateRedisClientAndServer(t *testing.T) *redisCache {
+	s := miniredis.RunT(t)
+	redisClient := redis.NewUniversalClient(&redis.UniversalOptions{
+		Addrs: []string{s.Addr()},
+	})
+	redisCache := newRedisCache(redisClient, redisConf)
+	return redisCache
+}
+
+func TestRedisCacheAddGet(t *testing.T) {
+	c := generateRedisClientAndServer(t)
+	c1 := generateRedisClientAndServer(t)
+	defer func() {
+		c1.Close()
+		c.Close()
+	}()
+	cacheAddGetHelper(t, c, c1)
+}
+
+func TestRedisCacheMiss(t *testing.T) {
+	c := generateRedisClientAndServer(t)
+	cacheMissHelper(t, c)
+}

cache/transaction_registry_redis.go

@@ -2,9 +2,10 @@ package cache
 
 import (
 	"context"
+	"time"
+
 	"github.com/contentsquare/chproxy/log"
 	"github.com/go-redis/redis/v8"
-	"time"
 )
 
 // pendingTransactionVal

cache/transaction_registry_redis_test.go

@@ -0,0 +1,43 @@
+package cache
+
+import (
+	"testing"
+	"time"
+
+	"github.com/alicebob/miniredis/v2"
+	"github.com/go-redis/redis/v8"
+)
+
+func TestRedisTransaction(t *testing.T) {
+	s := miniredis.RunT(t)
+
+	redisClient := redis.NewUniversalClient(&redis.UniversalOptions{
+		Addrs: []string{s.Addr()},
+	})
+
+	graceTime := 10 * time.Second
+	key := &Key{
+		Query: []byte("SELECT pending entries"),
+	}
+
+	redisTransaction := newRedisTransactionRegistry(redisClient, graceTime)
+
+	if err := redisTransaction.Register(key); err != nil {
+		t.Fatalf("unexpected error: %s while registering new transaction", err)
+	}
+
+	isDone := redisTransaction.IsDone(key)
+	if isDone {
+		t.Fatalf("unexpected: transaction should be pending")
+	}
+
+	if err := redisTransaction.Unregister(key); err != nil {
+		t.Fatalf("unexpected error: %s while unregistering transaction", err)
+	}
+
+	isDone = redisTransaction.IsDone(key)
+	if !isDone {
+		t.Fatalf("unexpected: transaction should be done")
+	}
+
+}