From 2ea3850f9298c6a8fb2687402e0314be95849e1e Mon Sep 17 00:00:00 2001 From: Barak Amar Date: Sun, 3 Jan 2021 09:16:32 +0200 Subject: [PATCH] Fix minor lint issues (#1128) * minor lint issues found * new error * Update cloud/aws/s3inventory/reader_test.go Co-authored-by: arielshaqed * compare commit log * fix test to verify entry not found Co-authored-by: arielshaqed --- Makefile | 2 +- catalog/mvcc/cataloger_delete_entry_test.go | 17 ++++-- catalog/mvcc/cataloger_merge_test.go | 3 +- catalog/rocks/entry_catalog_test.go | 2 - cloud/aws/s3inventory/reader_test.go | 3 + export/tasks_generator_test.go | 9 --- gateway/multiparts/tracker_test.go | 3 +- graveler/ref/resolver_test.go | 61 ++++++++++++--------- graveler/sstable/batch_test.go | 6 +- graveler/sstable/writer_test.go | 2 + parade/parade_test.go | 6 -- pyramid/tier_fs_test.go | 6 +- retention/map_error_test.go | 10 ++-- 13 files changed, 65 insertions(+), 65 deletions(-) diff --git a/Makefile b/Makefile index f9a5da276ce..0e37c5b284e 100644 --- a/Makefile +++ b/Makefile @@ -80,7 +80,7 @@ go-install: go-mod-download ## Install dependencies gen-api: go-install del-gen-api ## Run the go-swagger code generator - $(GOGENERATE) ./api/... + $(GOGENERATE) ./api del-gen-api: @rm -rf $(API_BUILD_DIR) diff --git a/catalog/mvcc/cataloger_delete_entry_test.go b/catalog/mvcc/cataloger_delete_entry_test.go index 757485d71be..42f92422c48 100644 --- a/catalog/mvcc/cataloger_delete_entry_test.go +++ b/catalog/mvcc/cataloger_delete_entry_test.go @@ -5,10 +5,9 @@ import ( "errors" "testing" - "github.com/treeverse/lakefs/testutil" - "github.com/treeverse/lakefs/catalog" "github.com/treeverse/lakefs/db" + "github.com/treeverse/lakefs/testutil" ) func TestCataloger_DeleteEntry(t *testing.T) { @@ -140,8 +139,9 @@ func TestCataloger_DeleteEntryAndCheckItRemainsInCommits(t *testing.T) { if err != nil { t.Fatal("Failed to commit before expect not found:", err) } - entry, err := c.GetEntry(ctx, repository, prevCommit.Reference, "/file2", catalog.GetEntryParams{}) - _ = entry + _, err = c.GetEntry(ctx, repository, prevCommit.Reference, "/file2", catalog.GetEntryParams{}) + testutil.Must(t, err) + err = c.DeleteEntry(ctx, repository, "master", "/file2") if err != nil { t.Fatal("delete failed: ", err) @@ -150,8 +150,13 @@ func TestCataloger_DeleteEntryAndCheckItRemainsInCommits(t *testing.T) { if err != nil { t.Fatal("Failed to commit after delete:", err) } - entry, err = c.GetEntry(ctx, repository, nextCommit.Reference, "/file2", catalog.GetEntryParams{}) - entry, err = c.GetEntry(ctx, repository, prevCommit.Reference, "/file2", catalog.GetEntryParams{}) + _, err = c.GetEntry(ctx, repository, nextCommit.Reference, "/file2", catalog.GetEntryParams{}) + if !errors.Is(err, catalog.ErrEntryNotFound) { + t.Fatalf("get deleted entry should not be found. err=%s", err) + } + _, err = c.GetEntry(ctx, repository, prevCommit.Reference, "/file2", catalog.GetEntryParams{}) + testutil.Must(t, err) + list, _, err := c.ListEntries(ctx, repository, nextCommit.Reference, "", "", "", 1000) if len(list) != 0 { t.Fatal("list entries returned deleted object") diff --git a/catalog/mvcc/cataloger_merge_test.go b/catalog/mvcc/cataloger_merge_test.go index c5f19fbff36..862729f3839 100644 --- a/catalog/mvcc/cataloger_merge_test.go +++ b/catalog/mvcc/cataloger_merge_test.go @@ -77,7 +77,7 @@ func TestCataloger_Merge_FromParentNoChangesInChild(t *testing.T) { t.Fatal("Merge Summary", diff) } // merge again - nothing should happen - res, err = c.Merge(ctx, repository, "master", "branch1", "tester", "", nil) + _, err = c.Merge(ctx, repository, "master", "branch1", "tester", "", nil) if err != catalog.ErrNoDifferenceWasFound { t.Fatal("Merge() expected ErrNoDifferenceWasFound, got:", err) } @@ -1087,6 +1087,7 @@ func TestCataloger_MergeOverDeletedEntries(t *testing.T) { // create and commit the same file, different content, on 'master', merge to 'b1' and check that we get the file on 'b1' testCatalogerCreateEntry(t, ctx, c, repository, "master", "fileX", nil, "master2") _, err = c.Commit(ctx, repository, "master", "fileX", "tester", nil) + testutil.MustDo(t, "commit on master", err) _, err = c.Merge(ctx, repository, "master", "b1", "tester", "merge changes from master to b1", nil) testutil.MustDo(t, "merge master to b1", err) ent, err := c.GetEntry(ctx, repository, "b1", "fileX", catalog.GetEntryParams{}) diff --git a/catalog/rocks/entry_catalog_test.go b/catalog/rocks/entry_catalog_test.go index bd2e7c0ec7d..803a9ed7c49 100644 --- a/catalog/rocks/entry_catalog_test.go +++ b/catalog/rocks/entry_catalog_test.go @@ -84,11 +84,9 @@ func TestEntryCatalog_ListEntries_NoDelimiter(t *testing.T) { func TestEntryCatalog_ListEntries_WithDelimiter(t *testing.T) { // prepare data - var entriesData []*Entry var gravelerData []*graveler.ValueRecord for _, name := range []string{"file1", "folder/file1", "folder/file2", "zzz"} { entry := &Entry{Address: name} - entriesData = append(entriesData, entry) record := &graveler.ValueRecord{Value: MustEntryToValue(entry), Key: graveler.Key(name)} gravelerData = append(gravelerData, record) } diff --git a/cloud/aws/s3inventory/reader_test.go b/cloud/aws/s3inventory/reader_test.go index 39d4778275f..34254fa2469 100644 --- a/cloud/aws/s3inventory/reader_test.go +++ b/cloud/aws/s3inventory/reader_test.go @@ -72,6 +72,9 @@ func generateParquet(t *testing.T, objs <-chan *TestObject, fieldToRemove string _ = fw.Close() }() jsonSchema, err := json.Marshal(parquetSchema(fieldToRemove)) + if err != nil { + t.Fatalf("failed to marshal to JSON %+v: %s", parquetSchema(fieldToRemove), err) + } jsonSchemaStr := string(jsonSchema) pw, err := writer.NewParquetWriter(fw, jsonSchemaStr, 4) if err != nil { diff --git a/export/tasks_generator_test.go b/export/tasks_generator_test.go index 840cae14970..a35f572e3f0 100644 --- a/export/tasks_generator_test.go +++ b/export/tasks_generator_test.go @@ -153,15 +153,6 @@ func makeAllDependencies(dd directDependencies) allDependencies { return &allDependenciesSet{dd: dd, s: make(map[parade.TaskID]map[parade.TaskID]struct{})} } -func validateAllAfter(t *testing.T, d allDependencies, typ string, before parade.TaskID, afters []parade.TaskID) { - t.Helper() - for _, after := range afters { - if !d.depends(before, after) { - t.Errorf("missing %s dependency %s->%s", typ, before, after) - } - } -} - func cleanup(tasks []parade.TaskData) []parade.TaskData { ret := make([]parade.TaskData, len(tasks)) for i, t := range tasks { diff --git a/gateway/multiparts/tracker_test.go b/gateway/multiparts/tracker_test.go index ca073c63928..6b93dd5a6c0 100644 --- a/gateway/multiparts/tracker_test.go +++ b/gateway/multiparts/tracker_test.go @@ -26,8 +26,7 @@ func Tracker_Get(t *testing.T) { } type args struct { - repository string - uploadID string + uploadID string } tests := []struct { name string diff --git a/graveler/ref/resolver_test.go b/graveler/ref/resolver_test.go index f30511343f0..f7458a83c09 100644 --- a/graveler/ref/resolver_test.go +++ b/graveler/ref/resolver_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/go-test/deep" "github.com/treeverse/lakefs/graveler" "github.com/treeverse/lakefs/testutil" ) @@ -43,36 +44,44 @@ func TestRefManager_Dereference(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - ids := make([]graveler.CommitID, 0) + var commitIDs []graveler.CommitID for iter.Next() { - c := iter.Value() - ids = append(ids, c.CommitID) + commit := iter.Value() + if commit == nil { + t.Fatal("Log iterator returned nil value after Next") + } + commitIDs = append(commitIDs, commit.CommitID) } - if iter.Err() != nil { - t.Fatalf("unexpected error: %v", iter.Err()) + if err := iter.Err(); err != nil { + t.Fatalf("unexpected error: %v", err) } - // commit log: - // "c3f815d633789cd7c1325352277d4de528844c758a9beedfa8a3cfcfb5c75627", - // "8549d7544244ba1b63b5967b6b328b331658f627369cb89bd442684719c318ae", - // "13dafa9c45bcf67e6997776039cbf8ab571ace560ce9e13665f383434a495774", - // "7de38592b9e6046ffb55915a40848f05749f168531f0cd6a2aa61fe6e8d92d02", - // "94c7773c89650e99671c33d46e31226230cdaeed79a77cdbd7c7419ea68b91ca", - // "0efcb6e81db6bdd2cfeb77664b6573a7d69f555bbe561fd1fd018a4e4cac7603", - // "d85e4ae46b63f641b439afde9ebab794a3c39c203a42190c0b9d7773ab71a60e", - // "a766cfdb311fe5f18f489d90d283f65ed522e719fe1ad5397277339eee0d1964", - // "67ea954d570e20172775f41ac9763905d16d73490d9b72731d353db33f85d437", - // "d3b16c2cf7f5b9adc2770976bcabe463a5bdd3b5dbf740034f09a9c663620aed", - // "d420fbf793716d6d53798218d7a247f38a5bbed095d57df71ee79e05446e46ec", - // "cc72bda1adade1a72b3de617472c16af187063c79e7edc7921c04e883b44de4c", - // "752581ac60bd8e38a2e65a754591a93a1703dc6c658f91380b8836013188c566", - // "3cf70857454c71fd0bbf69af8a5360671ba98f6ac9371b047144208c58c672a2", - // "bfa1e0382ff3c51905dc62ced0a67588b5219c1bba71a517ae7e7857f0c26afe", - // "d2248dcc1a4de004e10e3bc6b820655e649b8d986d983b60ec98a357a0df194b", - // "a2d98d820f6ff3f221223dbe6a22548f78549830d3b19286b101f13a0ee34085", - // "4f13621ec00d4e44e8a0f0ad340224f9d51db9b6518ee7bef17f598aea9e0431", - // "df87d5329f4438662d6ecb9b90ee17c0bdc9a78a884acc93c0c4fe9f0f79d059", - // "29706d36de7219e0796c31b278f87201ef835e8cdafbcc3c907d292cd31f77d5", + commitLog := []graveler.CommitID{ + "c3f815d633789cd7c1325352277d4de528844c758a9beedfa8a3cfcfb5c75627", + "8549d7544244ba1b63b5967b6b328b331658f627369cb89bd442684719c318ae", + "13dafa9c45bcf67e6997776039cbf8ab571ace560ce9e13665f383434a495774", + "7de38592b9e6046ffb55915a40848f05749f168531f0cd6a2aa61fe6e8d92d02", + "94c7773c89650e99671c33d46e31226230cdaeed79a77cdbd7c7419ea68b91ca", + "0efcb6e81db6bdd2cfeb77664b6573a7d69f555bbe561fd1fd018a4e4cac7603", + "d85e4ae46b63f641b439afde9ebab794a3c39c203a42190c0b9d7773ab71a60e", + "a766cfdb311fe5f18f489d90d283f65ed522e719fe1ad5397277339eee0d1964", + "67ea954d570e20172775f41ac9763905d16d73490d9b72731d353db33f85d437", + "d3b16c2cf7f5b9adc2770976bcabe463a5bdd3b5dbf740034f09a9c663620aed", + "d420fbf793716d6d53798218d7a247f38a5bbed095d57df71ee79e05446e46ec", + "cc72bda1adade1a72b3de617472c16af187063c79e7edc7921c04e883b44de4c", + "752581ac60bd8e38a2e65a754591a93a1703dc6c658f91380b8836013188c566", + "3cf70857454c71fd0bbf69af8a5360671ba98f6ac9371b047144208c58c672a2", + "bfa1e0382ff3c51905dc62ced0a67588b5219c1bba71a517ae7e7857f0c26afe", + "d2248dcc1a4de004e10e3bc6b820655e649b8d986d983b60ec98a357a0df194b", + "a2d98d820f6ff3f221223dbe6a22548f78549830d3b19286b101f13a0ee34085", + "4f13621ec00d4e44e8a0f0ad340224f9d51db9b6518ee7bef17f598aea9e0431", + "df87d5329f4438662d6ecb9b90ee17c0bdc9a78a884acc93c0c4fe9f0f79d059", + "29706d36de7219e0796c31b278f87201ef835e8cdafbcc3c907d292cd31f77d5", + } + + if diff := deep.Equal(commitIDs, commitLog); diff != nil { + t.Fatal("Difference found on commit log", diff) + } testutil.Must(t, r.SetBranch(ctx, "repo1", "branch1", graveler.Branch{ CommitID: "13dafa9c45bcf67e6997776039cbf8ab571ace560ce9e13665f383434a495774", diff --git a/graveler/sstable/batch_test.go b/graveler/sstable/batch_test.go index 0edcb1c41d6..53490f5d78a 100644 --- a/graveler/sstable/batch_test.go +++ b/graveler/sstable/batch_test.go @@ -46,7 +46,7 @@ func TestBatchCloserMultipleWaitCalls(t *testing.T) { writer := mock.NewMockRangeWriter(ctrl) writer.EXPECT().Close().Return(&committed.WriteResult{ - RangeID: committed.ID("last"), + RangeID: "last", First: committed.Key("row_1"), Last: committed.Key("row_2"), Count: 4321, @@ -62,8 +62,8 @@ func runSuccessScenario(t *testing.T) (*sstable.BatchCloser, *gomock.Controller) ctrl := gomock.NewController(t) defer ctrl.Finish() - writersCount := 10 - writers := make([]*mock.MockRangeWriter, writersCount, writersCount) + const writersCount = 10 + writers := make([]*mock.MockRangeWriter, writersCount) for i := 0; i < writersCount; i++ { writers[i] = mock.NewMockRangeWriter(ctrl) writers[i].EXPECT().Close().Return(&committed.WriteResult{ diff --git a/graveler/sstable/writer_test.go b/graveler/sstable/writer_test.go index 2cf50bdc2fb..9512293521f 100644 --- a/graveler/sstable/writer_test.go +++ b/graveler/sstable/writer_test.go @@ -91,6 +91,7 @@ func TestWriterAbort(t *testing.T) { Key: []byte("key-1"), Value: []byte("some-data"), }) + require.NoError(t, err) // Abort require.NoError(t, dw.Abort()) @@ -121,6 +122,7 @@ func TestWriterAbortAfterClose(t *testing.T) { Key: []byte("key-1"), Value: []byte("some-data"), }) + require.NoError(t, err) // Close result, err := dw.Close() diff --git a/parade/parade_test.go b/parade/parade_test.go index 99b091b9383..fa5b3c35ac9 100644 --- a/parade/parade_test.go +++ b/parade/parade_test.go @@ -22,16 +22,10 @@ import ( "github.com/ory/dockertest/v3" ) -const ( - dbContainerTimeoutSeconds = 10 * 60 // 10 min - dbName = "parade_db" -) - var ( pool *dockertest.Pool databaseURI string - postgresUrl = flag.String("postgres-url", "", "Postgres connection string. If unset, run a Postgres in a Docker container.") parallelism = flag.Int("parallelism", 16, "Number of concurrent client worker goroutines.") bulk = flag.Int("bulk", 2_000, "Number of tasks to acquire at once in each client goroutine.") taskFactor = flag.Int("task-factor", 20_000, "Scale benchmark N by this many tasks") diff --git a/pyramid/tier_fs_test.go b/pyramid/tier_fs_test.go index 2134718b1ae..b2285db4add 100644 --- a/pyramid/tier_fs_test.go +++ b/pyramid/tier_fs_test.go @@ -106,7 +106,7 @@ func TestStartup(t *testing.T) { require.True(t, os.IsNotExist(err)) f, err := localFS.Open(namespace, filename) - defer f.Close() + defer func() { _ = f.Close() }() require.NoError(t, err) bytes, err := ioutil.ReadAll(f) @@ -208,8 +208,8 @@ func checkContent(t *testing.T, namespace string, filename string, content []byt type mockEv struct{} -func (_ *mockEv) Touch(_ params.RelativePath) {} +func (*mockEv) Touch(params.RelativePath) {} -func (_ *mockEv) Store(_ params.RelativePath, _ int64) bool { +func (*mockEv) Store(params.RelativePath, int64) bool { return true } diff --git a/retention/map_error_test.go b/retention/map_error_test.go index 8e9c58b3a91..4acd53bf41e 100644 --- a/retention/map_error_test.go +++ b/retention/map_error_test.go @@ -2,7 +2,6 @@ package retention_test import ( "errors" - "fmt" "testing" "github.com/treeverse/lakefs/retention" @@ -52,11 +51,10 @@ func TestField_String(t *testing.T) { } } -var wErr error = fmt.Errorf("error for testing") - func TestMapError(t *testing.T) { - err := retention.MapError{Fields: retention.Fields{"a": 1, "b": 2}, WrappedError: wErr} - if !errors.Is(err, wErr) { - t.Errorf("error %s failed to wrap its base %s", err, wErr) + errTesting := errors.New("error for testing") + err := retention.MapError{Fields: retention.Fields{"a": 1, "b": 2}, WrappedError: errTesting} + if !errors.Is(err, errTesting) { + t.Errorf("error %s failed to wrap its base %s", err, errTesting) } }