diff --git a/pkg/cli/workspace.go b/pkg/cli/workspace.go index bb369e2..029e496 100644 --- a/pkg/cli/workspace.go +++ b/pkg/cli/workspace.go @@ -14,6 +14,7 @@ type workspaceProvider struct { DataHome string `usage:"The data home directory or bucket name" env:"WORKSPACE_PROVIDER_DATA_HOME"` S3Bucket string `usage:"The S3 bucket name" name:"s3-bucket" env:"WORKSPACE_PROVIDER_S3_BUCKET"` S3BaseEndpoint string `usage:"The S3 base endpoint to use with S3 compatible providers" name:"s3-base-endpoint" env:"WORKSPACE_PROVIDER_S3_BASE_ENDPOINT"` + S3UsePathStyle bool `usage:"Use path style addressing for S3 compatible providers" name:"s3-use-path-style" env:"WORKSPACE_PROVIDER_S3_USE_PATH_STYLE"` AzureContainer string `usage:"The Azure container name" name:"azure-container" env:"WORKSPACE_PROVIDER_AZURE_CONTAINER"` AzureConnectionString string `usage:"The Azure connection string" name:"azure-connection-string" env:"WORKSPACE_PROVIDER_AZURE_CONNECTION_STRING"` @@ -73,6 +74,7 @@ func (w *workspaceProvider) PersistentPre(cmd *cobra.Command, _ []string) error DirectoryDataHome: w.DataHome, S3BucketName: w.S3Bucket, S3BaseEndpoint: w.S3BaseEndpoint, + S3UsePathStyle: w.S3UsePathStyle, AzureContainerName: w.AzureContainer, AzureConnectionString: w.AzureConnectionString, }) diff --git a/pkg/client/client.go b/pkg/client/client.go index 8ee6f0c..e26f93f 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -42,6 +42,7 @@ type Options struct { DirectoryDataHome string S3BucketName string S3BaseEndpoint string + S3UsePathStyle bool AzureContainerName string AzureConnectionString string } @@ -59,6 +60,7 @@ func complete(opts ...Options) Options { if o.S3BaseEndpoint != "" { opt.S3BaseEndpoint = o.S3BaseEndpoint } + opt.S3UsePathStyle = o.S3UsePathStyle if o.AzureContainerName != "" { opt.AzureContainerName = o.AzureContainerName } @@ -82,7 +84,7 @@ func New(ctx context.Context, opts ...Options) (*Client, error) { } if opt.S3BucketName != "" { - factory, err := newS3(ctx, opt.S3BucketName, opt.S3BaseEndpoint) + factory, err := newS3(ctx, opt.S3BucketName, opt.S3BaseEndpoint, opt.S3UsePathStyle) if err != nil { return nil, err } diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 77f5e88..d4d1d4d 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -15,6 +15,7 @@ import ( var c, _ = New(context.Background(), Options{ S3BucketName: os.Getenv("WORKSPACE_PROVIDER_S3_BUCKET"), S3BaseEndpoint: os.Getenv("WORKSPACE_PROVIDER_S3_BASE_ENDPOINT"), + S3UsePathStyle: os.Getenv("WORKSPACE_PROVIDER_S3_USE_PATH_STYLE") == "true", AzureContainerName: os.Getenv("WORKSPACE_PROVIDER_AZURE_CONTAINER"), AzureConnectionString: os.Getenv("WORKSPACE_PROVIDER_AZURE_CONNECTION_STRING"), }) @@ -83,7 +84,7 @@ func TestCreateAndRmS3Provider(t *testing.T) { bucket, dir, _ := strings.Cut(strings.TrimPrefix(id, S3Provider+"://"), "/") testS3Provider := &s3Provider{ bucket: bucket, - client: s3Prv.client, + client: s3TestSetups[0].provider.client, } // Nothing should be created @@ -318,6 +319,10 @@ func TestCreateAndRmS3ProviderFromDirectoryProvider(t *testing.T) { t.Errorf("unexpected error when listing workspaceContent: %v", err) } + if len(workspaceContent) == 0 { + t.Fatalf("workspaceContent is empty") + } + if len(workspaceContent) != 1 { t.Errorf("unexpected number of workspaceContent: %d", len(workspaceContent)) } diff --git a/pkg/client/directory_test.go b/pkg/client/directory_test.go index ee95cad..263c66e 100644 --- a/pkg/client/directory_test.go +++ b/pkg/client/directory_test.go @@ -13,17 +13,22 @@ import ( "testing" ) +type s3TestSetup struct { + name string + factory workspaceFactory + testingID string + provider *s3Provider +} + var ( directoryFactory workspaceFactory - s3Factory workspaceFactory - azureFactory workspaceFactory directoryTestingID string - s3TestingID string - azureTestingID string dirPrv workspaceClient - s3Prv *s3Provider - azurePrv *azureProvider + s3TestSetups []s3TestSetup skipS3Tests = os.Getenv("WORKSPACE_PROVIDER_S3_BUCKET") == "" + azureFactory workspaceFactory + azureTestingID string + azurePrv *azureProvider skipAzureTests = os.Getenv("WORKSPACE_PROVIDER_AZURE_CONNECTION_STRING") == "" || os.Getenv("WORKSPACE_PROVIDER_AZURE_CONTAINER") == "" ) @@ -33,12 +38,30 @@ func TestMain(m *testing.M) { dirPrv, _ = directoryFactory.New(directoryTestingID) if !skipS3Tests { - s3Factory, _ = newS3(context.Background(), os.Getenv("WORKSPACE_PROVIDER_S3_BUCKET"), os.Getenv("WORKSPACE_PROVIDER_S3_BASE_ENDPOINT")) - // This won't ever error because it doesn't create anything. - s3TestingID = s3Factory.Create() + if os.Getenv("WORKSPACE_PROVIDER_S3_USE_PATH_STYLE") != "true" { + s3Factory, _ := newS3(context.Background(), os.Getenv("WORKSPACE_PROVIDER_S3_BUCKET"), os.Getenv("WORKSPACE_PROVIDER_S3_BASE_ENDPOINT"), false) + // This won't ever error because it doesn't create anything. + s3TestingID := s3Factory.Create() + + s3Client, _ := s3Factory.New(s3TestingID) + s3Prv := s3Client.(*s3Provider) + s3TestSetups = append(s3TestSetups, s3TestSetup{ + name: "default", + factory: s3Factory, + testingID: s3TestingID, + provider: s3Prv, + }) + } - s3Client, _ := s3Factory.New(s3TestingID) - s3Prv = s3Client.(*s3Provider) + s3PathStyleFactory, _ := newS3(context.Background(), os.Getenv("WORKSPACE_PROVIDER_S3_BUCKET"), os.Getenv("WORKSPACE_PROVIDER_S3_BASE_ENDPOINT"), true) + s3PathStyleTestingID := s3PathStyleFactory.Create() + s3PathStyleClient, _ := s3PathStyleFactory.New(s3PathStyleTestingID) + s3TestSetups = append(s3TestSetups, s3TestSetup{ + name: "use-path-style", + factory: s3PathStyleFactory, + testingID: s3PathStyleTestingID, + provider: s3PathStyleClient.(*s3Provider), + }) } if !skipAzureTests { @@ -58,8 +81,10 @@ func TestMain(m *testing.M) { } if !skipS3Tests { - if err := s3Factory.Rm(context.Background(), s3TestingID); err != nil { - errs = append(errs, fmt.Errorf("error removing s3 workspace: %v", err)) + for _, s3TS := range s3TestSetups { + if err := s3TS.factory.Rm(context.Background(), s3TS.testingID); err != nil { + errs = append(errs, fmt.Errorf("error removing s3 workspace: %v", err)) + } } } diff --git a/pkg/client/s3.go b/pkg/client/s3.go index 18f2498..8862366 100644 --- a/pkg/client/s3.go +++ b/pkg/client/s3.go @@ -21,7 +21,7 @@ import ( "github.com/gabriel-vasile/mimetype" ) -func newS3(ctx context.Context, bucket, baseEndpoint string) (workspaceFactory, error) { +func newS3(ctx context.Context, bucket string, baseEndpoint string, usePathStyle bool) (workspaceFactory, error) { cfg, err := config.LoadDefaultConfig(ctx) if err != nil { return nil, err @@ -31,6 +31,7 @@ func newS3(ctx context.Context, bucket, baseEndpoint string) (workspaceFactory, if baseEndpoint != "" { o.BaseEndpoint = aws.String(baseEndpoint) } + o.UsePathStyle = usePathStyle // often required e.g. for MinIO which requires extra configuration for virtual hosted-style requests }) return &s3Provider{ bucket: bucket, diff --git a/pkg/client/s3_test.go b/pkg/client/s3_test.go index 6a67886..031fe53 100644 --- a/pkg/client/s3_test.go +++ b/pkg/client/s3_test.go @@ -22,27 +22,31 @@ func TestCreateAndRmS3(t *testing.T) { t.Skip("Skipping S3 tests") } - id := s3Factory.Create() - if !strings.HasPrefix(id, S3Provider+"://") { - t.Errorf("unexpected id: %s", id) - } + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + id := s3TS.factory.Create() + if !strings.HasPrefix(id, S3Provider+"://") { + t.Errorf("unexpected id: %s", id) + } - bucket, dir, _ := strings.Cut(strings.TrimPrefix(id, S3Provider+"://"), "/") - testS3Provider := &s3Provider{ - bucket: bucket, - client: s3Prv.client, - } + bucket, dir, _ := strings.Cut(strings.TrimPrefix(id, S3Provider+"://"), "/") + testS3Provider := &s3Provider{ + bucket: bucket, + client: s3TS.provider.client, + } - // Nothing should be created - var respErr *http.ResponseError - if _, err := testS3Provider.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &testS3Provider.bucket, Key: &dir}); err == nil { - t.Errorf("expected error when checking if workspace exists") - } else if !errors.As(err, &respErr) || respErr.Response.StatusCode != 404 { - t.Errorf("unexpected error when checking if workspace exists: %v", err) - } + // Nothing should be created + var respErr *http.ResponseError + if _, err := testS3Provider.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &testS3Provider.bucket, Key: &dir}); err == nil { + t.Errorf("expected error when checking if workspace exists") + } else if !errors.As(err, &respErr) || respErr.Response.StatusCode != 404 { + t.Errorf("unexpected error when checking if workspace exists: %v", err) + } - if err := s3Factory.Rm(context.Background(), id); err != nil { - t.Errorf("unexpected error when removing workspace: %v", err) + if err := s3TS.factory.Rm(context.Background(), id); err != nil { + t.Errorf("unexpected error when removing workspace: %v", err) + } + }) } } @@ -51,45 +55,49 @@ func TestWriteAndDeleteFileInS3(t *testing.T) { t.Skip("Skipping S3 tests") } - // Copy a file into the workspace - if err := s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { - t.Fatalf("error getting file to write: %v", err) - } + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + // Copy a file into the workspace + if err := s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { + t.Fatalf("error getting file to write: %v", err) + } - // Ensure the file actually exists - obj, err := s3Prv.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3Prv.bucket, Key: aws.String(fmt.Sprintf("%s/%s", s3Prv.dir, "test.txt"))}) - if err != nil { - t.Errorf("error when checking if file exists: %v", err) - } - defer obj.Body.Close() + // Ensure the file actually exists + obj, err := s3TS.provider.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3TS.provider.bucket, Key: aws.String(fmt.Sprintf("%s/%s", s3TS.provider.dir, "test.txt"))}) + if err != nil { + t.Errorf("error when checking if file exists: %v", err) + } + defer obj.Body.Close() - // Stat the file and compare with the original - providerStat, err := s3Prv.StatFile(context.Background(), "test.txt", StatOptions{}) - if err != nil { - t.Errorf("unexpected error when statting file: %v", err) - } + // Stat the file and compare with the original + providerStat, err := s3TS.provider.StatFile(context.Background(), "test.txt", StatOptions{}) + if err != nil { + t.Errorf("unexpected error when statting file: %v", err) + } - if providerStat.WorkspaceID != s3TestingID { - t.Errorf("unexpected workspace id: %s", providerStat.WorkspaceID) - } - if providerStat.Size != aws.ToInt64(obj.ContentLength) { - t.Errorf("unexpected file size: %d", providerStat.Size) - } - if providerStat.Name != "test.txt" { - t.Errorf("unexpected file name: %s", providerStat.Name) - } - if providerStat.ModTime.Compare(aws.ToTime(obj.LastModified)) != 0 { - t.Errorf("unexpected file mod time: %s", providerStat.ModTime) - } + if providerStat.WorkspaceID != s3TS.testingID { + t.Errorf("unexpected workspace id: %s", providerStat.WorkspaceID) + } + if providerStat.Size != aws.ToInt64(obj.ContentLength) { + t.Errorf("unexpected file size: %d", providerStat.Size) + } + if providerStat.Name != "test.txt" { + t.Errorf("unexpected file name: %s", providerStat.Name) + } + if providerStat.ModTime.Compare(aws.ToTime(obj.LastModified)) != 0 { + t.Errorf("unexpected file mod time: %s", providerStat.ModTime) + } - // Delete the file - if err := s3Prv.DeleteFile(context.Background(), "test.txt"); err != nil { - t.Errorf("unexpected error when deleting file: %v", err) - } + // Delete the file + if err := s3TS.provider.DeleteFile(context.Background(), "test.txt"); err != nil { + t.Errorf("unexpected error when deleting file: %v", err) + } - // Ensure the file no longer exists - if _, err := s3Prv.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3Prv.bucket, Key: aws.String(fmt.Sprintf("%s/%s", s3Prv.dir, "test.txt"))}); err == nil { - t.Errorf("file should not exist after deleting: %v", err) + // Ensure the file no longer exists + if _, err := s3TS.provider.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3TS.provider.bucket, Key: aws.String(fmt.Sprintf("%s/%s", s3TS.provider.dir, "test.txt"))}); err == nil { + t.Errorf("file should not exist after deleting: %v", err) + } + }) } } @@ -98,25 +106,29 @@ func TestWriteAndDeleteFileInS3WithSubDir(t *testing.T) { t.Skip("Skipping S3 tests") } - filePath := filepath.Join("subdir", "test.txt") - // Copy a file into the workspace - if err := s3Prv.WriteFile(context.Background(), filePath, strings.NewReader("test"), WriteOptions{}); err != nil { - t.Fatalf("error getting file to write: %v", err) - } + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + filePath := filepath.Join("subdir", "test.txt") + // Copy a file into the workspace + if err := s3TS.provider.WriteFile(context.Background(), filePath, strings.NewReader("test"), WriteOptions{}); err != nil { + t.Fatalf("error getting file to write: %v", err) + } - // Ensure the file actually exists - if _, err := s3Prv.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3Prv.bucket, Key: aws.String(fmt.Sprintf("%s/%s", s3Prv.dir, filePath))}); err != nil { - t.Errorf("error when checking if file exists: %v", err) - } + // Ensure the file actually exists + if _, err := s3TS.provider.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3TS.provider.bucket, Key: aws.String(fmt.Sprintf("%s/%s", s3TS.provider.dir, filePath))}); err != nil { + t.Errorf("error when checking if file exists: %v", err) + } - // Delete the file - if err := s3Prv.DeleteFile(context.Background(), filePath); err != nil { - t.Errorf("unexpected error when deleting file: %v", err) - } + // Delete the file + if err := s3TS.provider.DeleteFile(context.Background(), filePath); err != nil { + t.Errorf("unexpected error when deleting file: %v", err) + } - // Ensure the file no longer exists - if _, err := s3Prv.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3Prv.bucket, Key: aws.String(fmt.Sprintf("%s/%s", s3Prv.dir, filePath))}); err == nil { - t.Errorf("file should not exist after deleting: %v", err) + // Ensure the file no longer exists + if _, err := s3TS.provider.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3TS.provider.bucket, Key: aws.String(fmt.Sprintf("%s/%s", s3TS.provider.dir, filePath))}); err == nil { + t.Errorf("file should not exist after deleting: %v", err) + } + }) } } @@ -125,36 +137,40 @@ func TestFileReadFromS3(t *testing.T) { t.Skip("Skipping S3 tests") } - if err := s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { - t.Fatalf("error getting file to write: %v", err) - } + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + if err := s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { + t.Fatalf("error getting file to write: %v", err) + } - readFile, err := s3Prv.OpenFile(context.Background(), "test.txt", OpenOptions{}) - if err != nil { - t.Errorf("unexpected error when reading file: %v", err) - } + readFile, err := s3TS.provider.OpenFile(context.Background(), "test.txt", OpenOptions{}) + if err != nil { + t.Errorf("unexpected error when reading file: %v", err) + } - content, err := io.ReadAll(readFile) - if err != nil { - t.Errorf("unexpected error when reading file: %v", err) - } + content, err := io.ReadAll(readFile) + if err != nil { + t.Errorf("unexpected error when reading file: %v", err) + } - if err = readFile.Close(); err != nil { - t.Errorf("error closing file: %v", err) - } + if err = readFile.Close(); err != nil { + t.Errorf("error closing file: %v", err) + } - if string(content) != "test" { - t.Errorf("unexpected content: %s", string(content)) - } + if string(content) != "test" { + t.Errorf("unexpected content: %s", string(content)) + } - // Delete the file - if err = s3Prv.DeleteFile(context.Background(), "test.txt"); err != nil { - t.Errorf("unexpected error when deleting file: %v", err) - } + // Delete the file + if err = s3TS.provider.DeleteFile(context.Background(), "test.txt"); err != nil { + t.Errorf("unexpected error when deleting file: %v", err) + } - // Deleting the file again should not throw an error - if err = s3Prv.DeleteFile(context.Background(), "test.txt"); err != nil { - t.Errorf("unexpected error when deleting file: %v", err) + // Deleting the file again should not throw an error + if err = s3TS.provider.DeleteFile(context.Background(), "test.txt"); err != nil { + t.Errorf("unexpected error when deleting file: %v", err) + } + }) } } @@ -163,45 +179,49 @@ func TestLsS3(t *testing.T) { t.Skip("Skipping S3 tests") } - // Write a bunch of files to the directory. They can be blank - for i := range 7 { - fileName := fmt.Sprintf("test%d.txt", i) - if err := s3Prv.WriteFile(context.Background(), fileName, strings.NewReader("test"), WriteOptions{}); err != nil { - t.Fatalf("error getting file to write: %v", err) - } + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + // Write a bunch of files to the directory. They can be blank + for i := range 7 { + fileName := fmt.Sprintf("test%d.txt", i) + if err := s3TS.provider.WriteFile(context.Background(), fileName, strings.NewReader("test"), WriteOptions{}); err != nil { + t.Fatalf("error getting file to write: %v", err) + } + + // deferring here is fine because these files shouldn't be deleted until the end of the test + t.Cleanup(func() { + err := s3TS.provider.DeleteFile(context.Background(), fileName) + if err != nil { + t.Errorf("unexpected error when deleting file %s: %v", fileName, err) + } + }) + } - // deferring here is fine because these files shouldn't be deleted until the end of the test - defer func() { - err := s3Prv.DeleteFile(context.Background(), fileName) + contents, err := s3TS.provider.Ls(context.Background(), "") if err != nil { - t.Errorf("unexpected error when deleting file %s: %v", fileName, err) + t.Fatalf("unexpected error when listing files: %v", err) } - }() - } - - contents, err := s3Prv.Ls(context.Background(), "") - if err != nil { - t.Fatalf("unexpected error when listing files: %v", err) - } - if len(contents) != 7 { - t.Errorf("unexpected number of files: %d", len(contents)) - } + if len(contents) != 7 { + t.Errorf("unexpected number of files: %d", len(contents)) + } - sort.Strings(contents) - if !reflect.DeepEqual( - contents, - []string{ - "test0.txt", - "test1.txt", - "test2.txt", - "test3.txt", - "test4.txt", - "test5.txt", - "test6.txt", - }, - ) { - t.Errorf("unexpected contents: %v", contents) + sort.Strings(contents) + if !reflect.DeepEqual( + contents, + []string{ + "test0.txt", + "test1.txt", + "test2.txt", + "test3.txt", + "test4.txt", + "test5.txt", + "test6.txt", + }, + ) { + t.Errorf("unexpected contents: %v", contents) + } + }) } } @@ -210,55 +230,59 @@ func TestLsWithSubDirsS3(t *testing.T) { t.Skip("Skipping S3 tests") } - defer func() { - err := s3Prv.RemoveAllWithPrefix(context.Background(), "testDir") - if err != nil { - t.Errorf("unexpected error when deleting file %s: %v", "testDir", err) - } - }() - - // Write a bunch of files to the directory. They can be blank - for i := range 7 { - fileName := fmt.Sprintf("test%d.txt", i) - if i >= 3 { - fileName = fmt.Sprintf("testDir/%s", fileName) - } - if err := s3Prv.WriteFile(context.Background(), fileName, strings.NewReader("test"), WriteOptions{}); err != nil { - t.Fatalf("error getting file to write: %v", err) - } - - // deferring here is fine because these files shouldn't be deleted until the end of the test - defer func() { - err := s3Prv.DeleteFile(context.Background(), fileName) - if err != nil { - t.Errorf("unexpected error when deleting file %s: %v", fileName, err) + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + defer func() { + err := s3TS.provider.RemoveAllWithPrefix(context.Background(), "testDir") + if err != nil { + t.Errorf("unexpected error when deleting file %s: %v", "testDir", err) + } + }() + + // Write a bunch of files to the directory. They can be blank + for i := range 7 { + fileName := fmt.Sprintf("test%d.txt", i) + if i >= 3 { + fileName = fmt.Sprintf("testDir/%s", fileName) + } + if err := s3TS.provider.WriteFile(context.Background(), fileName, strings.NewReader("test"), WriteOptions{}); err != nil { + t.Fatalf("error getting file to write: %v", err) + } + + // deferring here is fine because these files shouldn't be deleted until the end of the test + defer func() { + err := s3TS.provider.DeleteFile(context.Background(), fileName) + if err != nil { + t.Errorf("unexpected error when deleting file %s: %v", fileName, err) + } + }() } - }() - } - contents, err := s3Prv.Ls(context.Background(), "") - if err != nil { - t.Fatalf("unexpected error when listing files: %v", err) - } + contents, err := s3TS.provider.Ls(context.Background(), "") + if err != nil { + t.Fatalf("unexpected error when listing files: %v", err) + } - if len(contents) != 7 { - t.Errorf("unexpected number of children: %d", len(contents)) - } + if len(contents) != 7 { + t.Errorf("unexpected number of children: %d", len(contents)) + } - sort.Strings(contents) - if !reflect.DeepEqual( - contents, - []string{ - "test0.txt", - "test1.txt", - "test2.txt", - filepath.Join("testDir", "test3.txt"), - filepath.Join("testDir", "test4.txt"), - filepath.Join("testDir", "test5.txt"), - filepath.Join("testDir", "test6.txt"), - }, - ) { - t.Errorf("unexpected contents: %v", contents) + sort.Strings(contents) + if !reflect.DeepEqual( + contents, + []string{ + "test0.txt", + "test1.txt", + "test2.txt", + filepath.Join("testDir", "test3.txt"), + filepath.Join("testDir", "test4.txt"), + filepath.Join("testDir", "test5.txt"), + filepath.Join("testDir", "test6.txt"), + }, + ) { + t.Errorf("unexpected contents: %v", contents) + } + }) } } @@ -267,52 +291,56 @@ func TestLsWithPrefixS3(t *testing.T) { t.Skip("Skipping S3 tests") } - defer func() { - err := s3Prv.RemoveAllWithPrefix(context.Background(), "testDir") - if err != nil { - t.Errorf("unexpected error when deleting file %s: %v", "testDir", err) - } - }() - - // Write a bunch of files to the directory. They can be blank - for i := range 7 { - fileName := fmt.Sprintf("test%d.txt", i) - if i >= 3 { - fileName = fmt.Sprintf("testDir/%s", fileName) - } - if err := s3Prv.WriteFile(context.Background(), fileName, strings.NewReader("test"), WriteOptions{}); err != nil { - t.Fatalf("error getting file to write: %v", err) - } - - // deferring here is fine because these files shouldn't be deleted until the end of the test - defer func() { - err := s3Prv.DeleteFile(context.Background(), fileName) - if err != nil { - t.Errorf("unexpected error when deleting file %s: %v", fileName, err) + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + defer func() { + err := s3TS.provider.RemoveAllWithPrefix(context.Background(), "testDir") + if err != nil { + t.Errorf("unexpected error when deleting file %s: %v", "testDir", err) + } + }() + + // Write a bunch of files to the directory. They can be blank + for i := range 7 { + fileName := fmt.Sprintf("test%d.txt", i) + if i >= 3 { + fileName = fmt.Sprintf("testDir/%s", fileName) + } + if err := s3TS.provider.WriteFile(context.Background(), fileName, strings.NewReader("test"), WriteOptions{}); err != nil { + t.Fatalf("error getting file to write: %v", err) + } + + // deferring here is fine because these files shouldn't be deleted until the end of the test + defer func() { + err := s3TS.provider.DeleteFile(context.Background(), fileName) + if err != nil { + t.Errorf("unexpected error when deleting file %s: %v", fileName, err) + } + }() } - }() - } - contents, err := s3Prv.Ls(context.Background(), "testDir") - if err != nil { - t.Fatalf("unexpected error when listing files: %v", err) - } + contents, err := s3TS.provider.Ls(context.Background(), "testDir") + if err != nil { + t.Fatalf("unexpected error when listing files: %v", err) + } - if len(contents) != 4 { - t.Errorf("unexpected number of contents: %d", len(contents)) - } + if len(contents) != 4 { + t.Errorf("unexpected number of contents: %d", len(contents)) + } - sort.Strings(contents) - if !reflect.DeepEqual( - contents, - []string{ - "testDir/test3.txt", - "testDir/test4.txt", - "testDir/test5.txt", - "testDir/test6.txt", - }, - ) { - t.Errorf("unexpected contents: %v", contents) + sort.Strings(contents) + if !reflect.DeepEqual( + contents, + []string{ + "testDir/test3.txt", + "testDir/test4.txt", + "testDir/test5.txt", + "testDir/test6.txt", + }, + ) { + t.Errorf("unexpected contents: %v", contents) + } + }) } } @@ -321,49 +349,53 @@ func TestRemoveAllWithPrefixS3(t *testing.T) { t.Skip("Skipping S3 tests") } - // Write a bunch of files to the directory. They can be blank - for i := range 7 { - fileName := fmt.Sprintf("test%d.txt", i) - if i >= 3 { - fileName = fmt.Sprintf("testDir/%s", fileName) - } - if err := s3Prv.WriteFile(context.Background(), fileName, strings.NewReader("test"), WriteOptions{}); err != nil { - t.Fatalf("error getting file to write: %v", err) - } - - // deferring here is fine because these files shouldn't be deleted until the end of the test - defer func() { - err := s3Prv.DeleteFile(context.Background(), fileName) - if fnf := (*NotFoundError)(nil); err != nil && !errors.As(err, &fnf) { - t.Errorf("unexpected error when deleting file %s: %v", fileName, err) + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + // Write a bunch of files to the directory. They can be blank + for i := range 7 { + fileName := fmt.Sprintf("test%d.txt", i) + if i >= 3 { + fileName = fmt.Sprintf("testDir/%s", fileName) + } + if err := s3TS.provider.WriteFile(context.Background(), fileName, strings.NewReader("test"), WriteOptions{}); err != nil { + t.Fatalf("error getting file to write: %v", err) + } + + // deferring here is fine because these files shouldn't be deleted until the end of the test + defer func() { + err := s3TS.provider.DeleteFile(context.Background(), fileName) + if fnf := (*NotFoundError)(nil); err != nil && !errors.As(err, &fnf) { + t.Errorf("unexpected error when deleting file %s: %v", fileName, err) + } + }() } - }() - } - err := s3Prv.RemoveAllWithPrefix(context.Background(), "testDir") - if err != nil { - t.Errorf("unexpected error when deleting all with prefix testDir: %v", err) - } + err := s3TS.provider.RemoveAllWithPrefix(context.Background(), "testDir") + if err != nil { + t.Errorf("unexpected error when deleting all with prefix testDir: %v", err) + } - contents, err := s3Prv.Ls(context.Background(), "") - if err != nil { - t.Fatalf("unexpected error when listing files: %v", err) - } + contents, err := s3TS.provider.Ls(context.Background(), "") + if err != nil { + t.Fatalf("unexpected error when listing files: %v", err) + } - if len(contents) != 3 { - t.Errorf("unexpected number of children: %d", len(contents)) - } + if len(contents) != 3 { + t.Errorf("unexpected number of children: %d", len(contents)) + } - sort.Strings(contents) - if !reflect.DeepEqual( - contents, - []string{ - "test0.txt", - "test1.txt", - "test2.txt", - }, - ) { - t.Errorf("unexpected contents: %v", contents) + sort.Strings(contents) + if !reflect.DeepEqual( + contents, + []string{ + "test0.txt", + "test1.txt", + "test2.txt", + }, + ) { + t.Errorf("unexpected contents: %v", contents) + } + }) } } @@ -372,12 +404,16 @@ func TestOpeningFileDNENoErrorS3(t *testing.T) { t.Skip("Skipping S3 tests") } - var notFoundError *NotFoundError - if file, err := s3Prv.OpenFile(context.Background(), "test.txt", OpenOptions{}); err == nil { - _ = file.Close() - t.Errorf("expected error when deleting file that doesn't exist") - } else if !errors.As(err, ¬FoundError) { - t.Errorf("expected not found error when deleting file that doesn't exist") + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + var notFoundError *NotFoundError + if file, err := s3TS.provider.OpenFile(context.Background(), "test.txt", OpenOptions{}); err == nil { + _ = file.Close() + t.Errorf("expected error when deleting file that doesn't exist") + } else if !errors.As(err, ¬FoundError) { + t.Errorf("expected not found error when deleting file that doesn't exist") + } + }) } } @@ -386,105 +422,109 @@ func TestWriteEnsureRevisionS3(t *testing.T) { t.Skip("Skipping S3 tests") } - // Copy a file into the workspace - if err := s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { - t.Fatalf("error getting file to write: %v", err) - } - - // List revisions, there should be none - revisions, err := s3Prv.ListRevisions(context.Background(), "test.txt") - if err != nil { - t.Errorf("unexpected error when listing revisions: %v", err) - } - if len(revisions) != 0 { - t.Errorf("unexpected number of revisions: %d", len(revisions)) - } - - // Update the file - if err = s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test2"), WriteOptions{}); err != nil { - t.Errorf("error getting file to write: %v", err) - } - - // Ensure the file actually exists - obj, err := s3Prv.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3Prv.bucket, Key: aws.String(fmt.Sprintf("revisions/%s/%s", s3Prv.dir, "test.txt.1"))}) - if err != nil { - t.Errorf("error when checking if file exists: %v", err) - } - defer obj.Body.Close() - - // Now there should be one revision - revisions, err = s3Prv.ListRevisions(context.Background(), "test.txt") - if err != nil { - t.Errorf("unexpected error when listing revisions: %v", err) - } - if len(revisions) != 1 { - t.Errorf("unexpected number of revisions: %d", len(revisions)) - } else { - if revisions[0].WorkspaceID != s3TestingID { - t.Errorf("unexpected workspace id: %s", revisions[0].WorkspaceID) - } - if revisions[0].Size != aws.ToInt64(obj.ContentLength) { - t.Errorf("unexpected file size: %d", revisions[0].Size) - } - if revisions[0].Name != "test.txt" { - t.Errorf("unexpected file name: %s", revisions[0].Name) - } - if revisions[0].ModTime.Compare(aws.ToTime(obj.LastModified)) != 0 { - t.Errorf("unexpected file mod time: %s", revisions[0].ModTime) - } - - if revisions[0].RevisionID != "1" { - t.Errorf("unexpected revision id: %s", revisions[0].RevisionID) - } + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + // Copy a file into the workspace + if err := s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { + t.Fatalf("error getting file to write: %v", err) + } - // Get the revision and ensure that it has the correct content. - rev, err := s3Prv.GetRevision(context.Background(), "test.txt", revisions[0].RevisionID) - if err != nil { - t.Errorf("unexpected error when getting revision: %v", err) - } else { - defer rev.Close() - } + // List revisions, there should be none + revisions, err := s3TS.provider.ListRevisions(context.Background(), "test.txt") + if err != nil { + t.Errorf("unexpected error when listing revisions: %v", err) + } + if len(revisions) != 0 { + t.Errorf("unexpected number of revisions: %d", len(revisions)) + } - content, err := io.ReadAll(rev) - if err != nil { - t.Errorf("unexpected error when reading revision: %v", err) - } + // Update the file + if err = s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test2"), WriteOptions{}); err != nil { + t.Errorf("error getting file to write: %v", err) + } - if string(content) != "test" { - t.Errorf("unexpected content: %s", string(content)) - } + // Ensure the file actually exists + obj, err := s3TS.provider.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3TS.provider.bucket, Key: aws.String(fmt.Sprintf("revisions/%s/%s", s3TS.provider.dir, "test.txt.1"))}) + if err != nil { + t.Errorf("error when checking if file exists: %v", err) + } + defer obj.Body.Close() - revisionID, err := rev.GetRevisionID() - if err != nil { - t.Errorf("error getting revision: %v", err) - } - if revisionID != "1" { - t.Errorf("unexpected revision ID: %s", revisionID) - } - } + // Now there should be one revision + revisions, err = s3TS.provider.ListRevisions(context.Background(), "test.txt") + if err != nil { + t.Errorf("unexpected error when listing revisions: %v", err) + } + if len(revisions) != 1 { + t.Errorf("unexpected number of revisions: %d", len(revisions)) + } else { + if revisions[0].WorkspaceID != s3TS.testingID { + t.Errorf("unexpected workspace id: %s", revisions[0].WorkspaceID) + } + if revisions[0].Size != aws.ToInt64(obj.ContentLength) { + t.Errorf("unexpected file size: %d", revisions[0].Size) + } + if revisions[0].Name != "test.txt" { + t.Errorf("unexpected file name: %s", revisions[0].Name) + } + if revisions[0].ModTime.Compare(aws.ToTime(obj.LastModified)) != 0 { + t.Errorf("unexpected file mod time: %s", revisions[0].ModTime) + } + + if revisions[0].RevisionID != "1" { + t.Errorf("unexpected revision id: %s", revisions[0].RevisionID) + } + + // Get the revision and ensure that it has the correct content. + rev, err := s3TS.provider.GetRevision(context.Background(), "test.txt", revisions[0].RevisionID) + if err != nil { + t.Errorf("unexpected error when getting revision: %v", err) + } else { + defer rev.Close() + } + + content, err := io.ReadAll(rev) + if err != nil { + t.Errorf("unexpected error when reading revision: %v", err) + } + + if string(content) != "test" { + t.Errorf("unexpected content: %s", string(content)) + } + + revisionID, err := rev.GetRevisionID() + if err != nil { + t.Errorf("error getting revision: %v", err) + } + if revisionID != "1" { + t.Errorf("unexpected revision ID: %s", revisionID) + } + } - // Delete the file - if err = s3Prv.DeleteFile(context.Background(), "test.txt"); err != nil { - t.Errorf("unexpected error when deleting file: %v", err) - } + // Delete the file + if err = s3TS.provider.DeleteFile(context.Background(), "test.txt"); err != nil { + t.Errorf("unexpected error when deleting file: %v", err) + } - // Ensure the file no longer exists - if _, err = s3Prv.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3Prv.bucket, Key: aws.String(fmt.Sprintf("%s/%s", s3Prv.dir, "test.txt"))}); err == nil { - t.Errorf("file should not exist after deleting: %v", err) - } + // Ensure the file no longer exists + if _, err = s3TS.provider.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3TS.provider.bucket, Key: aws.String(fmt.Sprintf("%s/%s", s3TS.provider.dir, "test.txt"))}); err == nil { + t.Errorf("file should not exist after deleting: %v", err) + } - // Ensure the revision file no longer exists - if _, err = s3Prv.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3Prv.bucket, Key: aws.String(fmt.Sprintf("revisions/%s/%s", s3Prv.dir, "test.txt.1"))}); err == nil { - t.Errorf("file should not exist after deleting: %v", err) - } + // Ensure the revision file no longer exists + if _, err = s3TS.provider.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3TS.provider.bucket, Key: aws.String(fmt.Sprintf("revisions/%s/%s", s3TS.provider.dir, "test.txt.1"))}); err == nil { + t.Errorf("file should not exist after deleting: %v", err) + } - // Ensure the API returns no revisions for the file - revisions, err = s3Prv.ListRevisions(context.Background(), "test.txt") - if err != nil { - t.Errorf("unexpected error when listing revisions: %v", err) - } - if len(revisions) != 0 { - t.Errorf("unexpected number of revisions: %d", len(revisions)) + // Ensure the API returns no revisions for the file + revisions, err = s3TS.provider.ListRevisions(context.Background(), "test.txt") + if err != nil { + t.Errorf("unexpected error when listing revisions: %v", err) + } + if len(revisions) != 0 { + t.Errorf("unexpected number of revisions: %d", len(revisions)) + } + }) } } @@ -493,37 +533,41 @@ func TestWriteEnsureNoRevisionS3(t *testing.T) { t.Skip("Skipping S3 tests") } - // Copy a file into the workspace - if err := s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { - t.Fatalf("error getting file to write: %v", err) - } + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + // Copy a file into the workspace + if err := s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { + t.Fatalf("error getting file to write: %v", err) + } - // List revisions, there should be none - revisions, err := s3Prv.ListRevisions(context.Background(), "test.txt") - if err != nil { - t.Errorf("unexpected error when listing revisions: %v", err) - } - if len(revisions) != 0 { - t.Errorf("unexpected number of revisions: %d", len(revisions)) - } + // List revisions, there should be none + revisions, err := s3TS.provider.ListRevisions(context.Background(), "test.txt") + if err != nil { + t.Errorf("unexpected error when listing revisions: %v", err) + } + if len(revisions) != 0 { + t.Errorf("unexpected number of revisions: %d", len(revisions)) + } - // Update the file - if err = s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test2"), WriteOptions{CreateRevision: new(bool)}); err != nil { - t.Errorf("error getting file to write: %v", err) - } + // Update the file + if err = s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test2"), WriteOptions{CreateRevision: new(bool)}); err != nil { + t.Errorf("error getting file to write: %v", err) + } - // Now there should still be no revision - revisions, err = s3Prv.ListRevisions(context.Background(), "test.txt") - if err != nil { - t.Errorf("unexpected error when listing revisions: %v", err) - } - if len(revisions) != 0 { - t.Errorf("unexpected number of revisions: %d", len(revisions)) - } + // Now there should still be no revision + revisions, err = s3TS.provider.ListRevisions(context.Background(), "test.txt") + if err != nil { + t.Errorf("unexpected error when listing revisions: %v", err) + } + if len(revisions) != 0 { + t.Errorf("unexpected number of revisions: %d", len(revisions)) + } - // Delete the file - if err = s3Prv.DeleteFile(context.Background(), "test.txt"); err != nil { - t.Errorf("unexpected error when deleting file: %v", err) + // Delete the file + if err = s3TS.provider.DeleteFile(context.Background(), "test.txt"); err != nil { + t.Errorf("unexpected error when deleting file: %v", err) + } + }) } } @@ -532,93 +576,97 @@ func TestWriteEnsureConflictS3(t *testing.T) { t.Skip("Skipping S3 tests") } - // Copy a file into the workspace - if err := s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { - t.Fatalf("error getting file to write: %v", err) - } + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + // Copy a file into the workspace + if err := s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { + t.Fatalf("error getting file to write: %v", err) + } - // List revisions, there should be none - revisions, err := s3Prv.ListRevisions(context.Background(), "test.txt") - if err != nil { - t.Errorf("unexpected error when listing revisions: %v", err) - } - if len(revisions) != 0 { - t.Errorf("unexpected number of revisions: %d", len(revisions)) - } + // List revisions, there should be none + revisions, err := s3TS.provider.ListRevisions(context.Background(), "test.txt") + if err != nil { + t.Errorf("unexpected error when listing revisions: %v", err) + } + if len(revisions) != 0 { + t.Errorf("unexpected number of revisions: %d", len(revisions)) + } - ce := (*ConflictError)(nil) - // Trying to update the file with a non-zero revision ID should fail with a conflict error. - if err = s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test2"), WriteOptions{LatestRevisionID: "1"}); err == nil || !errors.As(err, &ce) { - t.Errorf("expected error when first updating file non-zero revision ID: %v", err) - } + ce := (*ConflictError)(nil) + // Trying to update the file with a non-zero revision ID should fail with a conflict error. + if err = s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test2"), WriteOptions{LatestRevisionID: "1"}); err == nil || !errors.As(err, &ce) { + t.Errorf("expected error when first updating file non-zero revision ID: %v", err) + } - // Also, using -1 for the revision ID should also fail because that is the same as "only write if the file doesn't exist" - if err = s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test2"), WriteOptions{LatestRevisionID: "-1"}); err == nil || !errors.As(err, &ce) { - t.Errorf("expected error when first updating file non-zero revision ID: %v", err) - } + // Also, using -1 for the revision ID should also fail because that is the same as "only write if the file doesn't exist" + if err = s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test2"), WriteOptions{LatestRevisionID: "-1"}); err == nil || !errors.As(err, &ce) { + t.Errorf("expected error when first updating file non-zero revision ID: %v", err) + } - // Update the file - if err = s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test2"), WriteOptions{}); err != nil { - t.Errorf("error getting file to write: %v", err) - } + // Update the file + if err = s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test2"), WriteOptions{}); err != nil { + t.Errorf("error getting file to write: %v", err) + } - // Now there should be one revision - revisions, err = s3Prv.ListRevisions(context.Background(), "test.txt") - if err != nil { - t.Errorf("unexpected error when listing revisions: %v", err) - } - if len(revisions) != 1 { - t.Errorf("unexpected number of revisions: %d", len(revisions)) - } + // Now there should be one revision + revisions, err = s3TS.provider.ListRevisions(context.Background(), "test.txt") + if err != nil { + t.Errorf("unexpected error when listing revisions: %v", err) + } + if len(revisions) != 1 { + t.Errorf("unexpected number of revisions: %d", len(revisions)) + } - // Update the file again - if err = s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test3"), WriteOptions{LatestRevisionID: revisions[0].RevisionID}); err != nil { - t.Errorf("error getting file to write: %v", err) - } + // Update the file again + if err = s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test3"), WriteOptions{LatestRevisionID: revisions[0].RevisionID}); err != nil { + t.Errorf("error getting file to write: %v", err) + } - // Now there should be two revisions - revisions, err = s3Prv.ListRevisions(context.Background(), "test.txt") - if err != nil { - t.Errorf("unexpected error when listing revisions: %v", err) - } - if len(revisions) != 2 { - t.Errorf("unexpected number of revisions: %d", len(revisions)) - } + // Now there should be two revisions + revisions, err = s3TS.provider.ListRevisions(context.Background(), "test.txt") + if err != nil { + t.Errorf("unexpected error when listing revisions: %v", err) + } + if len(revisions) != 2 { + t.Errorf("unexpected number of revisions: %d", len(revisions)) + } - // Trying to update the file again with the same revision ID should fail with a conflict error. - if err = s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test4"), WriteOptions{LatestRevisionID: revisions[0].RevisionID}); err == nil || !errors.As(err, &ce) { - t.Errorf("expected error when updating file with same revision ID: %v", err) - } + // Trying to update the file again with the same revision ID should fail with a conflict error. + if err = s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test4"), WriteOptions{LatestRevisionID: revisions[0].RevisionID}); err == nil || !errors.As(err, &ce) { + t.Errorf("expected error when updating file with same revision ID: %v", err) + } - latestRevisionID := revisions[1].RevisionID - // Delete the most recent revision - if err = s3Prv.DeleteRevision(context.Background(), "test.txt", latestRevisionID); err != nil { - t.Errorf("error deleting revision: %v", err) - } + latestRevisionID := revisions[1].RevisionID + // Delete the most recent revision + if err = s3TS.provider.DeleteRevision(context.Background(), "test.txt", latestRevisionID); err != nil { + t.Errorf("error deleting revision: %v", err) + } - // Now there should be one revision - revisions, err = s3Prv.ListRevisions(context.Background(), "test.txt") - if err != nil { - t.Errorf("unexpected error when listing revisions: %v", err) - } - if len(revisions) != 1 { - t.Errorf("unexpected number of revisions: %d", len(revisions)) - } + // Now there should be one revision + revisions, err = s3TS.provider.ListRevisions(context.Background(), "test.txt") + if err != nil { + t.Errorf("unexpected error when listing revisions: %v", err) + } + if len(revisions) != 1 { + t.Errorf("unexpected number of revisions: %d", len(revisions)) + } - // We cannot update the file with this revision ID - ce = (*ConflictError)(nil) - if err = s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test5"), WriteOptions{LatestRevisionID: revisions[0].RevisionID}); err == nil || !errors.As(err, &ce) { - t.Errorf("expected error when updating file with zero revision ID: %v", err) - } + // We cannot update the file with this revision ID + ce = (*ConflictError)(nil) + if err = s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test5"), WriteOptions{LatestRevisionID: revisions[0].RevisionID}); err == nil || !errors.As(err, &ce) { + t.Errorf("expected error when updating file with zero revision ID: %v", err) + } - // Ensure that we can still create a new revision - if err = s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test5"), WriteOptions{LatestRevisionID: latestRevisionID}); err != nil { - t.Errorf("error getting file to write: %v", err) - } + // Ensure that we can still create a new revision + if err = s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test5"), WriteOptions{LatestRevisionID: latestRevisionID}); err != nil { + t.Errorf("error getting file to write: %v", err) + } - // Delete the file - if err = s3Prv.DeleteFile(context.Background(), "test.txt"); err != nil { - t.Errorf("error removing file: %v", err) + // Delete the file + if err = s3TS.provider.DeleteFile(context.Background(), "test.txt"); err != nil { + t.Errorf("error removing file: %v", err) + } + }) } } @@ -627,79 +675,83 @@ func TestReadFileWithRevisionS3(t *testing.T) { t.Skip("Skipping S3 tests") } - // Copy a file into the workspace - if err := s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { - t.Fatalf("error getting file to write: %v", err) - } + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + // Copy a file into the workspace + if err := s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { + t.Fatalf("error getting file to write: %v", err) + } - // Read the file - f, err := s3Prv.OpenFile(context.Background(), "test.txt", OpenOptions{WithLatestRevisionID: true}) - if err != nil { - t.Errorf("error reading file: %v", err) - } + // Read the file + f, err := s3TS.provider.OpenFile(context.Background(), "test.txt", OpenOptions{WithLatestRevisionID: true}) + if err != nil { + t.Errorf("error reading file: %v", err) + } - // Read the file contents - data, err := io.ReadAll(f) - if err != nil { - t.Errorf("error reading file contents: %v", err) - } + // Read the file contents + data, err := io.ReadAll(f) + if err != nil { + t.Errorf("error reading file contents: %v", err) + } - // Close the file - if err := f.Close(); err != nil { - t.Errorf("error closing file: %v", err) - } + // Close the file + if err := f.Close(); err != nil { + t.Errorf("error closing file: %v", err) + } - if string(data) != "test" { - t.Errorf("unexpected file contents: %s", string(data)) - } + if string(data) != "test" { + t.Errorf("unexpected file contents: %s", string(data)) + } - // Ensure that the revision is set and correct. - revisionID, err := f.GetRevisionID() - if err != nil { - t.Errorf("error getting revision: %v", err) - } - if revisionID != "0" { - t.Errorf("unexpected revision ID: %s", revisionID) - } + // Ensure that the revision is set and correct. + revisionID, err := f.GetRevisionID() + if err != nil { + t.Errorf("error getting revision: %v", err) + } + if revisionID != "0" { + t.Errorf("unexpected revision ID: %s", revisionID) + } - // Update the file - if err = s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test2"), WriteOptions{LatestRevisionID: "0"}); err != nil { - t.Errorf("error getting file to write: %v", err) - } + // Update the file + if err = s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test2"), WriteOptions{LatestRevisionID: "0"}); err != nil { + t.Errorf("error getting file to write: %v", err) + } - // Read the file - f, err = s3Prv.OpenFile(context.Background(), "test.txt", OpenOptions{WithLatestRevisionID: true}) - if err != nil { - t.Errorf("error reading file: %v", err) - } + // Read the file + f, err = s3TS.provider.OpenFile(context.Background(), "test.txt", OpenOptions{WithLatestRevisionID: true}) + if err != nil { + t.Errorf("error reading file: %v", err) + } - // Read the file contents - data, err = io.ReadAll(f) - if err != nil { - t.Errorf("error reading file contents: %v", err) - } + // Read the file contents + data, err = io.ReadAll(f) + if err != nil { + t.Errorf("error reading file contents: %v", err) + } - // Close the file - if err := f.Close(); err != nil { - t.Errorf("error closing file: %v", err) - } + // Close the file + if err := f.Close(); err != nil { + t.Errorf("error closing file: %v", err) + } - if string(data) != "test2" { - t.Errorf("unexpected file contents: %s", string(data)) - } + if string(data) != "test2" { + t.Errorf("unexpected file contents: %s", string(data)) + } - // Get the revision ID - revisionID, err = f.GetRevisionID() - if err != nil { - t.Errorf("error getting revision: %v", err) - } - if revisionID != "1" { - t.Errorf("unexpected revision ID: %s", revisionID) - } + // Get the revision ID + revisionID, err = f.GetRevisionID() + if err != nil { + t.Errorf("error getting revision: %v", err) + } + if revisionID != "1" { + t.Errorf("unexpected revision ID: %s", revisionID) + } - // Delete the file - if err = s3Prv.DeleteFile(context.Background(), "test.txt"); err != nil { - t.Errorf("error removing file: %v", err) + // Delete the file + if err = s3TS.provider.DeleteFile(context.Background(), "test.txt"); err != nil { + t.Errorf("error removing file: %v", err) + } + }) } } @@ -708,80 +760,84 @@ func TestDeleteRevisionS3(t *testing.T) { t.Skip("Skipping S3 tests") } - // Copy a file into the workspace - if err := s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { - t.Fatalf("error getting file to write: %v", err) - } + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + // Copy a file into the workspace + if err := s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { + t.Fatalf("error getting file to write: %v", err) + } - // List revisions, there should be none - revisions, err := s3Prv.ListRevisions(context.Background(), "test.txt") - if err != nil { - t.Errorf("unexpected error when listing revisions: %v", err) - } - if len(revisions) != 0 { - t.Errorf("unexpected number of revisions: %d", len(revisions)) - } + // List revisions, there should be none + revisions, err := s3TS.provider.ListRevisions(context.Background(), "test.txt") + if err != nil { + t.Errorf("unexpected error when listing revisions: %v", err) + } + if len(revisions) != 0 { + t.Errorf("unexpected number of revisions: %d", len(revisions)) + } - // Update the file - if err = s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test2"), WriteOptions{}); err != nil { - t.Errorf("error getting file to write: %v", err) - } + // Update the file + if err = s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test2"), WriteOptions{}); err != nil { + t.Errorf("error getting file to write: %v", err) + } - // Now there should be one revision - revisions, err = s3Prv.ListRevisions(context.Background(), "test.txt") - if err != nil { - t.Errorf("unexpected error when listing revisions: %v", err) - } - if len(revisions) != 1 { - t.Errorf("unexpected number of revisions: %d", len(revisions)) - } + // Now there should be one revision + revisions, err = s3TS.provider.ListRevisions(context.Background(), "test.txt") + if err != nil { + t.Errorf("unexpected error when listing revisions: %v", err) + } + if len(revisions) != 1 { + t.Errorf("unexpected number of revisions: %d", len(revisions)) + } - // Update the file - if err = s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test3"), WriteOptions{}); err != nil { - t.Errorf("error getting file to write: %v", err) - } + // Update the file + if err = s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test3"), WriteOptions{}); err != nil { + t.Errorf("error getting file to write: %v", err) + } - // Now there should be two revisions - revisions, err = s3Prv.ListRevisions(context.Background(), "test.txt") - if err != nil { - t.Errorf("unexpected error when listing revisions: %v", err) - } - if len(revisions) != 2 { - t.Errorf("unexpected number of revisions: %d", len(revisions)) - } + // Now there should be two revisions + revisions, err = s3TS.provider.ListRevisions(context.Background(), "test.txt") + if err != nil { + t.Errorf("unexpected error when listing revisions: %v", err) + } + if len(revisions) != 2 { + t.Errorf("unexpected number of revisions: %d", len(revisions)) + } - // Delete the first revision - if err = s3Prv.DeleteRevision(context.Background(), "test.txt", "1"); err != nil { - t.Errorf("unexpected error when deleting revision: %v", err) - } + // Delete the first revision + if err = s3TS.provider.DeleteRevision(context.Background(), "test.txt", "1"); err != nil { + t.Errorf("unexpected error when deleting revision: %v", err) + } - // Now there should be one revision - revisions, err = s3Prv.ListRevisions(context.Background(), "test.txt") - if err != nil { - t.Errorf("unexpected error when listing revisions: %v", err) - } - if len(revisions) != 1 || revisions[0].RevisionID != "2" { - t.Errorf("unexpected number of revisions: %d", len(revisions)) - } + // Now there should be one revision + revisions, err = s3TS.provider.ListRevisions(context.Background(), "test.txt") + if err != nil { + t.Errorf("unexpected error when listing revisions: %v", err) + } + if len(revisions) != 1 || revisions[0].RevisionID != "2" { + t.Errorf("unexpected number of revisions: %d", len(revisions)) + } - // Deleting the revision again should not produce an error. - if err = s3Prv.DeleteRevision(context.Background(), "test.txt", "1"); err != nil { - t.Errorf("unexpected error when deleting revision: %v", err) - } + // Deleting the revision again should not produce an error. + if err = s3TS.provider.DeleteRevision(context.Background(), "test.txt", "1"); err != nil { + t.Errorf("unexpected error when deleting revision: %v", err) + } - // Delete the file - if err = s3Prv.DeleteFile(context.Background(), "test.txt"); err != nil { - t.Errorf("unexpected error when deleting file: %v", err) - } + // Delete the file + if err = s3TS.provider.DeleteFile(context.Background(), "test.txt"); err != nil { + t.Errorf("unexpected error when deleting file: %v", err) + } - // Ensure the file no longer exists - if _, err := s3Prv.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3Prv.bucket, Key: aws.String(fmt.Sprintf("%s/%s", s3Prv.dir, "test.txt"))}); err == nil { - t.Errorf("file should not exist after deleting: %v", err) - } + // Ensure the file no longer exists + if _, err := s3TS.provider.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3TS.provider.bucket, Key: aws.String(fmt.Sprintf("%s/%s", s3TS.provider.dir, "test.txt"))}); err == nil { + t.Errorf("file should not exist after deleting: %v", err) + } - // Ensure the revision file no longer exists - if _, err := s3Prv.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3Prv.bucket, Key: aws.String(fmt.Sprintf("%s/%s", s3Prv.dir, "test.txt.2"))}); err == nil { - t.Errorf("file should not exist after deleting: %v", err) + // Ensure the revision file no longer exists + if _, err := s3TS.provider.client.GetObject(context.Background(), &s3.GetObjectInput{Bucket: &s3TS.provider.bucket, Key: aws.String(fmt.Sprintf("%s/%s", s3TS.provider.dir, "test.txt.2"))}); err == nil { + t.Errorf("file should not exist after deleting: %v", err) + } + }) } } @@ -790,9 +846,13 @@ func TestNoCreateRevisionsClientS3(t *testing.T) { t.Skip("Skipping S3 tests") } - _, err := s3Factory.New(fmt.Sprintf("%s://%s/%s", S3Provider, os.Getenv("WORKSPACE_PROVIDER_S3_BUCKET"), revisionsDir)) - if err == nil { - t.Errorf("expected error when creating client for revisions dir") + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + _, err := s3TS.factory.New(fmt.Sprintf("%s://%s/%s", S3Provider, os.Getenv("WORKSPACE_PROVIDER_S3_BUCKET"), revisionsDir)) + if err == nil { + t.Errorf("expected error when creating client for revisions dir") + } + }) } } @@ -801,42 +861,46 @@ func TestStatFileS3(t *testing.T) { t.Skip("Skipping S3 tests") } - // Copy a file into the workspace - if err := s3Prv.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { - t.Fatalf("error getting file to write: %v", err) - } + for _, s3TS := range s3TestSetups { + t.Run(s3TS.name, func(t *testing.T) { + // Copy a file into the workspace + if err := s3TS.provider.WriteFile(context.Background(), "test.txt", strings.NewReader("test"), WriteOptions{}); err != nil { + t.Fatalf("error getting file to write: %v", err) + } - // Stat the file - providerStat, err := s3Prv.StatFile(context.Background(), "test.txt", StatOptions{}) - if err != nil { - t.Errorf("unexpected error when statting file: %v", err) - } + // Stat the file + providerStat, err := s3TS.provider.StatFile(context.Background(), "test.txt", StatOptions{}) + if err != nil { + t.Errorf("unexpected error when statting file: %v", err) + } - if providerStat.WorkspaceID != s3TestingID { - t.Errorf("unexpected workspace id: %s", providerStat.WorkspaceID) - } - if providerStat.Size != 4 { - t.Errorf("unexpected file size: %d", providerStat.Size) - } - if providerStat.Name != "test.txt" { - t.Errorf("unexpected file name: %s", providerStat.Name) - } - if providerStat.ModTime.IsZero() { - t.Errorf("unexpected file mod time: %s", providerStat.ModTime) - } - if _, err := providerStat.GetRevisionID(); !errors.Is(err, RevisionNotRequestedError) { - t.Errorf("unexpected error when revision not requested: %v", err) - } + if providerStat.WorkspaceID != s3TS.testingID { + t.Errorf("unexpected workspace id: %s", providerStat.WorkspaceID) + } + if providerStat.Size != 4 { + t.Errorf("unexpected file size: %d", providerStat.Size) + } + if providerStat.Name != "test.txt" { + t.Errorf("unexpected file name: %s", providerStat.Name) + } + if providerStat.ModTime.IsZero() { + t.Errorf("unexpected file mod time: %s", providerStat.ModTime) + } + if _, err := providerStat.GetRevisionID(); !errors.Is(err, RevisionNotRequestedError) { + t.Errorf("unexpected error when revision not requested: %v", err) + } - // Stat the file with revision - providerStat, err = s3Prv.StatFile(context.Background(), "test.txt", StatOptions{WithLatestRevisionID: true}) - if err != nil { - t.Errorf("unexpected error when statting file: %v", err) - } + // Stat the file with revision + providerStat, err = s3TS.provider.StatFile(context.Background(), "test.txt", StatOptions{WithLatestRevisionID: true}) + if err != nil { + t.Errorf("unexpected error when statting file: %v", err) + } - if rev, err := providerStat.GetRevisionID(); err != nil { - t.Errorf("unexpected error when revision not requested: %v", err) - } else if rev != "0" { - t.Errorf("unexpected revision id when revision requested: %s", rev) + if rev, err := providerStat.GetRevisionID(); err != nil { + t.Errorf("unexpected error when revision not requested: %v", err) + } else if rev != "0" { + t.Errorf("unexpected revision id when revision requested: %s", rev) + } + }) } }