Fix streaming of stdout, stdin, stderr in cobra test runner (#1742)

## 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
https://github.com/databricks/cli/pull/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
```
This commit is contained in:
shreyas-goenka 2024-09-02 19:13:17 +05:30 committed by GitHub
parent 0ce7be8ff0
commit 096123674a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 17 additions and 2 deletions

View File

@ -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 // Remove output starting from Warning until end of output
output = output[:bytes.Index([]byte(output), []byte("Warning:"))] output = output[:bytes.Index([]byte(output), []byte("Warning:"))]
cmdio.LogString(ctx, output) 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'.") ans, err := cmdio.AskYesOrNo(ctx, "Confirm import changes? Changes will be remotely applied only after running 'bundle deploy'.")
if err != nil { if err != nil {
return diag.FromErr(err) return diag.FromErr(err)

View File

@ -29,6 +29,12 @@ func (f *progressLoggerFlag) resolveModeDefault(format flags.ProgressLogFormat)
} }
func (f *progressLoggerFlag) initializeContext(ctx context.Context) (context.Context, error) { 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" && if f.log.level.String() != "disabled" && f.log.file.String() == "stderr" &&
f.ProgressLogFormat == flags.ModeInplace { f.ProgressLogFormat == flags.ModeInplace {
return nil, fmt.Errorf("inplace progress logging cannot be used when log-file is stderr") return nil, fmt.Errorf("inplace progress logging cannot be used when log-file is stderr")

View File

@ -11,6 +11,7 @@ import (
"github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go"
"github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/jobs"
"github.com/google/uuid" "github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
@ -101,12 +102,15 @@ func TestAccAbortBind(t *testing.T) {
destroyBundle(t, ctx, bundleRoot) destroyBundle(t, ctx, bundleRoot)
}) })
// Bind should fail because prompting is not possible.
t.Setenv("BUNDLE_ROOT", bundleRoot) t.Setenv("BUNDLE_ROOT", bundleRoot)
t.Setenv("TERM", "dumb")
c := internal.NewCobraTestRunner(t, "bundle", "deployment", "bind", "foo", fmt.Sprint(jobId)) 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() _, _, 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) err = deployBundle(t, ctx, bundleRoot)
require.NoError(t, err) require.NoError(t, err)