From cf041f8129955f6f03e25353fcbc7f0151bd3948 Mon Sep 17 00:00:00 2001 From: Barak Amar Date: Sun, 6 Dec 2020 15:50:21 +0200 Subject: [PATCH] check and fix the use of errors.As (#1004) --- .../mvcc/cataloger_create_repository_test.go | 25 ++++++++----------- catalog/mvcc/cataloger_delete_branch_test.go | 2 +- catalog/mvcc/cataloger_delete_entry_test.go | 8 +++--- catalog/mvcc/cataloger_merge.go | 2 +- catalog/mvcc/cataloger_reset_entry_test.go | 6 ++--- catalog/mvcc/cataloger_test.go | 2 +- 6 files changed, 20 insertions(+), 25 deletions(-) diff --git a/catalog/mvcc/cataloger_create_repository_test.go b/catalog/mvcc/cataloger_create_repository_test.go index 5c6811aab0b..a707a6157e1 100644 --- a/catalog/mvcc/cataloger_create_repository_test.go +++ b/catalog/mvcc/cataloger_create_repository_test.go @@ -22,45 +22,40 @@ func TestCataloger_CreateRepo(t *testing.T) { tests := []struct { name string args args - wantErr bool - asErr error + wantErr error }{ { name: "basic", args: args{name: "repo1", storage: "s3://bucket1", branch: "master"}, - wantErr: false, - asErr: nil, + wantErr: nil, }, { name: "unknown branch", args: args{name: "repo3", storage: "s3://bucket3", branch: ""}, - wantErr: true, - asErr: catalog.ErrInvalidValue, + wantErr: catalog.ErrInvalidValue, }, { name: "missing repo", args: args{name: "", storage: "s3://bucket1", branch: "master"}, - wantErr: true, - asErr: catalog.ErrInvalidValue, + wantErr: catalog.ErrInvalidValue, }, { name: "missing storage", args: args{name: "repo1", storage: "", branch: "master"}, - wantErr: true, - asErr: catalog.ErrInvalidValue, + wantErr: catalog.ErrInvalidValue, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { repo, err := c.CreateRepository(ctx, tt.args.name, tt.args.storage, tt.args.branch) - if (err != nil) != tt.wantErr { + if !errors.Is(err, tt.wantErr) { t.Fatalf("CreateRepository() error = %v, wantErr %v", err, tt.wantErr) } - if err != nil && tt.asErr != nil && !errors.As(err, &tt.asErr) { - t.Fatalf("CreateRepository() error = %v, expected as %v", err, tt.asErr) - } - if err != nil { + if tt.wantErr != nil { + if repo != nil { + t.Fatalf("CreateRepository() repo = %v, expected nil on error", repo) + } return } if repo.Name != tt.args.name { diff --git a/catalog/mvcc/cataloger_delete_branch_test.go b/catalog/mvcc/cataloger_delete_branch_test.go index 38498b199fd..3db69fe6b28 100644 --- a/catalog/mvcc/cataloger_delete_branch_test.go +++ b/catalog/mvcc/cataloger_delete_branch_test.go @@ -84,7 +84,7 @@ func TestCataloger_DeleteBranch(t *testing.T) { return } _, err = c.GetBranchReference(ctx, tt.args.repository, tt.args.branch) - if !errors.As(err, &db.ErrNotFound) { + if !errors.Is(err, db.ErrNotFound) { t.Errorf("Branch should not be found after delete, got err=%s", err) return } diff --git a/catalog/mvcc/cataloger_delete_entry_test.go b/catalog/mvcc/cataloger_delete_entry_test.go index 23a111c9698..8e0c47a5806 100644 --- a/catalog/mvcc/cataloger_delete_entry_test.go +++ b/catalog/mvcc/cataloger_delete_entry_test.go @@ -17,7 +17,7 @@ func TestCataloger_DeleteEntry(t *testing.T) { t.Run("delete file not exists", func(t *testing.T) { err := c.DeleteEntry(ctx, repository, "master", "/file1") wantErr := catalog.ErrEntryNotFound - if !errors.As(err, &wantErr) { + if !errors.Is(err, wantErr) { t.Errorf("DeleteEntry() error = %s, want = %s", err, wantErr) } }) @@ -98,13 +98,13 @@ func TestCataloger_DeleteEntry(t *testing.T) { func testDeleteEntryExpectNotFound(t *testing.T, ctx context.Context, c catalog.Cataloger, repository, branch string, path string) { _, err := c.GetEntry(ctx, repository, MakeReference(branch, UncommittedID), path, catalog.GetEntryParams{}) wantErr := db.ErrNotFound - if !errors.As(err, &wantErr) { + if !errors.Is(err, wantErr) { t.Fatalf("DeleteEntry() get entry err = %s, want = %s", err, wantErr) } // expect a second delete to fail on entry not found err = c.DeleteEntry(ctx, repository, branch, path) wantErr = catalog.ErrEntryNotFound - if !errors.As(err, &wantErr) { + if !errors.Is(err, wantErr) { t.Fatalf("DeleteEntry() error = %s, want = %s", err, wantErr) } } @@ -116,7 +116,7 @@ func testDeleteEntryCommitAndExpectNotFound(t *testing.T, ctx context.Context, c } _, err = c.GetEntry(ctx, repository, branch+CommittedSuffix, path, catalog.GetEntryParams{}) wantErr := db.ErrNotFound - if !errors.As(err, &wantErr) { + if !errors.Is(err, wantErr) { t.Fatalf("DeleteEntry() get entry err = %s, want = %s", err, wantErr) } } diff --git a/catalog/mvcc/cataloger_merge.go b/catalog/mvcc/cataloger_merge.go index 83f88990c58..cfb4054041f 100644 --- a/catalog/mvcc/cataloger_merge.go +++ b/catalog/mvcc/cataloger_merge.go @@ -277,7 +277,7 @@ func insertMergeCommit(tx db.Tx, relation RelationType, leftID int64, rightID in var parentLastLineage []int64 err = tx.Get(&parentLastLineage, `SELECT DISTINCT ON (branch_id) lineage_commits FROM catalog_commits WHERE branch_id = $1 AND merge_type = 'from_parent' ORDER BY branch_id,commit_id DESC`, leftID) - if err != nil && !errors.As(err, &db.ErrNotFound) { + if err != nil && !errors.Is(err, db.ErrNotFound) { return err } childNewLineage = append([]int64{int64(leftLastCommitID)}, parentLastLineage...) diff --git a/catalog/mvcc/cataloger_reset_entry_test.go b/catalog/mvcc/cataloger_reset_entry_test.go index 5e7dd6fc86f..0bf0fd56c7d 100644 --- a/catalog/mvcc/cataloger_reset_entry_test.go +++ b/catalog/mvcc/cataloger_reset_entry_test.go @@ -130,7 +130,7 @@ func TestCataloger_ResetEntry_NewToNone(t *testing.T) { } _, err := c.GetEntry(ctx, repository, MakeReference("master", UncommittedID), "/file1", catalog.GetEntryParams{}) expectedErr := db.ErrNotFound - if !errors.As(err, &expectedErr) { + if !errors.Is(err, expectedErr) { t.Fatalf("ResetEntry expecting the file to be gone with %s, got = %s", expectedErr, err) } } @@ -194,7 +194,7 @@ func TestCataloger_ResetEntry_Committed(t *testing.T) { } err := c.ResetEntry(ctx, repository, "master", "/file1") expectedErr := db.ErrNotFound - if !errors.As(err, &expectedErr) { + if !errors.Is(err, expectedErr) { t.Fatal("ResetEntry expected not to find file in case nothing to reset: ", err) } } @@ -222,7 +222,7 @@ func TestCataloger_ResetEntry_CommittedParentBranch(t *testing.T) { } err = c.ResetEntry(ctx, repository, "b1", "/file1") expectedErr := db.ErrNotFound - if !errors.As(err, &expectedErr) { + if !errors.Is(err, expectedErr) { t.Fatal("ResetEntry expected not to find file in case nothing to reset:", err) } } diff --git a/catalog/mvcc/cataloger_test.go b/catalog/mvcc/cataloger_test.go index 2b306c31f33..93e31e0406f 100644 --- a/catalog/mvcc/cataloger_test.go +++ b/catalog/mvcc/cataloger_test.go @@ -99,7 +99,7 @@ func testVerifyEntries(t testing.TB, ctx context.Context, c catalog.Cataloger, r for _, entry := range entries { ent, err := c.GetEntry(ctx, repository, reference, entry.Path, catalog.GetEntryParams{}) if entry.Deleted { - if !errors.As(err, &db.ErrNotFound) { + if !errors.Is(err, db.ErrNotFound) { t.Fatalf("Get entry '%s', err = %s, expected not found", entry.Path, err) } } else {