From bbbef44d8a4a733a7b95f1c4224aa3e072233910 Mon Sep 17 00:00:00 2001 From: Jarek Kowalski Date: Sat, 11 Dec 2021 23:27:42 -0800 Subject: [PATCH] More coverage improvements (#1577) * increased direct coverage for internal/cache * object: code coverage improvements for object writer --- internal/cache/cache_storage.go | 5 +- internal/cache/cache_storage_test.go | 28 +++++ internal/cache/persistent_lru_cache_test.go | 64 ++++++++-- repo/object/object_manager_test.go | 132 +++++++++++++++++--- repo/object/object_writer.go | 13 +- site/.go-version | 2 +- 6 files changed, 200 insertions(+), 44 deletions(-) create mode 100644 internal/cache/cache_storage_test.go diff --git a/internal/cache/cache_storage.go b/internal/cache/cache_storage.go index f07873b9e..260dc7b30 100644 --- a/internal/cache/cache_storage.go +++ b/internal/cache/cache_storage.go @@ -48,9 +48,6 @@ func NewStorageOrNil(ctx context.Context, cacheDir string, maxBytes int64, subdi DirectoryShards: []int{2}, }, }, false) - if err != nil { - return nil, errors.Wrap(err, "error initializing filesystem cache") - } - return fs.(Storage), nil + return fs.(Storage), errors.Wrap(err, "error initializing filesystem cache") } diff --git a/internal/cache/cache_storage_test.go b/internal/cache/cache_storage_test.go new file mode 100644 index 000000000..663c1c067 --- /dev/null +++ b/internal/cache/cache_storage_test.go @@ -0,0 +1,28 @@ +package cache_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/kopia/kopia/internal/cache" + "github.com/kopia/kopia/internal/testlogging" + "github.com/kopia/kopia/internal/testutil" +) + +func TestNewStorageOrNil(t *testing.T) { + ctx := testlogging.Context(t) + + // empty cache dir + st, err := cache.NewStorageOrNil(ctx, "", 1000, "subdir") + require.NoError(t, err) + require.Nil(t, st) + + // zero size + st, err = cache.NewStorageOrNil(ctx, testutil.TempDirectory(t), 0, "subdir") + require.NoError(t, err) + require.Nil(t, st) + + _, err = cache.NewStorageOrNil(ctx, "relative/path/to/cache/dir", 1000, "subdir") + require.Error(t, err) +} diff --git a/internal/cache/persistent_lru_cache_test.go b/internal/cache/persistent_lru_cache_test.go index 480ec57ed..2eff84ff0 100644 --- a/internal/cache/persistent_lru_cache_test.go +++ b/internal/cache/persistent_lru_cache_test.go @@ -23,18 +23,14 @@ func TestPersistentLRUCache(t *testing.T) { const maxSizeBytes = 1000 cs, err := cache.NewStorageOrNil(ctx, cacheDir, maxSizeBytes, "subdir") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) pc, err := cache.NewPersistentCache(ctx, "testing", cs, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{ MaxSizeBytes: maxSizeBytes, TouchThreshold: cache.DefaultTouchThreshold, SweepFrequency: cache.DefaultSweepFrequency, }) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) var tmp gather.WriteBuffer defer tmp.Close() @@ -79,9 +75,7 @@ func TestPersistentLRUCache(t *testing.T) { TouchThreshold: cache.DefaultTouchThreshold, SweepFrequency: cache.DefaultSweepFrequency, }) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) verifyCached(ctx, t, pc, "key1", nil) verifyCached(ctx, t, pc, "key2", someData) @@ -95,9 +89,7 @@ func TestPersistentLRUCache(t *testing.T) { TouchThreshold: cache.DefaultTouchThreshold, SweepFrequency: cache.DefaultSweepFrequency, }) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) var tmp2 gather.WriteBuffer defer tmp2.Close() @@ -115,6 +107,54 @@ func TestPersistentLRUCache(t *testing.T) { verifyCached(ctx, t, pc, "key2", nil) } +func TestPersistentLRUCacheNil(t *testing.T) { + ctx := testlogging.Context(t) + + var pc *cache.PersistentCache + + // no-op + pc.Close(ctx) + pc.Put(ctx, "key", gather.FromSlice([]byte{1, 2, 3})) + + var tmp gather.WriteBuffer + + require.False(t, pc.Get(ctx, "key", 0, -1, &tmp)) + + called := false + + dummyError := errors.Errorf("dummy error") + + require.ErrorIs(t, pc.GetOrLoad(ctx, "key", func(output *gather.WriteBuffer) error { + called = true + return dummyError + }, &tmp), dummyError) + + require.True(t, called) +} + +func TestPersistentLRUCache_Defaults(t *testing.T) { + cacheDir := testutil.TempDirectory(t) + ctx := testlogging.Context(t) + + const maxSizeBytes = 1000 + + cs, err := cache.NewStorageOrNil(ctx, cacheDir, maxSizeBytes, "subdir") + require.NoError(t, err) + + pc, err := cache.NewPersistentCache(ctx, "testing", cs, nil, cache.SweepSettings{ + MaxSizeBytes: maxSizeBytes, + }) + require.NoError(t, err) + + pc.Put(ctx, "key1", gather.FromSlice([]byte{1, 2, 3})) + + var tmp gather.WriteBuffer + defer tmp.Close() + + cs.GetBlob(ctx, "key1", 0, -1, &tmp) + require.Len(t, tmp.ToByteSlice(), 3) +} + func verifyCached(ctx context.Context, t *testing.T, pc *cache.PersistentCache, key string, want []byte) { t.Helper() diff --git a/repo/object/object_manager_test.go b/repo/object/object_manager_test.go index bc0650ff1..168e2ade2 100644 --- a/repo/object/object_manager_test.go +++ b/repo/object/object_manager_test.go @@ -29,11 +29,14 @@ "github.com/kopia/kopia/repo/splitter" ) +var errSomeError = errors.Errorf("some error") + type fakeContentManager struct { mu sync.Mutex data map[content.ID][]byte compresionIDs map[content.ID]compression.HeaderID supportsContentCompression bool + writeContentError error } func (f *fakeContentManager) GetContent(ctx context.Context, contentID content.ID) ([]byte, error) { @@ -48,6 +51,10 @@ func (f *fakeContentManager) GetContent(ctx context.Context, contentID content.I } func (f *fakeContentManager) WriteContent(ctx context.Context, data gather.Bytes, prefix content.ID, comp compression.HeaderID) (content.ID, error) { + if f.writeContentError != nil { + return "", f.writeContentError + } + h := sha256.New() data.WriteTo(h) contentID := prefix + content.ID(hex.EncodeToString(h.Sum(nil))) @@ -82,23 +89,25 @@ func (f *fakeContentManager) Flush(ctx context.Context) error { return nil } -func setupTest(t *testing.T, compressionHeaderID map[content.ID]compression.HeaderID) (map[content.ID][]byte, *Manager) { +func setupTest(t *testing.T, compressionHeaderID map[content.ID]compression.HeaderID) (map[content.ID][]byte, *fakeContentManager, *Manager) { t.Helper() data := map[content.ID][]byte{} - r, err := NewObjectManager(testlogging.Context(t), &fakeContentManager{ + fcm := &fakeContentManager{ data: data, supportsContentCompression: compressionHeaderID != nil, compresionIDs: compressionHeaderID, - }, Format{ + } + + r, err := NewObjectManager(testlogging.Context(t), fcm, Format{ Splitter: "FIXED-1M", }) if err != nil { t.Fatalf("can't create object manager: %v", err) } - return data, r + return data, fcm, r } func TestWriters(t *testing.T) { @@ -115,7 +124,7 @@ func TestWriters(t *testing.T) { } for _, c := range cases { - data, om := setupTest(t, nil) + data, _, om := setupTest(t, nil) writer := om.NewWriter(ctx, WriterOptions{}) @@ -154,7 +163,7 @@ func TestCompression_ContentCompressionEnabled(t *testing.T) { ctx := testlogging.Context(t) cmap := map[content.ID]compression.HeaderID{} - _, om := setupTest(t, cmap) + _, _, om := setupTest(t, cmap) w := om.NewWriter(ctx, WriterOptions{ Compressor: "gzip", @@ -173,7 +182,7 @@ func TestCompression_ContentCompressionDisabled(t *testing.T) { ctx := testlogging.Context(t) // this disables content compression - _, om := setupTest(t, nil) + _, _, om := setupTest(t, nil) w := om.NewWriter(ctx, WriterOptions{ Compressor: "gzip", @@ -189,7 +198,7 @@ func TestCompression_ContentCompressionDisabled(t *testing.T) { func TestWriterCompleteChunkInTwoWrites(t *testing.T) { ctx := testlogging.Context(t) - _, om := setupTest(t, nil) + _, _, om := setupTest(t, nil) b := make([]byte, 100) writer := om.NewWriter(ctx, WriterOptions{}) @@ -204,7 +213,7 @@ func TestWriterCompleteChunkInTwoWrites(t *testing.T) { func TestCheckpointing(t *testing.T) { ctx := testlogging.Context(t) - _, om := setupTest(t, nil) + _, _, om := setupTest(t, nil) writer := om.NewWriter(ctx, WriterOptions{}) @@ -390,7 +399,7 @@ func TestIndirection(t *testing.T) { } for _, c := range cases { - data, om := setupTest(t, nil) + data, _, om := setupTest(t, nil) contentBytes := make([]byte, c.dataLength) @@ -442,7 +451,7 @@ func TestHMAC(t *testing.T) { ctx := testlogging.Context(t) c := bytes.Repeat([]byte{0xcd}, 50) - _, om := setupTest(t, nil) + _, _, om := setupTest(t, nil) w := om.NewWriter(ctx, WriterOptions{}) w.Write(c) @@ -456,7 +465,7 @@ func TestHMAC(t *testing.T) { // nolint:gocyclo func TestConcatenate(t *testing.T) { ctx := testlogging.Context(t) - _, om := setupTest(t, nil) + _, _, om := setupTest(t, nil) phrase := []byte("hello world\n") phraseLength := len(phrase) @@ -591,7 +600,7 @@ func mustWriteObject(t *testing.T, om *Manager, data []byte, compressor compress func TestReader(t *testing.T) { ctx := testlogging.Context(t) - data, om := setupTest(t, nil) + data, _, om := setupTest(t, nil) storedPayload := []byte("foo\nbar") data["a76999788386641a3ec798554f1fe7e6"] = storedPayload @@ -631,7 +640,7 @@ func TestReader(t *testing.T) { func TestReaderStoredBlockNotFound(t *testing.T) { ctx := testlogging.Context(t) - _, om := setupTest(t, nil) + _, _, om := setupTest(t, nil) objectID, err := ParseID("deadbeef") if err != nil { @@ -652,7 +661,7 @@ func TestEndToEndReadAndSeek(t *testing.T) { t.Parallel() ctx := testlogging.Context(t) - _, om := setupTest(t, nil) + _, _, om := setupTest(t, nil) for _, size := range []int{1, 199, 200, 201, 9999, 512434, 5012434} { // Create some random data sample of the specified size. @@ -705,7 +714,7 @@ func TestEndToEndReadAndSeekWithCompression(t *testing.T) { totalBytesWritten := 0 - data, om := setupTest(t, nil) + data, _, om := setupTest(t, nil) for _, size := range sizes { var inputData []byte @@ -804,7 +813,7 @@ func verify(ctx context.Context, t *testing.T, cr contentReader, objectID ID, ex // nolint:gocyclo func TestSeek(t *testing.T) { ctx := testlogging.Context(t) - _, om := setupTest(t, nil) + _, _, om := setupTest(t, nil) for _, size := range []int{0, 1, 500000, 15000000} { randomData := make([]byte, size) @@ -851,3 +860,92 @@ func TestSeek(t *testing.T) { } } } + +func TestWriterFlushFailure_OnWrite(t *testing.T) { + _, fcm, om := setupTest(t, nil) + + ctx := testlogging.Context(t) + w := om.NewWriter(ctx, WriterOptions{}) + + fcm.writeContentError = errSomeError + + n, err := w.Write(bytes.Repeat([]byte{1, 2, 3, 4}, 1e6)) + require.ErrorIs(t, err, errSomeError) + require.Equal(t, n, 0) +} + +func TestWriterFlushFailure_OnFlush(t *testing.T) { + _, fcm, om := setupTest(t, nil) + + ctx := testlogging.Context(t) + w := om.NewWriter(ctx, WriterOptions{}) + + n, err := w.Write(bytes.Repeat([]byte{1, 2, 3, 4}, 1e6)) + require.NoError(t, err, errSomeError) + require.Equal(t, n, 4000000) + + fcm.writeContentError = errSomeError + + _, err = w.Result() + require.ErrorIs(t, err, errSomeError) +} + +func TestWriterFlushFailure_OnCheckpoint(t *testing.T) { + _, fcm, om := setupTest(t, nil) + + ctx := testlogging.Context(t) + w := om.NewWriter(ctx, WriterOptions{}) + + w.Write(bytes.Repeat([]byte{1, 2, 3, 4}, 1e6)) + + fcm.writeContentError = errSomeError + _, err := w.Checkpoint() + + require.ErrorIs(t, err, errSomeError) +} + +func TestWriterFlushFailure_OnAsyncWrite(t *testing.T) { + _, fcm, om := setupTest(t, nil) + + ctx := testlogging.Context(t) + w := om.NewWriter(ctx, WriterOptions{ + AsyncWrites: 1, + }) + + fcm.writeContentError = errSomeError + + n, err := w.Write(bytes.Repeat([]byte{1, 2, 3, 4}, 1e6)) + require.NoError(t, err, errSomeError) + require.Equal(t, n, 4000000) + + _, err = w.Result() + require.ErrorIs(t, err, errSomeError) +} + +type faultyCompressor struct{} + +func (faultyCompressor) Compress(output io.Writer, input io.Reader) error { + return errSomeError +} + +func (faultyCompressor) Decompress(output io.Writer, input io.Reader, withHeader bool) error { + return nil +} + +func (faultyCompressor) HeaderID() compression.HeaderID { + return 0 +} + +func TestWriterFailure_OnCompression(t *testing.T) { + _, _, om := setupTest(t, nil) + + compression.RegisterCompressor("faulty", &faultyCompressor{}) + + ctx := testlogging.Context(t) + w := om.NewWriter(ctx, WriterOptions{ + Compressor: "faulty", + }) + + _, err := w.Write(bytes.Repeat([]byte{1, 2, 3, 4}, 1e6)) + require.Error(t, err, errSomeError) +} diff --git a/repo/object/object_writer.go b/repo/object/object_writer.go index 7af1963ea..b676aca14 100644 --- a/repo/object/object_writer.go +++ b/repo/object/object_writer.go @@ -117,17 +117,12 @@ func (w *objectWriter) Write(data []byte) (n int, err error) { n := w.splitter.NextSplitPoint(data) if n < 0 { // no split points in the buffer - if _, err := w.buffer.Write(data); err != nil { - return 0, errors.Wrap(err, "error writing to buffer") - } - + w.buffer.Append(data) break } // found a split point after `n` bytes, write first n bytes then flush and repeat with the remainder. - if _, err := w.buffer.Write(data[0:n]); err != nil { - return 0, errors.Wrap(err, "error writing to buffer") - } + w.buffer.Append(data[0:n]) if err := w.flushBuffer(); err != nil { return 0, err @@ -162,9 +157,7 @@ func (w *objectWriter) flushBuffer() error { w.asyncWritesWG.Add(1) asyncBuf := gather.NewWriteBuffer() - if _, err := w.buffer.Bytes().WriteTo(asyncBuf); err != nil { - return errors.Wrap(err, "error copying buffer for async copy") - } + w.buffer.Bytes().WriteTo(asyncBuf) // nolint:errcheck go func() { defer func() { diff --git a/site/.go-version b/site/.go-version index e71519696..b48f32260 100644 --- a/site/.go-version +++ b/site/.go-version @@ -1 +1 @@ -1.16 +1.17