diff options
-rw-r--r-- | internal/vfs/zip/vfs_test.go | 184 |
1 files changed, 106 insertions, 78 deletions
diff --git a/internal/vfs/zip/vfs_test.go b/internal/vfs/zip/vfs_test.go index 90023144..8a5e77a8 100644 --- a/internal/vfs/zip/vfs_test.go +++ b/internal/vfs/zip/vfs_test.go @@ -101,97 +101,125 @@ func TestVFSFindOrOpenArchiveConcurrentAccess(t *testing.T) { }, time.Second, time.Nanosecond) } -func TestVFSArchiveCacheEvict(t *testing.T) { - testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public.zip", nil) - defer cleanup() - - path := testServerURL + "/public.zip" - - vfs := New( - WithCacheExpirationInterval(time.Nanosecond), - ).(*zipVFS) - - archivesMetric := metrics.ZipCachedEntries.WithLabelValues("archive") - archivesCount := testutil.ToFloat64(archivesMetric) - - // create a new archive and increase counters - archive, err := vfs.Root(context.Background(), path) - require.NoError(t, err) - require.NotNil(t, archive) - - // wait for archive to expire - time.Sleep(time.Nanosecond) - - // a new object is created - archive2, err := vfs.Root(context.Background(), path) - require.NoError(t, err) - require.NotNil(t, archive2) - require.NotEqual(t, archive, archive2, "a different archive is returned") - - archivesCountEnd := testutil.ToFloat64(archivesMetric) - require.Equal(t, float64(1), archivesCountEnd-archivesCount, "all expired archives are evicted") -} - func TestVFSFindOrOpenArchiveRefresh(t *testing.T) { testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public.zip", nil) defer cleanup() - vfs := New( - // using defaultCacheExpirationInterval ensures refresh would be called - WithCacheRefreshInterval(defaultCacheExpirationInterval), - ).(*zipVFS) + // It should be large enough to not have flaky executions + const expiryInterval = 10 * time.Millisecond tests := map[string]struct { - path string - expectOpenError bool - expectEqualExpiry bool + path string + expirationInterval time.Duration + refreshInterval time.Duration + + expectNewArchive bool + expectOpenError bool + expectArchiveRefreshed bool }{ - "successful_refresh": {path: "/public.zip", expectOpenError: false, expectEqualExpiry: false}, - "errored_archive": {path: "/unknown.zip", expectOpenError: true, expectEqualExpiry: true}, + "after cache expiry of successful open a new archive is returned": { + path: "/public.zip", + expirationInterval: expiryInterval, + expectNewArchive: true, + expectOpenError: false, + }, + "after cache expiry of errored open a new archive is returned": { + path: "/unknown.zip", + expirationInterval: expiryInterval, + expectNewArchive: true, + expectOpenError: true, + }, + "subsequent open during refresh interval does refresh archive": { + path: "/public.zip", + expirationInterval: time.Second, + refreshInterval: time.Second, // refresh always + expectNewArchive: false, + expectOpenError: false, + expectArchiveRefreshed: true, + }, + "subsequent open before refresh interval does not refresh archive": { + path: "/public.zip", + expirationInterval: time.Second, + refreshInterval: time.Millisecond, // very short interval should not refresh + expectNewArchive: false, + expectOpenError: false, + expectArchiveRefreshed: false, + }, + "subsequent open of errored archive during refresh interval does not refresh": { + path: "/unknown.zip", + expirationInterval: time.Second, + refreshInterval: time.Second, // refresh always (if not error) + expectNewArchive: false, + expectOpenError: true, + expectArchiveRefreshed: false, + }, } for name, test := range tests { t.Run(name, func(t *testing.T) { - archivesMetric := metrics.ZipCachedEntries.WithLabelValues("archive") - archivesCount := testutil.ToFloat64(archivesMetric) - - path := testServerURL + test.path - - // create a new archive and increase counters - archive1, err1 := vfs.findOrOpenArchive(context.Background(), path) - if test.expectOpenError { - require.Error(t, err1) - require.Nil(t, archive1) - } else { - require.NoError(t, err1) - } - - item1, exp1, found := vfs.cache.GetWithExpiration(path) - require.True(t, found) - - // should return errored archive - archive2, err2 := vfs.findOrOpenArchive(context.Background(), path) - if test.expectOpenError { - require.Error(t, err2) - require.Nil(t, archive2) - } else { - require.NoError(t, err2) - } + withExpectedArchiveCount(t, 1, func(t *testing.T) { + vfs := New( + WithCacheExpirationInterval(test.expirationInterval), + WithCacheRefreshInterval(test.refreshInterval), + ).(*zipVFS) + + path := testServerURL + test.path + + // create a new archive and increase counters + archive1, err1 := vfs.findOrOpenArchive(context.Background(), path) + if test.expectOpenError { + require.Error(t, err1) + require.Nil(t, archive1) + } else { + require.NoError(t, err1) + } + + item1, exp1, found := vfs.cache.GetWithExpiration(path) + require.True(t, found) + + // give some time to for timeouts to fire + time.Sleep(expiryInterval) + + if test.expectNewArchive { + // should return a new archive + archive2, err2 := vfs.findOrOpenArchive(context.Background(), path) + if test.expectOpenError { + require.Error(t, err2) + require.Nil(t, archive2) + } else { + require.NoError(t, err2) + require.NotEqual(t, archive1, archive2, "a new archive should be returned") + } + return + } + + // should return exactly the same archive + archive2, err2 := vfs.findOrOpenArchive(context.Background(), path) + require.Equal(t, archive1, archive2, "same archive is returned") + require.Equal(t, err1, err2, "same error for the same archive") + + item2, exp2, found := vfs.cache.GetWithExpiration(path) + require.True(t, found) + require.Equal(t, item1, item2, "same item is returned") + + if test.expectArchiveRefreshed { + require.Greater(t, exp2.UnixNano(), exp1.UnixNano(), "archive should be refreshed") + } else { + require.Equal(t, exp1.UnixNano(), exp2.UnixNano(), "archive has not been refreshed") + } + }) + }) + } +} - require.Equal(t, err1, err2, "same error for the same archive") +func withExpectedArchiveCount(t *testing.T, archiveCount int, fn func(t *testing.T)) { + t.Helper() - item2, exp2, found := vfs.cache.GetWithExpiration(path) - require.True(t, found) - require.Equal(t, item1, item2, "same item is returned") + archivesMetric := metrics.ZipCachedEntries.WithLabelValues("archive") + archivesCount := testutil.ToFloat64(archivesMetric) - if test.expectEqualExpiry { - require.True(t, exp1.Equal(exp2), "archive has not been refreshed") - } else { - require.True(t, exp2.After(exp1), "exp2 should be after exp1 due to refresh") - } + fn(t) - archivesCountEnd := testutil.ToFloat64(archivesMetric) - require.Equal(t, float64(1), archivesCountEnd-archivesCount, "only one archive cached") - }) - } + archivesCountEnd := testutil.ToFloat64(archivesMetric) + require.Equal(t, float64(archiveCount), archivesCountEnd-archivesCount, "exact number of archives is cached") } |