Fix TestSimpleCache_Multiple

[apocelipes]

The problem is triggered randomly, so I wrote a script to run this test 10,000 times in a row, and eventually found that it failed 6 times. And I also found out why it failed.

The script is:

for i in `seq 1 10000`
do
    go test -run TestSimpleCache_Multiple | grep -A5 -B5 FAIL
done

Think about a cache after adding elements, it could has these keys: ["10", "499", "100", ... , "467"] (This is possible because we randomly delete elements). Then we simpleCache.Add("10", []byte{}), Add will call evictItems because the cache already has 10 items, so one of them will be evicted. Because we randomly delete elements, the key "499" was evicted, then the cache only has 9 items left. Finally, Add uses items["10"] = value to add items, this does not change the number of elements in the map. Now the number of elements in cache is not as expected.

To test this hypothesis we can make a small modification to the test and then we can see that the two errors occur at the same time, so the hypothesis is correct:

func TestSimpleCache_Multiple(t *testing.T) {
    simpleCache := NewSimpleCache(10, 60)

    for i := 0; i < 500; i++ {
        simpleCache.Add(fmt.Sprintf("%d", i), []byte{})
    }

    if _, ok := simpleCache.items["10"]; ok {
        t.Error(`got key "10"`)
    }

    simpleCache.Add("10", []byte{})

    if len(simpleCache.items) != 10 {
        t.Errorf("expected 10 items got %v, %#v", len(simpleCache.items), simpleCache.items)
    }
}

Changing the newly added key to a value that will never appear in cache can fix this test, and in the actual code we basically don't re-add a key that already exists.

[boyter]

I love this bot that tells me how large the change is.

Thanks for the fix. Merged.