From 6e02c5fb65b975e8e8615a169b6590976d51d89a Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 10 Mar 2025 22:02:09 +0100 Subject: [PATCH 1/4] Add CloudLong config setting --- acceptance/acceptance_test.go | 46 ++++++++++++++++++++++++++--------- acceptance/config_test.go | 4 +++ 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index f0feb7b399..8b4d42456a 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -220,17 +220,39 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont } cloudEnv := os.Getenv("CLOUD_ENV") - if !isTruePtr(config.Local) && cloudEnv == "" { - t.Skipf("Disabled via Local setting in %s (CLOUD_ENV=%s)", configPath, cloudEnv) - } + isRunningOnCloud := cloudEnv != "" + tailOutput := Tail - if !isTruePtr(config.Cloud) && cloudEnv != "" { - t.Skipf("Disabled via Cloud setting in %s (CLOUD_ENV=%s)", configPath, cloudEnv) - } + if isRunningOnCloud { + if isTruePtr(config.CloudLong) { + if testing.Short() { + t.Skipf("Disabled via CloudLong setting in %s (CLOUD_ENV=%s, Short=%v)", configPath, cloudEnv, testing.Short()) + } + + if testing.Verbose() { + // Combination of CloudLong and -v auto-enables -tail + tailOutput = true + } + } + + isCloudEnabled := isTruePtr(config.Cloud) || isTruePtr(config.CloudLong) + if !isCloudEnabled { + t.Skipf("Disabled via Cloud/CloudLong setting in %s (CLOUD_ENV=%s, Cloud=%v, CloudLong=%v)", + configPath, + cloudEnv, + isTruePtr(config.Cloud), + isTruePtr(config.CloudLong), + ) + } - if cloudEnv != "" { if isTruePtr(config.RequiresUnityCatalog) && os.Getenv("TEST_METASTORE_ID") == "" { - t.Skipf("Skipping on non-UC workspaces") + t.Skipf("Disable via RequiresUnityCatalog setting in %s (TEST_METASTORE_ID=%s)", configPath, os.Getenv("TEST_METASTORE_ID")) + } + + } else { + // Local run + if !isTruePtr(config.Local) { + t.Skipf("Disabled via Local setting in %s (CLOUD_ENV=%s)", configPath, cloudEnv) } } @@ -267,7 +289,7 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont // specifies a custom server stubs. var server *testserver.Server - if cloudEnv == "" { + if !isRunningOnCloud { // Start a new server for this test if either: // 1. A custom server spec is defined in the test configuration. // 2. The test is configured to record requests and assert on them. We need @@ -374,7 +396,7 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont require.NoError(t, err) defer out.Close() - err = runWithLog(t, cmd, out) + err = runWithLog(t, cmd, out, tailOutput) // Include exit code in output (if non-zero) formatOutput(out, err) @@ -725,7 +747,7 @@ func isTruePtr(value *bool) bool { return value != nil && *value } -func runWithLog(t *testing.T, cmd *exec.Cmd, out *os.File) error { +func runWithLog(t *testing.T, cmd *exec.Cmd, out *os.File, tail bool) error { r, w := io.Pipe() cmd.Stdout = w cmd.Stderr = w @@ -743,7 +765,7 @@ func runWithLog(t *testing.T, cmd *exec.Cmd, out *os.File) error { reader := bufio.NewReader(r) for { line, err := reader.ReadString('\n') - if Tail { + if tail { msg := strings.TrimRight(line, "\n") if len(msg) > 0 { d := time.Since(start) diff --git a/acceptance/config_test.go b/acceptance/config_test.go index fa236275b4..488f1db8a5 100644 --- a/acceptance/config_test.go +++ b/acceptance/config_test.go @@ -31,6 +31,10 @@ type TestConfig struct { // If true, run this test when running with cloud env configured Cloud *bool + // If true, this test is not run if -short is passed and cloud env is configured + // This implies Cloud = true. + CloudLong *bool + // If true and Cloud=true, run this test only if unity catalog is available in the cloud environment RequiresUnityCatalog *bool From 027057deb72e6728fbe25248596eb72a24c7ab57 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 11 Mar 2025 15:17:56 +0100 Subject: [PATCH 2/4] update a comment --- acceptance/config_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/acceptance/config_test.go b/acceptance/config_test.go index 488f1db8a5..b9aeb02a28 100644 --- a/acceptance/config_test.go +++ b/acceptance/config_test.go @@ -31,8 +31,8 @@ type TestConfig struct { // If true, run this test when running with cloud env configured Cloud *bool - // If true, this test is not run if -short is passed and cloud env is configured - // This implies Cloud = true. + // If true, run this test when running with cloud env configured and -short is not passed + // This also sets -tail when -v is passed. CloudLong *bool // If true and Cloud=true, run this test only if unity catalog is available in the cloud environment From 08edbff5f4c3e07aa646b951a5a54542d24c3795 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 11 Mar 2025 15:19:25 +0100 Subject: [PATCH 3/4] fix error message --- acceptance/acceptance_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 8b4d42456a..3763b1e684 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -246,7 +246,7 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont } if isTruePtr(config.RequiresUnityCatalog) && os.Getenv("TEST_METASTORE_ID") == "" { - t.Skipf("Disable via RequiresUnityCatalog setting in %s (TEST_METASTORE_ID=%s)", configPath, os.Getenv("TEST_METASTORE_ID")) + t.Skipf("Disabled via RequiresUnityCatalog setting in %s (TEST_METASTORE_ID=%s)", configPath, os.Getenv("TEST_METASTORE_ID")) } } else { From d47217f125663c3c2aac5e4f8782e643cecb0b4b Mon Sep 17 00:00:00 2001 From: "Denis Bilenko (aider)" Date: Tue, 11 Mar 2025 16:24:45 +0100 Subject: [PATCH 4/4] refactor: Rename CloudLong to CloudSlow in acceptance tests --- acceptance/acceptance_test.go | 12 ++++++------ acceptance/config_test.go | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 3763b1e684..6c97aca4c0 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -224,24 +224,24 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont tailOutput := Tail if isRunningOnCloud { - if isTruePtr(config.CloudLong) { + if isTruePtr(config.CloudSlow) { if testing.Short() { - t.Skipf("Disabled via CloudLong setting in %s (CLOUD_ENV=%s, Short=%v)", configPath, cloudEnv, testing.Short()) + t.Skipf("Disabled via CloudSlow setting in %s (CLOUD_ENV=%s, Short=%v)", configPath, cloudEnv, testing.Short()) } if testing.Verbose() { - // Combination of CloudLong and -v auto-enables -tail + // Combination of CloudSlow and -v auto-enables -tail tailOutput = true } } - isCloudEnabled := isTruePtr(config.Cloud) || isTruePtr(config.CloudLong) + isCloudEnabled := isTruePtr(config.Cloud) || isTruePtr(config.CloudSlow) if !isCloudEnabled { - t.Skipf("Disabled via Cloud/CloudLong setting in %s (CLOUD_ENV=%s, Cloud=%v, CloudLong=%v)", + t.Skipf("Disabled via Cloud/CloudSlow setting in %s (CLOUD_ENV=%s, Cloud=%v, CloudSlow=%v)", configPath, cloudEnv, isTruePtr(config.Cloud), - isTruePtr(config.CloudLong), + isTruePtr(config.CloudSlow), ) } diff --git a/acceptance/config_test.go b/acceptance/config_test.go index b9aeb02a28..e61d91ad39 100644 --- a/acceptance/config_test.go +++ b/acceptance/config_test.go @@ -33,7 +33,7 @@ type TestConfig struct { // If true, run this test when running with cloud env configured and -short is not passed // This also sets -tail when -v is passed. - CloudLong *bool + CloudSlow *bool // If true and Cloud=true, run this test only if unity catalog is available in the cloud environment RequiresUnityCatalog *bool