Inject output streams and avoid copying ui.UI (#91)

Currently the ui.UI type copies it self when running the build methods which makes it hard to reason about configuraiton of its values in different parts of the code base. This change makes the builder pattern mutable while updating all usage to be pointer based. This makes sure changes to the UI propagates correctly. Further more the output streams are now injected at construction time to force setting it and the cmd.Execute function is updated to take these as inputs. All in all this makes sure we actually output everything to the configured streams.
lunarway · Mar 4, 2022 · 3277246 · 3277246
1 parent 49b0065
commit 3277246
Show file tree

Hide file tree

Showing 13 changed files with 59 additions and 72 deletions.
diff --git a/cmd/cmd.go b/cmd/cmd.go
@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"fmt"
+	"io"
 	"log"
 	"os"
 	"path"
@@ -86,9 +87,8 @@ projects no matter what technologies the project is using.
 Read more about shuttle at https://github.com/lunarway/shuttle`, version),
 		PersistentPreRun: func(cmd *cobra.Command, args []string) {
 			if verboseFlag {
-				*uii = uii.SetUserLevel(ui.LevelVerbose)
+				uii.SetUserLevel(ui.LevelVerbose)
 			}
-			uii.SetOutput(cmd.OutOrStdout(), cmd.ErrOrStderr())
 			uii.Verboseln("Running shuttle")
 			uii.Verboseln("- version: %s", version)
 			uii.Verboseln("- commit: %s", commit)
@@ -114,33 +114,36 @@ If none of above is used, then the argument will expect a full plan spec.`)
 	return rootCmd, ctxProvider
 }
 
-func Execute() {
-	rootCmd := initializedRoot()
+func Execute(out, err io.Writer) {
+	rootCmd, uii := initializedRoot(out, err)
 
 	if err := rootCmd.Execute(); err != nil {
-		uii := ui.Create()
-		checkError(&uii, err)
+		checkError(uii, err)
 	}
 }
 
-func initializedRoot() *cobra.Command {
-	uii := ui.Create()
-
-	rootCmd, ctxProvider := newRoot(&uii)
-
-	rootCmd.AddCommand(newDocumentation(&uii, ctxProvider))
-	rootCmd.AddCommand(newCompletion(&uii))
-	rootCmd.AddCommand(newGet(&uii, ctxProvider))
-	rootCmd.AddCommand(newGitPlan(&uii, ctxProvider))
-	rootCmd.AddCommand(newHas(&uii, ctxProvider))
-	rootCmd.AddCommand(newLs(&uii, ctxProvider))
-	rootCmd.AddCommand(newPlan(&uii, ctxProvider))
-	rootCmd.AddCommand(newPrepare(&uii, ctxProvider))
-	rootCmd.AddCommand(newRun(&uii, ctxProvider))
-	rootCmd.AddCommand(newTemplate(&uii, ctxProvider))
-	rootCmd.AddCommand(newVersion(&uii))
+func initializedRoot(out, err io.Writer) (*cobra.Command, *ui.UI) {
+	uii := ui.Create(out, err)
+
+	rootCmd, ctxProvider := newRoot(uii)
+	rootCmd.SetOut(out)
+	rootCmd.SetErr(err)
+
+	rootCmd.AddCommand(
+		newDocumentation(uii, ctxProvider),
+		newCompletion(uii),
+		newGet(uii, ctxProvider),
+		newGitPlan(uii, ctxProvider),
+		newHas(uii, ctxProvider),
+		newLs(uii, ctxProvider),
+		newPlan(uii, ctxProvider),
+		newPrepare(uii, ctxProvider),
+		newRun(uii, ctxProvider),
+		newTemplate(uii, ctxProvider),
+		newVersion(uii),
+	)
 
-	return rootCmd
+	return rootCmd, uii
 }
 
 type contextProvider func() (config.ShuttleProjectContext, error)
@@ -169,7 +172,7 @@ func getProjectContext(rootCmd *cobra.Command, uii *ui.UI, projectPath string, c
 	}
 
 	var c config.ShuttleProjectContext
-	_, err = c.Setup(fullProjectPath, *uii, clean, skipGitPlanPulling, plan, projectFlagSet)
+	_, err = c.Setup(fullProjectPath, uii, clean, skipGitPlanPulling, plan, projectFlagSet)
 	if err != nil {
 		return config.ShuttleProjectContext{}, err
 	}

diff --git a/cmd/completion.go b/cmd/completion.go
@@ -81,7 +81,7 @@ Installing bash completion on Linux
 			return nil
 		},
 		Run: func(cmd *cobra.Command, args []string) {
-			*uii = uii.SetContext(ui.LevelSilent)
+			uii.SetContext(ui.LevelSilent)
 			switch args[0] {
 			case "zsh":
 				runCompletionZsh(cmd.OutOrStdout(), cmd.Root())

diff --git a/cmd/get.go b/cmd/get.go
@@ -21,7 +21,7 @@ func newGet(uii *ui.UI, contextProvider contextProvider) *cobra.Command {
 		Args:  cobra.ExactArgs(1),
 		//Long:  ``,
 		Run: func(cmd *cobra.Command, args []string) {
-			*uii = uii.SetContext(ui.LevelError)
+			uii.SetContext(ui.LevelError)
 			context, err := contextProvider()
 			checkError(uii, err)
 			path := args[0]

diff --git a/cmd/has.go b/cmd/has.go
@@ -23,7 +23,7 @@ func newHas(uii *ui.UI, contextProvider contextProvider) *cobra.Command {
 		SilenceErrors: true,
 		//Long:  ``,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			*uii = uii.SetContext(ui.LevelSilent)
+			uii.SetContext(ui.LevelSilent)
 
 			context, err := contextProvider()
 			checkError(uii, err)

diff --git a/cmd/plan.go b/cmd/plan.go
@@ -42,7 +42,7 @@ Available fields are:
 				ProjectPath       string
 				TempDirectoryPath string
 			}
-			*uii = uii.SetUserLevel(ui.LevelError)
+			uii.SetUserLevel(ui.LevelError)
 			context, err := contextProvider()
 			checkError(uii, err)
 

diff --git a/cmd/test.go b/cmd/test.go
@@ -21,9 +21,7 @@ func executeTestCases(t *testing.T, testCases []testCase) {
 			stdBuf := new(bytes.Buffer)
 			errBuf := new(bytes.Buffer)
 
-			rootCmd := initializedRoot()
-			rootCmd.SetOut(stdBuf)
-			rootCmd.SetErr(errBuf)
+			rootCmd, _ := initializedRoot(stdBuf, errBuf)
 			rootCmd.SetArgs(tc.input)
 
 			err := rootCmd.Execute()

diff --git a/cmd/version.go b/cmd/version.go
@@ -14,7 +14,7 @@ func newVersion(uii *ui.UI) *cobra.Command {
 		Use:   "version",
 		Short: "Info about version of shuttle",
 		Run: func(cmd *cobra.Command, args []string) {
-			*uii = uii.SetContext(ui.LevelSilent)
+			uii.SetContext(ui.LevelSilent)
 			if showCommit {
 				fmt.Println(commit)
 			} else {

diff --git a/main.go b/main.go
@@ -1,9 +1,11 @@
 package main
 
 import (
+	"os"
+
 	"github.com/lunarway/shuttle/cmd"
 )
 
 func main() {
-	cmd.Execute()
+	cmd.Execute(os.Stdout, os.Stderr)
 }
diff --git a/pkg/config/shuttleconfig.go b/pkg/config/shuttleconfig.go
@@ -32,11 +32,11 @@ type ShuttleProjectContext struct {
 	LocalPlanPath             string
 	Plan                      ShuttlePlanConfiguration
 	Scripts                   map[string]ShuttlePlanScript
-	UI                        ui.UI
+	UI                        *ui.UI
 }
 
 // Setup the ShuttleProjectContext for a specific path
-func (c *ShuttleProjectContext) Setup(projectPath string, uii ui.UI, clean bool, skipGitPlanPulling bool, planArgument string, strictConfigLookup bool) (*ShuttleProjectContext, error) {
+func (c *ShuttleProjectContext) Setup(projectPath string, uii *ui.UI, clean bool, skipGitPlanPulling bool, planArgument string, strictConfigLookup bool) (*ShuttleProjectContext, error) {
 	projectPath, err := c.Config.getConf(projectPath, strictConfigLookup)
 	if err != nil {
 		return nil, err

diff --git a/pkg/config/shuttleplan.go b/pkg/config/shuttleplan.go
@@ -85,7 +85,7 @@ func (p *ShuttlePlanConfiguration) Load(planPath string) (*ShuttlePlanConfigurat
 }
 
 // FetchPlan so it exists locally and return path to that plan
-func FetchPlan(plan string, projectPath string, localShuttleDirectoryPath string, uii ui.UI, skipGitPlanPulling bool, planArgument string) (string, error) {
+func FetchPlan(plan string, projectPath string, localShuttleDirectoryPath string, uii *ui.UI, skipGitPlanPulling bool, planArgument string) (string, error) {
 	if isPlanArgumentAPlan(planArgument) {
 		uii.Infoln("Using overloaded plan %v", planArgument)
 		return FetchPlan(getPlanFromPlanArgument(planArgument), projectPath, localShuttleDirectoryPath, uii, skipGitPlanPulling, "")

diff --git a/pkg/executors/executor_test.go b/pkg/executors/executor_test.go
@@ -1,6 +1,7 @@
 package executors
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"testing"
@@ -160,7 +161,7 @@ func TestExecute(t *testing.T) {
 	}
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
-			verboseUI := ui.Create()
+			verboseUI := ui.Create(&bytes.Buffer{}, &bytes.Buffer{})
 			verboseUI.SetUserLevel(ui.LevelVerbose)
 
 			err := Execute(context.Background(), config.ShuttleProjectContext{
@@ -191,7 +192,7 @@ func TestExecute(t *testing.T) {
 func TestExecute_contextCancellation(t *testing.T) {
 	imageName := fmt.Sprintf("shuttle-test-execute-cancellation-%d", time.Now().UnixNano())
 	t.Logf("Starting image %s", imageName)
-	verboseUI := ui.Create()
+	verboseUI := ui.Create(&bytes.Buffer{}, &bytes.Buffer{})
 	verboseUI.SetUserLevel(ui.LevelVerbose)
 	projectContext := config.ShuttleProjectContext{
 		UI: verboseUI,

diff --git a/pkg/git/git.go b/pkg/git/git.go
@@ -70,7 +70,7 @@ func IsPlan(plan string) bool {
 }
 
 // GetGitPlan will pull git repository and return its path
-func GetGitPlan(plan string, localShuttleDirectoryPath string, uii ui.UI, skipGitPlanPulling bool, planArgument string) (string, error) {
+func GetGitPlan(plan string, localShuttleDirectoryPath string, uii *ui.UI, skipGitPlanPulling bool, planArgument string) (string, error) {
 	parsedGitPlan := ParsePlan(plan)
 
 	if strings.HasPrefix(planArgument, "#") {
@@ -148,7 +148,7 @@ func GetGitPlan(plan string, localShuttleDirectoryPath string, uii ui.UI, skipGi
 	return planPath, nil
 }
 
-func RunGitPlanCommand(command string, plan string, uii ui.UI) {
+func RunGitPlanCommand(command string, plan string, uii *ui.UI) {
 
 	cmdOptions := go_cmd.Options{
 		Buffered:  false,
@@ -211,7 +211,7 @@ func expandHome(path string) string {
 	return strings.Replace(path, "~/", usr.HomeDir+"/", 1)
 }
 
-func gitCmd(command string, dir string, uii ui.UI) error {
+func gitCmd(command string, dir string, uii *ui.UI) error {
 	cmdOptions := go_cmd.Options{
 		Buffered:  true,
 		Streaming: true,

diff --git a/pkg/ui/ui.go b/pkg/ui/ui.go
@@ -3,7 +3,6 @@ package ui
 import (
 	"fmt"
 	"io"
-	"os"
 )
 
 // UI is the abstraction of handling terminal output for shuttle
@@ -17,50 +16,34 @@ type UI struct {
 }
 
 // Create doc
-func Create() UI {
-	return UI{
+func Create(out, err io.Writer) *UI {
+	return &UI{
 		EffectiveLevel: LevelInfo,
 		DefaultLevel:   LevelInfo,
 		UserLevelSet:   false,
-		Out:            os.Stdout,
-		Err:            os.Stderr,
+		Out:            out,
+		Err:            err,
 	}
 }
 
-func (ui *UI) SetOutput(out io.Writer, err io.Writer) {
-	ui.Out = out
-	ui.Err = err
-}
-
 // SetUserLevel doc
-func (ui *UI) SetUserLevel(level Level) UI {
-	return UI{
-		EffectiveLevel: level,
-		DefaultLevel:   ui.DefaultLevel,
-		UserLevel:      level,
-		UserLevelSet:   true,
-		Out:            ui.Out,
-		Err:            ui.Err,
-	}
+func (ui *UI) SetUserLevel(level Level) *UI {
+	ui.EffectiveLevel = level
+	ui.UserLevel = level
+	ui.UserLevelSet = true
+	return ui
 }
 
 // SetContext doc
-func (ui *UI) SetContext(level Level) UI {
-	var effectiveLevel Level
+func (ui *UI) SetContext(level Level) *UI {
 	if ui.UserLevelSet {
-		effectiveLevel = ui.UserLevel
+		ui.EffectiveLevel = ui.UserLevel
 	} else {
-		effectiveLevel = level
+		ui.EffectiveLevel = level
 	}
+	ui.DefaultLevel = level
 
-	return UI{
-		EffectiveLevel: effectiveLevel,
-		DefaultLevel:   level,
-		UserLevel:      ui.UserLevel,
-		UserLevelSet:   ui.UserLevelSet,
-		Out:            ui.Out,
-		Err:            ui.Err,
-	}
+	return ui
 }
 
 // Verboseln prints a formatted verbose message line.