Skip to content

Commit

Permalink
Fix streaming of stdout, stdin, stderr in cobra test runner (#1742)
Browse files Browse the repository at this point in the history
## Changes
We were not using the readers and writers set in the test fixtures in
the progress logger. This PR fixes that. It also modifies
`TestAccAbortBind`, which was implicitly relying on the bug.

I encountered this bug while working on
#1672.

## Tests
Manually. 

From non-tty:
```
Error: failed to bind the resource, 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.
```

From tty, bind works as expected.
```
Confirm import changes? Changes will be remotely applied only after running 'bundle deploy'. [y/n]: y
Updating deployment state...
Successfully bound databricks_pipeline with an id '9d2dedbb-f522-4503-96ba-4bc4d5bfa77d'. Run 'bundle deploy' to deploy changes to your workspace
```
  • Loading branch information
shreyas-goenka authored Sep 2, 2024
1 parent 0ce7be8 commit 0961236
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 2 deletions.
5 changes: 5 additions & 0 deletions bundle/deploy/terraform/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions cmd/root/progress_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
8 changes: 6 additions & 2 deletions internal/bundle/bind_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -101,12 +102,15 @@ func TestAccAbortBind(t *testing.T) {
destroyBundle(t, ctx, bundleRoot)
})

// Bind should fail because prompting is not possible.
t.Setenv("BUNDLE_ROOT", bundleRoot)
t.Setenv("TERM", "dumb")
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.
// Expect error suggesting to use --auto-approve
_, _, err = c.Run()
require.ErrorContains(t, err, "failed to bind the resource")
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)
Expand Down

0 comments on commit 0961236

Please sign in to comment.