Fix minor lint issues (#1128)

* minor lint issues found * new error * Update cloud/aws/s3inventory/reader_test.go Co-authored-by: arielshaqed <ariels@treeverse.io> * compare commit log * fix test to verify entry not found Co-authored-by: arielshaqed <ariels@treeverse.io>
treeverse · Jan 3, 2021 · 2ea3850 · 2ea3850
1 parent ef700b3
commit 2ea3850
Show file tree

Hide file tree

Showing 13 changed files with 65 additions and 65 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
 	}
 }