Remove dependency on external cache package #823
Conversation
server/cache.go
Outdated
| } | ||
|
|
||
| // Cache interface | ||
| type CacheInterface interface { |
There was a problem hiding this comment.
suggestion: rename to cache
It seems unnecessary to have interface in the name. cache is the interface and SimpleCache the implementation.
server/cache.go
Outdated
| } | ||
|
|
||
| // retrieveFromCache safely retrieves a value from a named cache | ||
| func retrieveFromCache( |
There was a problem hiding this comment.
suggestion: make the retrieval generic
Given the design of the simple cache it looks like we need call retrieveFromCache then do a type assertion when ever we want to get something. We could make this a generic retrival.
server/cache_test.go
Outdated
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| type testStruct struct { |
There was a problem hiding this comment.
suggestion: make this local the test its used in
server/cache_test.go
Outdated
| B string | ||
| } | ||
|
|
||
| func TestSetGetDelete(t *testing.T) { |
There was a problem hiding this comment.
question: is there a reason we are testing multiple things in the same test opposed to just the method? For exampe,
func TestNewSimpleCache_SetDefault(t *testing.T) {
var tests = []struct {
name string
given given
exp expected
}{
{
name: "no_expiration",
},
{
name: "default_expiration",
},
{
name: "set_multiple_types",
},
...
server/cache_test.go
Outdated
| assert.False(t, found, "Expected key %q to be deleted", key) | ||
| } | ||
|
|
||
| func TestExpiration(t *testing.T) { |
There was a problem hiding this comment.
suggestion: make this part of the SetDefault tests
the tests don really align with the prod code
2 participants
No description provided.