Skip to content

Commit

Permalink
[FAB-9492] Fix flaky discovery cache test
Browse files Browse the repository at this point in the history
The discovery TestCachePurgeCache sometimes fails because of wrong
assumptions of partial cache purging that don't always hold:

When the test reaches line 184, the cache (capacity of 4) might have:
identity2
identity3
identity4
identity5

and in lines 185-195 we load the cache with:
identity1, identity2, identity3, identity4

Therefore, the following scenario might happen:

- identity1 is being put, so identity2 is evicted
- identity2 is being put, so identity3 is evicted
- identity3 is being put, so identity4 is evicted
- identity4 is being put, so identity5 is evicted

We got 4 evictions so far, in contrast to the (false) assumption
of having between 1 and 2.

This test changes the identities that are loaded initiality in the warmup
phase to 4 instead of 5,

and in the next step - we start with identity5 to be sure we evicted
some identity, and then only put 2 identities and we just check that
the whole cache wasn't purged.

Change-Id: If28ae7db4adbc6319d95a3d41e954d8b420c7876
Signed-off-by: yacovm <yacovm@il.ibm.com>
  • Loading branch information
yacovm committed Apr 13, 2018
1 parent e51bd92 commit 5c62a6c
Showing 1 changed file with 4 additions and 5 deletions.
9 changes: 4 additions & 5 deletions discovery/authcache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,8 @@ func TestCachePurgeCache(t *testing.T) {
cache := newAuthCache(as, authCacheConfig{maxCacheSize: 4, purgeRetentionRatio: 0.75})
as.On("ConfigSequence", "mychannel").Return(uint64(0))

// Warm up the cache - attempt to place 5 identities but the cache size is 4 so among the first 4 identities,
// only 3 of them should be cached
for _, id := range []string{"identity1", "identity2", "identity3", "identity4", "identity5"} {
// Warm up the cache - attempt to place 4 identities to fill it up
for _, id := range []string{"identity1", "identity2", "identity3", "identity4"} {
sd := common.SignedData{
Data: []byte{1, 2, 3},
Identity: []byte(id),
Expand All @@ -185,7 +184,7 @@ func TestCachePurgeCache(t *testing.T) {

// Now, ensure that at least 1 of the identities was evicted from the cache, but not all
var evicted int
for _, id := range []string{"identity1", "identity2", "identity3", "identity4"} {
for _, id := range []string{"identity5", "identity1", "identity2"} {
sd := common.SignedData{
Data: []byte{1, 2, 3},
Identity: []byte(id),
Expand All @@ -197,7 +196,7 @@ func TestCachePurgeCache(t *testing.T) {
evicted++
}
}
assert.True(t, evicted > 0 && evicted < 3)
assert.True(t, evicted > 0 && evicted < 4, "evicted: %d, but expected between 1 and 3 evictions", evicted)
}

func TestCacheConcurrentConfigUpdate(t *testing.T) {
Expand Down

0 comments on commit 5c62a6c

Please sign in to comment.