Skip to content

Commit

Permalink
Remove cleanup in testcli package (#2108)
Browse files Browse the repository at this point in the history
## Changes

The main CLI entry point used to be a global variable, and the global
state had to be cleaned up after every test run. This hasn't been the
case for a while, and instead, the CLI is initialized in a function
call. State accumulated by a single CLI "instance" can no longer leak
into other instances, so we no longer have to perform cleanup.

## Tests

Existing tests pass.
  • Loading branch information
pietern authored Jan 10, 2025
1 parent 99cd3fe commit dc3a157
Showing 1 changed file with 0 additions and 48 deletions.
48 changes: 0 additions & 48 deletions internal/testcli/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,10 @@ import (
"context"
"encoding/json"
"io"
"reflect"
"strings"
"sync"
"time"

"github.com/spf13/cobra"
"github.com/spf13/pflag"
"github.com/stretchr/testify/require"

"github.com/databricks/cli/cmd"
Expand Down Expand Up @@ -68,39 +65,6 @@ func consumeLines(ctx context.Context, wg *sync.WaitGroup, r io.Reader) <-chan s
return ch
}

func (r *Runner) registerFlagCleanup(c *cobra.Command) {
r.Helper()
// Find target command that will be run. Example: if the command run is `databricks fs cp`,
// target command corresponds to `cp`
targetCmd, _, err := c.Find(r.args)
if err != nil && strings.HasPrefix(err.Error(), "unknown command") {
// even if command is unknown, we can proceed
require.NotNil(r, targetCmd)
} else {
require.NoError(r, err)
}

// Force initialization of default flags.
// These are initialized by cobra at execution time and would otherwise
// not be cleaned up by the cleanup function below.
targetCmd.InitDefaultHelpFlag()
targetCmd.InitDefaultVersionFlag()

// Restore flag values to their original value on test completion.
targetCmd.Flags().VisitAll(func(f *pflag.Flag) {
v := reflect.ValueOf(f.Value)
if v.Kind() == reflect.Ptr {
v = v.Elem()
}
// Store copy of the current flag value.
reset := reflect.New(v.Type()).Elem()
reset.Set(v)
r.Cleanup(func() {
v.Set(reset)
})
})
}

// Like [Runner.Eventually], but more specific
func (r *Runner) WaitForTextPrinted(text string, timeout time.Duration) {
r.Eventually(func() bool {
Expand Down Expand Up @@ -159,12 +123,6 @@ func (r *Runner) RunBackground() {
cli.SetIn(r.stdinR)
}

// Register cleanup function to restore flags to their original values
// once test has been executed. This is needed because flag values reside
// in a global singleton data-structure, and thus subsequent tests might
// otherwise interfere with each other
r.registerFlagCleanup(cli)

errch := make(chan error)
ctx, cancel := context.WithCancel(ctx)

Expand Down Expand Up @@ -208,12 +166,6 @@ func (r *Runner) RunBackground() {
}
}

// Reset context on command for the next test.
// These commands are globals so we have to clean up to the best of our ability after each run.
// See https://github.com/spf13/cobra/blob/a6f198b635c4b18fff81930c40d464904e55b161/command.go#L1062-L1066
//nolint:staticcheck // cobra sets the context and doesn't clear it
cli.SetContext(nil)

// Make caller aware of error.
errch <- err
close(errch)
Expand Down

0 comments on commit dc3a157

Please sign in to comment.