From a83897050105191729eaf63170fd94675c4ceded Mon Sep 17 00:00:00 2001 From: nadavsteindler Date: Thu, 4 Jul 2024 10:00:10 +0300 Subject: [PATCH] suite change (#7822) --- pkg/block/blocktest/adapter.go | 193 +---------------- pkg/block/blocktest/basic_suite.go | 201 ++++++++++++++++++ .../{multipart.go => multipart_suite.go} | 8 + 3 files changed, 211 insertions(+), 191 deletions(-) create mode 100644 pkg/block/blocktest/basic_suite.go rename pkg/block/blocktest/{multipart.go => multipart_suite.go} (94%) diff --git a/pkg/block/blocktest/adapter.go b/pkg/block/blocktest/adapter.go index 51f5e826a34..1b1e4bf61e6 100644 --- a/pkg/block/blocktest/adapter.go +++ b/pkg/block/blocktest/adapter.go @@ -12,7 +12,6 @@ import ( "testing" "time" - "github.com/go-test/deep" "github.com/stretchr/testify/require" "github.com/treeverse/lakefs/pkg/block" "github.com/treeverse/lakefs/pkg/ingest/store" @@ -20,201 +19,13 @@ import ( // AdapterTest Test suite of basic adapter functionality func AdapterTest(t *testing.T, adapter block.Adapter, storageNamespace, externalPath string) { - t.Run("Adapter_PutGet", func(t *testing.T) { testAdapterPutGet(t, adapter, storageNamespace, externalPath) }) - t.Run("Adapter_Copy", func(t *testing.T) { testAdapterCopy(t, adapter, storageNamespace) }) - t.Run("Adapter_Remove", func(t *testing.T) { testAdapterRemove(t, adapter, storageNamespace) }) - t.Run("Adapter_MultipartUpload", func(t *testing.T) { testAdapterMultipartUpload(t, adapter, storageNamespace) }) - t.Run("Adapter_AbortMultiPartUpload", func(t *testing.T) { testAdapterAbortMultipartUpload(t, adapter, storageNamespace) }) - t.Run("Adapter_CopyPart", func(t *testing.T) { testAdapterCopyPart(t, adapter, storageNamespace) }) - t.Run("Adapter_CopyPartRange", func(t *testing.T) { testAdapterCopyPartRange(t, adapter, storageNamespace) }) - t.Run("Adapter_Exists", func(t *testing.T) { testAdapterExists(t, adapter, storageNamespace) }) + AdapterBasicObjectTest(t, adapter, storageNamespace, externalPath) + AdapterMultipartTest(t, adapter, storageNamespace, externalPath) t.Run("Adapter_GetRange", func(t *testing.T) { testAdapterGetRange(t, adapter, storageNamespace) }) t.Run("Adapter_Walker", func(t *testing.T) { testAdapterWalker(t, adapter, storageNamespace) }) t.Run("Adapter_GetPreSignedURL", func(t *testing.T) { testGetPreSignedURL(t, adapter, storageNamespace) }) } -// Parameterized test to first Put object via Storage Adapter then Get it and check that the contents match -func testAdapterPutGet(t *testing.T, adapter block.Adapter, storageNamespace, externalPath string) { - ctx := context.Background() - const contents = "test_file" - size := int64(len(contents)) - - cases := []struct { - name string - identifierType block.IdentifierType - path string - }{ - {"identifier_relative", block.IdentifierTypeRelative, "test_file"}, - {"identifier_full", block.IdentifierTypeFull, externalPath + "/" + "test_file"}, - {"identifier_unknown_relative", block.IdentifierTypeUnknownDeprecated, "test_file"}, //nolint:staticcheck - {"identifier_unknown_full", block.IdentifierTypeUnknownDeprecated, externalPath + "/" + "test_file"}, //nolint:staticcheck - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - obj := block.ObjectPointer{ - StorageNamespace: storageNamespace, - Identifier: c.path, - IdentifierType: c.identifierType, - } - - err := adapter.Put(ctx, obj, size, strings.NewReader(contents), block.PutOpts{}) - require.NoError(t, err) - - reader, err := adapter.Get(ctx, obj) - require.NoError(t, err) - defer func() { - require.NoError(t, reader.Close()) - }() - got, err := io.ReadAll(reader) - require.NoError(t, err) - require.Equal(t, contents, string(got)) - }) - } -} - -// Test to Copy an object via Storage Adapter, then check that the contents of the copied object matches the original -func testAdapterCopy(t *testing.T, adapter block.Adapter, storageNamespace string) { - ctx := context.Background() - contents := "foo bar baz quux" - src := block.ObjectPointer{ - StorageNamespace: storageNamespace, - Identifier: "src", - IdentifierType: block.IdentifierTypeRelative, - } - dst := block.ObjectPointer{ - StorageNamespace: storageNamespace, - Identifier: "export/to/dst", - IdentifierType: block.IdentifierTypeRelative, - } - - require.NoError(t, adapter.Put(ctx, src, int64(len(contents)), strings.NewReader(contents), block.PutOpts{})) - - require.NoError(t, adapter.Copy(ctx, src, dst)) - reader, err := adapter.Get(ctx, dst) - require.NoError(t, err) - got, err := io.ReadAll(reader) - require.NoError(t, err) - require.Equal(t, contents, string(got)) -} - -// Parameterized test to test valid and invalid cases for Removing an object via the adaptor -func testAdapterRemove(t *testing.T, adapter block.Adapter, storageNamespace string) { - ctx := context.Background() - const content = "Content used for testing" - tests := []struct { - name string - additionalObjects []string - path string - wantErr bool - wantTree []string - }{ - { - name: "test_single", - path: "README", - wantErr: false, - wantTree: []string{}, - }, - - { - name: "test_under_folder", - path: "src/tools.go", - wantErr: false, - wantTree: []string{}, - }, - { - name: "test_under_multiple_folders", - path: "a/b/c/d.txt", - wantErr: false, - wantTree: []string{}, - }, - { - name: "file_in_the_way", - path: "a/b/c/d.txt", - additionalObjects: []string{"a/b/blocker.txt"}, - wantErr: false, - wantTree: []string{"/a/b/blocker.txt"}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // setup env - envObjects := tt.additionalObjects - envObjects = append(envObjects, tt.path) - for _, p := range envObjects { - obj := block.ObjectPointer{ - StorageNamespace: storageNamespace, - Identifier: tt.name + "/" + p, - IdentifierType: block.IdentifierTypeRelative, - } - require.NoError(t, adapter.Put(ctx, obj, int64(len(content)), strings.NewReader(content), block.PutOpts{})) - } - - // test Remove - obj := block.ObjectPointer{ - StorageNamespace: storageNamespace, - Identifier: tt.name + "/" + tt.path, - IdentifierType: block.IdentifierTypeRelative, - } - if err := adapter.Remove(ctx, obj); (err != nil) != tt.wantErr { - t.Errorf("Remove() error = %v, wantErr %v", err, tt.wantErr) - } - - qk, err := adapter.ResolveNamespace(storageNamespace, tt.name, block.IdentifierTypeRelative) - require.NoError(t, err) - - tree := dumpPathTree(t, ctx, adapter, qk) - if diff := deep.Equal(tt.wantTree, tree); diff != nil { - t.Errorf("Remove() tree diff = %s", diff) - } - }) - } -} - -// Parameterized test of the object Exists method of the Storage adapter -func testAdapterExists(t *testing.T, adapter block.Adapter, storageNamespace string) { - // TODO (niro): Test abs paths - const contents = "exists" - ctx := context.Background() - err := adapter.Put(ctx, block.ObjectPointer{ - StorageNamespace: storageNamespace, - Identifier: contents, - IdentifierType: block.IdentifierTypeRelative, - }, int64(len(contents)), strings.NewReader(contents), block.PutOpts{}) - require.NoError(t, err) - - err = adapter.Put(ctx, block.ObjectPointer{ - StorageNamespace: storageNamespace, - Identifier: "nested/and/" + contents, - IdentifierType: block.IdentifierTypeRelative, - }, int64(len(contents)), strings.NewReader(contents), block.PutOpts{}) - require.NoError(t, err) - - cases := []struct { - name string - path string - exists bool - }{ - {"exists", "exists", true}, - {"nested_exists", "nested/and/exists", true}, - {"simple_missing", "missing", false}, - {"nested_missing", "nested/down", false}, - {"nested_deep_missing", "nested/quite/deeply/and/missing", false}, - } - for _, tt := range cases { - t.Run(tt.name, func(t *testing.T) { - ok, err := adapter.Exists(ctx, block.ObjectPointer{ - StorageNamespace: storageNamespace, - Identifier: tt.path, - IdentifierType: block.IdentifierTypeRelative, - }) - require.NoError(t, err) - require.Equal(t, tt.exists, ok) - }) - } -} - // Parameterized test of the GetRange functionality func testAdapterGetRange(t *testing.T, adapter block.Adapter, storageNamespace string) { ctx := context.Background() diff --git a/pkg/block/blocktest/basic_suite.go b/pkg/block/blocktest/basic_suite.go new file mode 100644 index 00000000000..adaf4e1261d --- /dev/null +++ b/pkg/block/blocktest/basic_suite.go @@ -0,0 +1,201 @@ +package blocktest + +import ( + "context" + "github.com/go-test/deep" + "github.com/stretchr/testify/require" + "github.com/treeverse/lakefs/pkg/block" + "io" + "strings" + "testing" +) + +// AdapterBasicObjectTest Test suite of adapter basic functionality on objects +func AdapterBasicObjectTest(t *testing.T, adapter block.Adapter, storageNamespace, externalPath string) { + t.Run("Adapter_PutGet", func(t *testing.T) { testAdapterPutGet(t, adapter, storageNamespace, externalPath) }) + t.Run("Adapter_Copy", func(t *testing.T) { testAdapterCopy(t, adapter, storageNamespace) }) + t.Run("Adapter_Remove", func(t *testing.T) { testAdapterRemove(t, adapter, storageNamespace) }) + t.Run("Adapter_Exists", func(t *testing.T) { testAdapterExists(t, adapter, storageNamespace) }) +} + +// Parameterized test to first Put object via Storage Adapter then Get it and check that the contents match +func testAdapterPutGet(t *testing.T, adapter block.Adapter, storageNamespace, externalPath string) { + ctx := context.Background() + const contents = "test_file" + size := int64(len(contents)) + + cases := []struct { + name string + identifierType block.IdentifierType + path string + }{ + {"identifier_relative", block.IdentifierTypeRelative, "test_file"}, + {"identifier_full", block.IdentifierTypeFull, externalPath + "/" + "test_file"}, + {"identifier_unknown_relative", block.IdentifierTypeUnknownDeprecated, "test_file"}, //nolint:staticcheck + {"identifier_unknown_full", block.IdentifierTypeUnknownDeprecated, externalPath + "/" + "test_file"}, //nolint:staticcheck + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + obj := block.ObjectPointer{ + StorageNamespace: storageNamespace, + Identifier: c.path, + IdentifierType: c.identifierType, + } + + err := adapter.Put(ctx, obj, size, strings.NewReader(contents), block.PutOpts{}) + require.NoError(t, err) + + reader, err := adapter.Get(ctx, obj) + require.NoError(t, err) + defer func() { + require.NoError(t, reader.Close()) + }() + got, err := io.ReadAll(reader) + require.NoError(t, err) + require.Equal(t, contents, string(got)) + }) + } +} + +// Test to Copy an object via Storage Adapter, then check that the contents of the copied object matches the original +func testAdapterCopy(t *testing.T, adapter block.Adapter, storageNamespace string) { + ctx := context.Background() + contents := "foo bar baz quux" + src := block.ObjectPointer{ + StorageNamespace: storageNamespace, + Identifier: "src", + IdentifierType: block.IdentifierTypeRelative, + } + dst := block.ObjectPointer{ + StorageNamespace: storageNamespace, + Identifier: "export/to/dst", + IdentifierType: block.IdentifierTypeRelative, + } + + require.NoError(t, adapter.Put(ctx, src, int64(len(contents)), strings.NewReader(contents), block.PutOpts{})) + + require.NoError(t, adapter.Copy(ctx, src, dst)) + reader, err := adapter.Get(ctx, dst) + require.NoError(t, err) + got, err := io.ReadAll(reader) + require.NoError(t, err) + require.Equal(t, contents, string(got)) +} + +// Parameterized test to test valid and invalid cases for Removing an object via the adaptor +func testAdapterRemove(t *testing.T, adapter block.Adapter, storageNamespace string) { + ctx := context.Background() + const content = "Content used for testing" + tests := []struct { + name string + additionalObjects []string + path string + wantErr bool + wantTree []string + }{ + { + name: "test_single", + path: "README", + wantErr: false, + wantTree: []string{}, + }, + + { + name: "test_under_folder", + path: "src/tools.go", + wantErr: false, + wantTree: []string{}, + }, + { + name: "test_under_multiple_folders", + path: "a/b/c/d.txt", + wantErr: false, + wantTree: []string{}, + }, + { + name: "file_in_the_way", + path: "a/b/c/d.txt", + additionalObjects: []string{"a/b/blocker.txt"}, + wantErr: false, + wantTree: []string{"/a/b/blocker.txt"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // setup env + envObjects := tt.additionalObjects + envObjects = append(envObjects, tt.path) + for _, p := range envObjects { + obj := block.ObjectPointer{ + StorageNamespace: storageNamespace, + Identifier: tt.name + "/" + p, + IdentifierType: block.IdentifierTypeRelative, + } + require.NoError(t, adapter.Put(ctx, obj, int64(len(content)), strings.NewReader(content), block.PutOpts{})) + } + + // test Remove + obj := block.ObjectPointer{ + StorageNamespace: storageNamespace, + Identifier: tt.name + "/" + tt.path, + IdentifierType: block.IdentifierTypeRelative, + } + if err := adapter.Remove(ctx, obj); (err != nil) != tt.wantErr { + t.Errorf("Remove() error = %v, wantErr %v", err, tt.wantErr) + } + + qk, err := adapter.ResolveNamespace(storageNamespace, tt.name, block.IdentifierTypeRelative) + require.NoError(t, err) + + tree := dumpPathTree(t, ctx, adapter, qk) + if diff := deep.Equal(tt.wantTree, tree); diff != nil { + t.Errorf("Remove() tree diff = %s", diff) + } + }) + } +} + +// Parameterized test of the object Exists method of the Storage adapter +func testAdapterExists(t *testing.T, adapter block.Adapter, storageNamespace string) { + // TODO (niro): Test abs paths + const contents = "exists" + ctx := context.Background() + err := adapter.Put(ctx, block.ObjectPointer{ + StorageNamespace: storageNamespace, + Identifier: contents, + IdentifierType: block.IdentifierTypeRelative, + }, int64(len(contents)), strings.NewReader(contents), block.PutOpts{}) + require.NoError(t, err) + + err = adapter.Put(ctx, block.ObjectPointer{ + StorageNamespace: storageNamespace, + Identifier: "nested/and/" + contents, + IdentifierType: block.IdentifierTypeRelative, + }, int64(len(contents)), strings.NewReader(contents), block.PutOpts{}) + require.NoError(t, err) + + cases := []struct { + name string + path string + exists bool + }{ + {"exists", "exists", true}, + {"nested_exists", "nested/and/exists", true}, + {"simple_missing", "missing", false}, + {"nested_missing", "nested/down", false}, + {"nested_deep_missing", "nested/quite/deeply/and/missing", false}, + } + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + ok, err := adapter.Exists(ctx, block.ObjectPointer{ + StorageNamespace: storageNamespace, + Identifier: tt.path, + IdentifierType: block.IdentifierTypeRelative, + }) + require.NoError(t, err) + require.Equal(t, tt.exists, ok) + }) + } +} diff --git a/pkg/block/blocktest/multipart.go b/pkg/block/blocktest/multipart_suite.go similarity index 94% rename from pkg/block/blocktest/multipart.go rename to pkg/block/blocktest/multipart_suite.go index de96b236fdb..2959a087fce 100644 --- a/pkg/block/blocktest/multipart.go +++ b/pkg/block/blocktest/multipart_suite.go @@ -17,6 +17,14 @@ const ( multipartPartSize = 5 * 1024 * 1024 // generally the minimum size for multi-part upload ) +// AdapterMultipartTest Test suite of adapter multipart functionality +func AdapterMultipartTest(t *testing.T, adapter block.Adapter, storageNamespace, externalPath string) { + t.Run("Adapter_MultipartUpload", func(t *testing.T) { testAdapterMultipartUpload(t, adapter, storageNamespace) }) + t.Run("Adapter_AbortMultiPartUpload", func(t *testing.T) { testAdapterAbortMultipartUpload(t, adapter, storageNamespace) }) + t.Run("Adapter_CopyPart", func(t *testing.T) { testAdapterCopyPart(t, adapter, storageNamespace) }) + t.Run("Adapter_CopyPartRange", func(t *testing.T) { testAdapterCopyPartRange(t, adapter, storageNamespace) }) +} + // Parameterized test of the Multipart Upload APIs. After successful upload we Get the result and compare to the original func testAdapterMultipartUpload(t *testing.T, adapter block.Adapter, storageNamespace string) { ctx := context.Background()