Skip to content

Commit

Permalink
refactor(general): Cleaner error checking in retention tests (#3164)
Browse files Browse the repository at this point in the history 
* More robust error comparisons in retention tests

Update tests for retention to use `ErrorIs` checks instead of comparing
error messages.

* Use `require.NoError` in retention tests

Minor cleanup to reduce branches in code by using `require.NoError`
instead of if-blocks and `t.Fatal`.
  • Loading branch information
@ashmrtn
ashmrtn committed Jul 21, 2023
1 parent d976796 commit 2b73527
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 37 deletions.
4 changes: 3 additions & 1 deletion internal/blobtesting/object_locking_map.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"github.com/kopia/kopia/repo/blob"
)

var ErrBlobLocked = errors.New("cannot alter object before retention period expires")

type entry struct {
value []byte
mtime time.Time
Expand Down Expand Up @@ -60,7 +62,7 @@ func (s *objectLockingMap) getLatestForMutationLocked(id blob.ID) (*entry, error
}

if !e.retentionTime.IsZero() && e.retentionTime.After(s.timeNow()) {
return nil, errors.New("cannot alter object before retention period expires")
return nil, errors.WithStack(ErrBlobLocked)
}

return e, nil
Expand Down
22 changes: 8 additions & 14 deletions repo/blob/storage_extend_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/kopia/kopia/internal/blobtesting"
"github.com/kopia/kopia/internal/cache"
"github.com/kopia/kopia/internal/faketime"
"github.com/kopia/kopia/internal/repotesting"
Expand Down Expand Up @@ -46,9 +48,7 @@ func (s *formatSpecificTestSuite) TestExtendBlobRetention(t *testing.T) {
env.RepositoryWriter.Flush(ctx)

blobsBefore, err := blob.ListAllBlobs(ctx, env.RepositoryWriter.BlobStorage(), "")
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)

if got, want := len(blobsBefore), 4; got != want {
t.Fatalf("unexpected number of blobs after writing: %v", blobsBefore)
Expand All @@ -59,37 +59,31 @@ func (s *formatSpecificTestSuite) TestExtendBlobRetention(t *testing.T) {

// Verify that file is locked
_, err = st.TouchBlob(ctx, blobsBefore[lastBlobIdx].BlobID, time.Hour)
assert.EqualErrorf(t, err, "cannot alter object before retention period expires", "Altering locked object should fail")
assert.ErrorIs(t, err, blobtesting.ErrBlobLocked, "Altering locked object should fail")

ta.Advance(7 * 24 * time.Hour)

// Verify that file is unlocked
_, err = st.TouchBlob(ctx, blobsBefore[lastBlobIdx].BlobID, time.Hour)
if err != nil {
t.Fatalf("Altering expired object failed")
}
require.NoError(t, err, "Altering expired object failed")

// Relock blob
err = env.RepositoryWriter.BlobStorage().ExtendBlobRetention(ctx, blobsBefore[lastBlobIdx].BlobID, blob.ExtendOptions{
RetentionMode: blob.Governance,
RetentionPeriod: 2 * time.Hour,
})
if err != nil {
t.Fatalf("Extending Retention time failed, got err: %v", err)
}
require.NoErrorf(t, err, "Extending Retention time failed, got err: %v", err)

// Verify Lock period
ta.Advance(1 * time.Hour)

_, err = st.TouchBlob(ctx, blobsBefore[lastBlobIdx].BlobID, time.Hour)
assert.EqualErrorf(t, err, "cannot alter object before retention period expires", "Altering locked object should fail")
assert.ErrorIs(t, err, blobtesting.ErrBlobLocked, "Altering locked object should fail")

ta.Advance(2 * time.Hour)

_, err = st.TouchBlob(ctx, blobsBefore[lastBlobIdx].BlobID, time.Hour)
if err != nil {
t.Fatalf("Altering expired object failed")
}
require.NoError(t, err, "Altering expired object failed")
}

func (s *formatSpecificTestSuite) TestExtendBlobRetentionUnsupported(t *testing.T) {
Expand Down
37 changes: 15 additions & 22 deletions repo/maintenance/blob_retain_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/kopia/kopia/internal/blobtesting"
"github.com/kopia/kopia/internal/cache"
"github.com/kopia/kopia/internal/faketime"
"github.com/kopia/kopia/internal/repotesting"
Expand Down Expand Up @@ -45,9 +47,7 @@ func (s *formatSpecificTestSuite) TestExtendBlobRetentionTime(t *testing.T) {
env.RepositoryWriter.Flush(ctx)

blobsBefore, err := blob.ListAllBlobs(ctx, env.RepositoryWriter.BlobStorage(), "")
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)

if got, want := len(blobsBefore), 4; got != want {
t.Fatalf("unexpected number of blobs after writing: %v", blobsBefore)
Expand All @@ -58,17 +58,15 @@ func (s *formatSpecificTestSuite) TestExtendBlobRetentionTime(t *testing.T) {

ta.Advance(7 * 24 * time.Hour)

if _, err = st.TouchBlob(ctx, blobsBefore[lastBlobIdx].BlobID, time.Hour); err != nil {
t.Fatalf("Altering expired object failed")
}
_, err = st.TouchBlob(ctx, blobsBefore[lastBlobIdx].BlobID, time.Hour)
require.NoError(t, err, "Altering expired object failed")

// extend retention time of all blobs
if _, err = maintenance.ExtendBlobRetentionTime(ctx, env.RepositoryWriter, maintenance.ExtendBlobRetentionTimeOptions{}); err != nil {
t.Fatal(err)
}
_, err = maintenance.ExtendBlobRetentionTime(ctx, env.RepositoryWriter, maintenance.ExtendBlobRetentionTimeOptions{})
require.NoError(t, err)

_, err = st.TouchBlob(ctx, blobsBefore[lastBlobIdx].BlobID, time.Hour)
assert.EqualErrorf(t, err, "cannot alter object before retention period expires", "Altering locked object should fail")
assert.ErrorIs(t, err, blobtesting.ErrBlobLocked, "Altering locked object should fail")
}

func (s *formatSpecificTestSuite) TestExtendBlobRetentionTimeDisabled(t *testing.T) {
Expand All @@ -95,9 +93,7 @@ func (s *formatSpecificTestSuite) TestExtendBlobRetentionTimeDisabled(t *testing
env.RepositoryWriter.Flush(ctx)

blobsBefore, err := blob.ListAllBlobs(ctx, env.RepositoryWriter.BlobStorage(), "")
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)

if got, want := len(blobsBefore), 4; got != want {
t.Fatalf("unexpected number of blobs after writing: %v", blobsBefore)
Expand All @@ -108,16 +104,13 @@ func (s *formatSpecificTestSuite) TestExtendBlobRetentionTimeDisabled(t *testing

ta.Advance(7 * 24 * time.Hour)

if _, err = st.TouchBlob(ctx, blobsBefore[lastBlobIdx].BlobID, time.Hour); err != nil {
t.Fatalf("Altering expired object failed")
}
_, err = st.TouchBlob(ctx, blobsBefore[lastBlobIdx].BlobID, time.Hour)
require.NoError(t, err, "Altering expired object failed")

// extend retention time of all blobs
if _, err = maintenance.ExtendBlobRetentionTime(ctx, env.RepositoryWriter, maintenance.ExtendBlobRetentionTimeOptions{}); err != nil {
t.Fatal(err)
}
_, err = maintenance.ExtendBlobRetentionTime(ctx, env.RepositoryWriter, maintenance.ExtendBlobRetentionTimeOptions{})
require.NoError(t, err)

if _, err = st.TouchBlob(ctx, blobsBefore[lastBlobIdx].BlobID, time.Hour); err != nil {
t.Fatalf("Altering expired object failed")
}
_, err = st.TouchBlob(ctx, blobsBefore[lastBlobIdx].BlobID, time.Hour)
require.NoError(t, err, "Altering expired object failed")
}

0 comments on commit 2b73527

Please sign in to comment.