From d5dfcf0ae2522ee1342ae1889c1556c3dc0ca163 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 2 Sep 2024 13:56:58 +0200 Subject: [PATCH 1/2] Fix streaming of stdout, stdin, stderr in cobra test runner --- bundle/deploy/terraform/import.go | 5 +++++ cmd/root/progress_logger.go | 6 ++++++ internal/bundle/bind_resource_test.go | 12 ++++++------ internal/bundle/deploy_test.go | 3 ++- internal/bundle/helpers.go | 7 ++----- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/bundle/deploy/terraform/import.go b/bundle/deploy/terraform/import.go index 7c1a681583..dfe60a5814 100644 --- a/bundle/deploy/terraform/import.go +++ b/bundle/deploy/terraform/import.go @@ -69,6 +69,11 @@ func (m *importResource) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn // Remove output starting from Warning until end of output output = output[:bytes.Index([]byte(output), []byte("Warning:"))] cmdio.LogString(ctx, output) + + if !cmdio.IsPromptSupported(ctx) { + return diag.Errorf("This bind operation requires user confirmation, but the current console does not support prompting. Please specify --auto-approve if you would like to skip prompts and proceed.") + } + ans, err := cmdio.AskYesOrNo(ctx, "Confirm import changes? Changes will be remotely applied only after running 'bundle deploy'.") if err != nil { return diag.FromErr(err) diff --git a/cmd/root/progress_logger.go b/cmd/root/progress_logger.go index c05ecb0437..7d6a1fa46d 100644 --- a/cmd/root/progress_logger.go +++ b/cmd/root/progress_logger.go @@ -29,6 +29,12 @@ func (f *progressLoggerFlag) resolveModeDefault(format flags.ProgressLogFormat) } func (f *progressLoggerFlag) initializeContext(ctx context.Context) (context.Context, error) { + // No need to initialize the logger if it's already set in the context. This + // happens in unit tests where the logger is setup as a fixture. + if _, ok := cmdio.FromContext(ctx); ok { + return ctx, nil + } + if f.log.level.String() != "disabled" && f.log.file.String() == "stderr" && f.ProgressLogFormat == flags.ModeInplace { return nil, fmt.Errorf("inplace progress logging cannot be used when log-file is stderr") diff --git a/internal/bundle/bind_resource_test.go b/internal/bundle/bind_resource_test.go index d44ad2c316..76c39650bd 100644 --- a/internal/bundle/bind_resource_test.go +++ b/internal/bundle/bind_resource_test.go @@ -101,12 +101,12 @@ func TestAccAbortBind(t *testing.T) { destroyBundle(t, ctx, bundleRoot) }) - t.Setenv("BUNDLE_ROOT", bundleRoot) - c := internal.NewCobraTestRunner(t, "bundle", "deployment", "bind", "foo", fmt.Sprint(jobId)) - - // Simulate user aborting the bind. This is done by not providing any input to the prompt in non-interactive mode. - _, _, err = c.Run() - require.ErrorContains(t, err, "failed to bind the resource") + // Bind should fail because prompting is not possible. + stdout, stderr, err := blackBoxRun(t, bundleRoot, "bundle", "deployment", "bind", "foo", fmt.Sprint(jobId)) + require.Error(t, err) + require.Contains(t, stderr, "failed to bind the resource") + require.Contains(t, stderr, "This bind operation requires user confirmation, but the current console does not support prompting. Please specify --auto-approve if you would like to skip prompts and proceed.") + require.Equal(t, "", stdout) err = deployBundle(t, ctx, bundleRoot) require.NoError(t, err) diff --git a/internal/bundle/deploy_test.go b/internal/bundle/deploy_test.go index 269b7c80a0..ba256cf254 100644 --- a/internal/bundle/deploy_test.go +++ b/internal/bundle/deploy_test.go @@ -145,7 +145,8 @@ func TestAccDeployBasicBundleLogs(t *testing.T) { currentUser, err := wt.W.CurrentUser.Me(ctx) require.NoError(t, err) - stdout, stderr := blackBoxRun(t, root, "bundle", "deploy") + stdout, stderr, err := blackBoxRun(t, root, "bundle", "deploy") + require.NoError(t, err) assert.Equal(t, strings.Join([]string{ fmt.Sprintf("Uploading bundle files to /Users/%s/.bundle/%s/files...", currentUser.UserName, uniqueId), "Deploying resources...", diff --git a/internal/bundle/helpers.go b/internal/bundle/helpers.go index 3547c17559..d7cb763bb4 100644 --- a/internal/bundle/helpers.go +++ b/internal/bundle/helpers.go @@ -118,7 +118,7 @@ func getBundleRemoteRootPath(w *databricks.WorkspaceClient, t *testing.T, unique return root } -func blackBoxRun(t *testing.T, root string, args ...string) (stdout string, stderr string) { +func blackBoxRun(t *testing.T, root string, args ...string) (stdout string, stderr string, err error) { cwd := vfs.MustNew(".") gitRoot, err := vfs.FindLeafInTree(cwd, ".git") require.NoError(t, err) @@ -134,11 +134,8 @@ func blackBoxRun(t *testing.T, root string, args ...string) (stdout string, stde cmd.Stdout = &outBuffer cmd.Stderr = &errBuffer - // Run the command + // Run the command, Get the output err = cmd.Run() - require.NoError(t, err) - - // Get the output stdout = outBuffer.String() stderr = errBuffer.String() return From 2135a3fd4ccbc1e6c208e5f6dcb551013ba2b5f9 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 2 Sep 2024 14:53:00 +0200 Subject: [PATCH 2/2] do not black box --- internal/bundle/bind_resource_test.go | 14 +++++++++----- internal/bundle/deploy_test.go | 3 +-- internal/bundle/helpers.go | 7 +++++-- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/internal/bundle/bind_resource_test.go b/internal/bundle/bind_resource_test.go index 76c39650bd..2449c31f2c 100644 --- a/internal/bundle/bind_resource_test.go +++ b/internal/bundle/bind_resource_test.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -102,11 +103,14 @@ func TestAccAbortBind(t *testing.T) { }) // Bind should fail because prompting is not possible. - stdout, stderr, err := blackBoxRun(t, bundleRoot, "bundle", "deployment", "bind", "foo", fmt.Sprint(jobId)) - require.Error(t, err) - require.Contains(t, stderr, "failed to bind the resource") - require.Contains(t, stderr, "This bind operation requires user confirmation, but the current console does not support prompting. Please specify --auto-approve if you would like to skip prompts and proceed.") - require.Equal(t, "", stdout) + t.Setenv("BUNDLE_ROOT", bundleRoot) + t.Setenv("TERM", "dumb") + c := internal.NewCobraTestRunner(t, "bundle", "deployment", "bind", "foo", fmt.Sprint(jobId)) + + // Expect error suggesting to use --auto-approve + _, _, err = c.Run() + assert.ErrorContains(t, err, "failed to bind the resource") + assert.ErrorContains(t, err, "This bind operation requires user confirmation, but the current console does not support prompting. Please specify --auto-approve if you would like to skip prompts and proceed") err = deployBundle(t, ctx, bundleRoot) require.NoError(t, err) diff --git a/internal/bundle/deploy_test.go b/internal/bundle/deploy_test.go index ba256cf254..269b7c80a0 100644 --- a/internal/bundle/deploy_test.go +++ b/internal/bundle/deploy_test.go @@ -145,8 +145,7 @@ func TestAccDeployBasicBundleLogs(t *testing.T) { currentUser, err := wt.W.CurrentUser.Me(ctx) require.NoError(t, err) - stdout, stderr, err := blackBoxRun(t, root, "bundle", "deploy") - require.NoError(t, err) + stdout, stderr := blackBoxRun(t, root, "bundle", "deploy") assert.Equal(t, strings.Join([]string{ fmt.Sprintf("Uploading bundle files to /Users/%s/.bundle/%s/files...", currentUser.UserName, uniqueId), "Deploying resources...", diff --git a/internal/bundle/helpers.go b/internal/bundle/helpers.go index d7cb763bb4..3547c17559 100644 --- a/internal/bundle/helpers.go +++ b/internal/bundle/helpers.go @@ -118,7 +118,7 @@ func getBundleRemoteRootPath(w *databricks.WorkspaceClient, t *testing.T, unique return root } -func blackBoxRun(t *testing.T, root string, args ...string) (stdout string, stderr string, err error) { +func blackBoxRun(t *testing.T, root string, args ...string) (stdout string, stderr string) { cwd := vfs.MustNew(".") gitRoot, err := vfs.FindLeafInTree(cwd, ".git") require.NoError(t, err) @@ -134,8 +134,11 @@ func blackBoxRun(t *testing.T, root string, args ...string) (stdout string, stde cmd.Stdout = &outBuffer cmd.Stderr = &errBuffer - // Run the command, Get the output + // Run the command err = cmd.Run() + require.NoError(t, err) + + // Get the output stdout = outBuffer.String() stderr = errBuffer.String() return