From 8ffff241fe30b3523aadf97f5664dd24f4457f63 Mon Sep 17 00:00:00 2001 From: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com> Date: Wed, 26 Jul 2023 15:03:10 +0200 Subject: [PATCH] Add TestAcc prefix to filer test and fix any failing tests (#611) ## Changes Fs integration tests were not running on our nightlies before because the nightlies only run tests with the `TestAcc` prefix. A couple of them were also broken! This PR fixes the tests and adds the prefix to all fs integration tests. As a followup we can automate the check for this prefix. ## Tested Fs tests are green and pass on both azure and aws --- internal/fs_cat_test.go | 10 +++++----- internal/fs_ls_test.go | 14 +++++++------- internal/{mkdir_test.go => fs_mkdir_test.go} | 13 ++++++++----- internal/{rm_test.go => fs_rm_test.go} | 20 ++++++++++---------- 4 files changed, 30 insertions(+), 27 deletions(-) rename internal/{mkdir_test.go => fs_mkdir_test.go} (85%) rename internal/{rm_test.go => fs_rm_test.go} (86%) diff --git a/internal/fs_cat_test.go b/internal/fs_cat_test.go index 5d6952f4..f3c8e59c 100644 --- a/internal/fs_cat_test.go +++ b/internal/fs_cat_test.go @@ -13,7 +13,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestFsCatForDbfs(t *testing.T) { +func TestAccFsCatForDbfs(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx := context.Background() @@ -33,21 +33,21 @@ func TestFsCatForDbfs(t *testing.T) { assert.Equal(t, "abc", stdout.String()) } -func TestFsCatForDbfsOnNonExistentFile(t *testing.T) { +func TestAccFsCatForDbfsOnNonExistentFile(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) _, _, err := RequireErrorRun(t, "fs", "cat", "dbfs:/non-existent-file") assert.ErrorIs(t, err, fs.ErrNotExist) } -func TestFsCatForDbfsInvalidScheme(t *testing.T) { +func TestAccFsCatForDbfsInvalidScheme(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) _, _, err := RequireErrorRun(t, "fs", "cat", "dab:/non-existent-file") - assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): dab:/non-existent-file") + assert.ErrorContains(t, err, "invalid scheme: dab") } -func TestFsCatDoesNotSupportOutputModeJson(t *testing.T) { +func TestAccFsCatDoesNotSupportOutputModeJson(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx := context.Background() diff --git a/internal/fs_ls_test.go b/internal/fs_ls_test.go index 885fc31f..d2181728 100644 --- a/internal/fs_ls_test.go +++ b/internal/fs_ls_test.go @@ -16,7 +16,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestFsLsForDbfs(t *testing.T) { +func TestAccFsLsForDbfs(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx := context.Background() @@ -51,7 +51,7 @@ func TestFsLsForDbfs(t *testing.T) { assert.Equal(t, float64(3), parsedStdout[1]["size"]) } -func TestFsLsForDbfsWithAbsolutePaths(t *testing.T) { +func TestAccFsLsForDbfsWithAbsolutePaths(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx := context.Background() @@ -87,7 +87,7 @@ func TestFsLsForDbfsWithAbsolutePaths(t *testing.T) { assert.Equal(t, float64(3), parsedStdout[1]["size"]) } -func TestFsLsForDbfsOnFile(t *testing.T) { +func TestAccFsLsForDbfsOnFile(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx := context.Background() @@ -108,7 +108,7 @@ func TestFsLsForDbfsOnFile(t *testing.T) { assert.Regexp(t, regexp.MustCompile("not a directory: .*/a/hello.txt"), err.Error()) } -func TestFsLsForDbfsOnEmptyDir(t *testing.T) { +func TestAccFsLsForDbfsOnEmptyDir(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) w, err := databricks.NewWorkspaceClient() @@ -126,16 +126,16 @@ func TestFsLsForDbfsOnEmptyDir(t *testing.T) { assert.Equal(t, 0, len(parsedStdout)) } -func TestFsLsForDbfsForNonexistingDir(t *testing.T) { +func TestAccFsLsForDbfsForNonexistingDir(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) _, _, err := RequireErrorRun(t, "fs", "ls", "dbfs:/john-cena", "--output=json") assert.ErrorIs(t, err, fs.ErrNotExist) } -func TestFsLsWithoutScheme(t *testing.T) { +func TestAccFsLsWithoutScheme(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) _, _, err := RequireErrorRun(t, "fs", "ls", "/ray-mysterio", "--output=json") - assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): /ray-mysterio") + assert.ErrorIs(t, err, fs.ErrNotExist) } diff --git a/internal/mkdir_test.go b/internal/fs_mkdir_test.go similarity index 85% rename from internal/mkdir_test.go rename to internal/fs_mkdir_test.go index 7c96e63b..137750e2 100644 --- a/internal/mkdir_test.go +++ b/internal/fs_mkdir_test.go @@ -3,6 +3,7 @@ package internal import ( "context" "path" + "regexp" "strings" "testing" @@ -12,7 +13,7 @@ import ( "github.com/stretchr/testify/require" ) -func TesFsMkdirCreatesDirectory(t *testing.T) { +func TestAccFsMkdirCreatesDirectory(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx := context.Background() @@ -36,7 +37,7 @@ func TesFsMkdirCreatesDirectory(t *testing.T) { assert.Equal(t, true, info.IsDir()) } -func TestFsMkdirCreatesMultipleDirectories(t *testing.T) { +func TestAccFsMkdirCreatesMultipleDirectories(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx := context.Background() @@ -72,7 +73,7 @@ func TestFsMkdirCreatesMultipleDirectories(t *testing.T) { assert.Equal(t, true, infoC.IsDir()) } -func TestFsMkdirWhenDirectoryAlreadyExists(t *testing.T) { +func TestAccFsMkdirWhenDirectoryAlreadyExists(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx := context.Background() @@ -93,7 +94,7 @@ func TestFsMkdirWhenDirectoryAlreadyExists(t *testing.T) { assert.Equal(t, "", stdout.String()) } -func TestFsMkdirWhenFileExistsAtPath(t *testing.T) { +func TestAccFsMkdirWhenFileExistsAtPath(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx := context.Background() @@ -110,5 +111,7 @@ func TestFsMkdirWhenFileExistsAtPath(t *testing.T) { // assert run fails _, _, err = RequireErrorRun(t, "fs", "mkdir", "dbfs:"+path.Join(tmpDir, "hello")) - assert.ErrorContains(t, err, "Cannot create directory") + // Different backends return different errors (for example: file in s3 vs dbfs) + regex := regexp.MustCompile(`^Path is a file: .*$|^Cannot create directory .* because .* is an existing file`) + assert.Regexp(t, regex, err.Error()) } diff --git a/internal/rm_test.go b/internal/fs_rm_test.go similarity index 86% rename from internal/rm_test.go rename to internal/fs_rm_test.go index dd6a2859..1bee06c7 100644 --- a/internal/rm_test.go +++ b/internal/fs_rm_test.go @@ -13,7 +13,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestFsRmForFile(t *testing.T) { +func TestAccFsRmForFile(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx := context.Background() @@ -45,7 +45,7 @@ func TestFsRmForFile(t *testing.T) { assert.ErrorIs(t, err, fs.ErrNotExist) } -func TestFsRmForEmptyDirectory(t *testing.T) { +func TestAccFsRmForEmptyDirectory(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx := context.Background() @@ -77,7 +77,7 @@ func TestFsRmForEmptyDirectory(t *testing.T) { assert.ErrorIs(t, err, fs.ErrNotExist) } -func TestFsRmForNonEmptyDirectory(t *testing.T) { +func TestAccFsRmForNonEmptyDirectory(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx := context.Background() @@ -101,19 +101,19 @@ func TestFsRmForNonEmptyDirectory(t *testing.T) { // Run rm command _, _, err = RequireErrorRun(t, "fs", "rm", "dbfs:"+path.Join(tmpDir, "avacado")) - assert.ErrorContains(t, err, "Non-recursive delete of non-empty directory") + assert.ErrorIs(t, err, fs.ErrInvalid) + assert.ErrorContains(t, err, "directory not empty") } -func TestFsRmForNonExistentFile(t *testing.T) { +func TestAccFsRmForNonExistentFile(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) - // No error is returned on command run - stdout, stderr := RequireSuccessfulRun(t, "fs", "rm", "dbfs:/does-not-exist") - assert.Equal(t, "", stderr.String()) - assert.Equal(t, "", stdout.String()) + // Expect error if file does not exist + _, _, err := RequireErrorRun(t, "fs", "rm", "dbfs:/does-not-exist") + assert.ErrorIs(t, err, fs.ErrNotExist) } -func TestFsRmForNonEmptyDirectoryWithRecursiveFlag(t *testing.T) { +func TestAccFsRmForNonEmptyDirectoryWithRecursiveFlag(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx := context.Background()