From b402fd9d3ab0fc79244a5df178564579d7f72c1a Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 10 May 2021 10:20:43 -0700 Subject: [PATCH] command/views: Remove command-specific runningInAutomation We now have RunningInAutomation has a general concern in views.View, so we no longer need to specify it for each command-specific constructor separately. For this initial change I focused only on changing the exported interface of the views package and let the command-specific views go on having their own unexported fields containing a copy of the flag because it made this change less invasive and I wasn't feeling sure yet about whether we ought to have code within command-specific views directly access the internals of views.View. However, maybe we'll simplify this further in a later commit if we conclude that these copies of the flag are burdensome. The general version of this gets set directly inside the main package, which might at some future point allow us to make the command package itself unaware of this "running in automation" idea and thus reinforce that it's intended as a presentation-only thing rather than as a behavioral thing, but we'll save more invasive refactoring for another day. --- command/apply.go | 2 +- command/plan.go | 2 +- command/refresh.go | 2 +- command/views/apply.go | 4 ++-- command/views/apply_test.go | 16 ++++++++-------- command/views/plan.go | 4 ++-- command/views/plan_test.go | 4 ++-- command/views/refresh.go | 4 ++-- command/views/refresh_test.go | 10 +++++----- command/views/view.go | 4 ++++ 10 files changed, 28 insertions(+), 24 deletions(-) diff --git a/command/apply.go b/command/apply.go index 3942f4ad9..cca550011 100644 --- a/command/apply.go +++ b/command/apply.go @@ -46,7 +46,7 @@ func (c *ApplyCommand) Run(rawArgs []string) int { // Instantiate the view, even if there are flag errors, so that we render // diagnostics according to the desired view var view views.Apply - view = views.NewApply(args.ViewType, c.Destroy, c.RunningInAutomation, c.View) + view = views.NewApply(args.ViewType, c.Destroy, c.View) if diags.HasErrors() { view.Diagnostics(diags) diff --git a/command/plan.go b/command/plan.go index f4c81053f..8f3d1195b 100644 --- a/command/plan.go +++ b/command/plan.go @@ -31,7 +31,7 @@ func (c *PlanCommand) Run(rawArgs []string) int { // Instantiate the view, even if there are flag errors, so that we render // diagnostics according to the desired view - view := views.NewPlan(args.ViewType, c.RunningInAutomation, c.View) + view := views.NewPlan(args.ViewType, c.View) if diags.HasErrors() { view.Diagnostics(diags) diff --git a/command/refresh.go b/command/refresh.go index c7dbf7173..77942fd67 100644 --- a/command/refresh.go +++ b/command/refresh.go @@ -27,7 +27,7 @@ func (c *RefreshCommand) Run(rawArgs []string) int { // Instantiate the view, even if there are flag errors, so that we render // diagnostics according to the desired view var view views.Refresh - view = views.NewRefresh(args.ViewType, c.RunningInAutomation, c.View) + view = views.NewRefresh(args.ViewType, c.View) if diags.HasErrors() { view.Diagnostics(diags) diff --git a/command/views/apply.go b/command/views/apply.go index 4f2b3bba5..988924f99 100644 --- a/command/views/apply.go +++ b/command/views/apply.go @@ -24,7 +24,7 @@ type Apply interface { } // NewApply returns an initialized Apply implementation for the given ViewType. -func NewApply(vt arguments.ViewType, destroy bool, runningInAutomation bool, view *View) Apply { +func NewApply(vt arguments.ViewType, destroy bool, view *View) Apply { switch vt { case arguments.ViewJSON: return &ApplyJSON{ @@ -36,7 +36,7 @@ func NewApply(vt arguments.ViewType, destroy bool, runningInAutomation bool, vie return &ApplyHuman{ view: view, destroy: destroy, - inAutomation: runningInAutomation, + inAutomation: view.RunningInAutomation(), countHook: &countHook{}, } default: diff --git a/command/views/apply_test.go b/command/views/apply_test.go index 3d4ccee6e..fac161ac4 100644 --- a/command/views/apply_test.go +++ b/command/views/apply_test.go @@ -16,7 +16,7 @@ import ( func TestApply_new(t *testing.T) { streams, done := terminal.StreamsForTesting(t) defer done(t) - v := NewApply(arguments.ViewHuman, false, true, NewView(streams)) + v := NewApply(arguments.ViewHuman, false, NewView(streams).SetRunningInAutomation(true)) hv, ok := v.(*ApplyHuman) if !ok { t.Fatalf("unexpected return type %t", v) @@ -35,7 +35,7 @@ func TestApply_new(t *testing.T) { // elsewhere. func TestApplyHuman_outputs(t *testing.T) { streams, done := terminal.StreamsForTesting(t) - v := NewApply(arguments.ViewHuman, false, false, NewView(streams)) + v := NewApply(arguments.ViewHuman, false, NewView(streams)) v.Outputs(map[string]*states.OutputValue{ "foo": {Value: cty.StringVal("secret")}, @@ -52,7 +52,7 @@ func TestApplyHuman_outputs(t *testing.T) { // Outputs should do nothing if there are no outputs to render. func TestApplyHuman_outputsEmpty(t *testing.T) { streams, done := terminal.StreamsForTesting(t) - v := NewApply(arguments.ViewHuman, false, false, NewView(streams)) + v := NewApply(arguments.ViewHuman, false, NewView(streams)) v.Outputs(map[string]*states.OutputValue{}) @@ -67,7 +67,7 @@ func TestApplyHuman_outputsEmpty(t *testing.T) { func TestApplyHuman_operation(t *testing.T) { streams, done := terminal.StreamsForTesting(t) defer done(t) - v := NewApply(arguments.ViewHuman, false, true, NewView(streams)).Operation() + v := NewApply(arguments.ViewHuman, false, NewView(streams).SetRunningInAutomation(true)).Operation() if hv, ok := v.(*OperationHuman); !ok { t.Fatalf("unexpected return type %t", v) } else if hv.inAutomation != true { @@ -86,7 +86,7 @@ func TestApplyHuman_help(t *testing.T) { for name, destroy := range testCases { t.Run(name, func(t *testing.T) { streams, done := terminal.StreamsForTesting(t) - v := NewApply(arguments.ViewHuman, destroy, false, NewView(streams)) + v := NewApply(arguments.ViewHuman, destroy, NewView(streams)) v.HelpPrompt() got := done(t).Stderr() if !strings.Contains(got, name) { @@ -120,7 +120,7 @@ func TestApply_resourceCount(t *testing.T) { for _, viewType := range views { t.Run(fmt.Sprintf("%s (%s view)", name, viewType), func(t *testing.T) { streams, done := terminal.StreamsForTesting(t) - v := NewApply(viewType, tc.destroy, false, NewView(streams)) + v := NewApply(viewType, tc.destroy, NewView(streams)) hooks := v.Hooks() var count *countHook @@ -189,7 +189,7 @@ func TestApplyHuman_resourceCountStatePath(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { streams, done := terminal.StreamsForTesting(t) - v := NewApply(arguments.ViewHuman, false, false, NewView(streams)) + v := NewApply(arguments.ViewHuman, false, NewView(streams)) hooks := v.Hooks() var count *countHook @@ -224,7 +224,7 @@ func TestApplyHuman_resourceCountStatePath(t *testing.T) { // elsewhere. func TestApplyJSON_outputs(t *testing.T) { streams, done := terminal.StreamsForTesting(t) - v := NewApply(arguments.ViewJSON, false, false, NewView(streams)) + v := NewApply(arguments.ViewJSON, false, NewView(streams)) v.Outputs(map[string]*states.OutputValue{ "boop_count": {Value: cty.NumberIntVal(92)}, diff --git a/command/views/plan.go b/command/views/plan.go index bddf8aac3..fca9f0ec6 100644 --- a/command/views/plan.go +++ b/command/views/plan.go @@ -24,7 +24,7 @@ type Plan interface { } // NewPlan returns an initialized Plan implementation for the given ViewType. -func NewPlan(vt arguments.ViewType, runningInAutomation bool, view *View) Plan { +func NewPlan(vt arguments.ViewType, view *View) Plan { switch vt { case arguments.ViewJSON: return &PlanJSON{ @@ -33,7 +33,7 @@ func NewPlan(vt arguments.ViewType, runningInAutomation bool, view *View) Plan { case arguments.ViewHuman: return &PlanHuman{ view: view, - inAutomation: runningInAutomation, + inAutomation: view.RunningInAutomation(), } default: panic(fmt.Sprintf("unknown view type %v", vt)) diff --git a/command/views/plan_test.go b/command/views/plan_test.go index 481b05605..f3c33abba 100644 --- a/command/views/plan_test.go +++ b/command/views/plan_test.go @@ -18,7 +18,7 @@ import ( func TestPlanHuman_operation(t *testing.T) { streams, done := terminal.StreamsForTesting(t) defer done(t) - v := NewPlan(arguments.ViewHuman, true, NewView(streams)).Operation() + v := NewPlan(arguments.ViewHuman, NewView(streams).SetRunningInAutomation(true)).Operation() if hv, ok := v.(*OperationHuman); !ok { t.Fatalf("unexpected return type %t", v) } else if hv.inAutomation != true { @@ -30,7 +30,7 @@ func TestPlanHuman_operation(t *testing.T) { func TestPlanHuman_hooks(t *testing.T) { streams, done := terminal.StreamsForTesting(t) defer done(t) - v := NewPlan(arguments.ViewHuman, true, NewView(streams)) + v := NewPlan(arguments.ViewHuman, NewView(streams).SetRunningInAutomation((true))) hooks := v.Hooks() var uiHook *UiHook diff --git a/command/views/refresh.go b/command/views/refresh.go index dbc316574..39db5a3bf 100644 --- a/command/views/refresh.go +++ b/command/views/refresh.go @@ -22,7 +22,7 @@ type Refresh interface { } // NewRefresh returns an initialized Refresh implementation for the given ViewType. -func NewRefresh(vt arguments.ViewType, runningInAutomation bool, view *View) Refresh { +func NewRefresh(vt arguments.ViewType, view *View) Refresh { switch vt { case arguments.ViewJSON: return &RefreshJSON{ @@ -31,7 +31,7 @@ func NewRefresh(vt arguments.ViewType, runningInAutomation bool, view *View) Ref case arguments.ViewHuman: return &RefreshHuman{ view: view, - inAutomation: runningInAutomation, + inAutomation: view.RunningInAutomation(), countHook: &countHook{}, } default: diff --git a/command/views/refresh_test.go b/command/views/refresh_test.go index 3a409de67..ff1451184 100644 --- a/command/views/refresh_test.go +++ b/command/views/refresh_test.go @@ -15,7 +15,7 @@ import ( func TestRefreshHuman_operation(t *testing.T) { streams, done := terminal.StreamsForTesting(t) defer done(t) - v := NewRefresh(arguments.ViewHuman, true, NewView(streams)).Operation() + v := NewRefresh(arguments.ViewHuman, NewView(streams).SetRunningInAutomation(true)).Operation() if hv, ok := v.(*OperationHuman); !ok { t.Fatalf("unexpected return type %t", v) } else if hv.inAutomation != true { @@ -27,7 +27,7 @@ func TestRefreshHuman_operation(t *testing.T) { func TestRefreshHuman_hooks(t *testing.T) { streams, done := terminal.StreamsForTesting(t) defer done(t) - v := NewRefresh(arguments.ViewHuman, true, NewView(streams)) + v := NewRefresh(arguments.ViewHuman, NewView(streams).SetRunningInAutomation(true)) hooks := v.Hooks() var uiHook *UiHook @@ -45,7 +45,7 @@ func TestRefreshHuman_hooks(t *testing.T) { // elsewhere. func TestRefreshHuman_outputs(t *testing.T) { streams, done := terminal.StreamsForTesting(t) - v := NewRefresh(arguments.ViewHuman, false, NewView(streams)) + v := NewRefresh(arguments.ViewHuman, NewView(streams)) v.Outputs(map[string]*states.OutputValue{ "foo": {Value: cty.StringVal("secret")}, @@ -62,7 +62,7 @@ func TestRefreshHuman_outputs(t *testing.T) { // Outputs should do nothing if there are no outputs to render. func TestRefreshHuman_outputsEmpty(t *testing.T) { streams, done := terminal.StreamsForTesting(t) - v := NewRefresh(arguments.ViewHuman, false, NewView(streams)) + v := NewRefresh(arguments.ViewHuman, NewView(streams)) v.Outputs(map[string]*states.OutputValue{}) @@ -76,7 +76,7 @@ func TestRefreshHuman_outputsEmpty(t *testing.T) { // elsewhere. func TestRefreshJSON_outputs(t *testing.T) { streams, done := terminal.StreamsForTesting(t) - v := NewRefresh(arguments.ViewJSON, false, NewView(streams)) + v := NewRefresh(arguments.ViewJSON, NewView(streams)) v.Outputs(map[string]*states.OutputValue{ "boop_count": {Value: cty.NumberIntVal(92)}, diff --git a/command/views/view.go b/command/views/view.go index eb8a3bd79..f59fd4405 100644 --- a/command/views/view.go +++ b/command/views/view.go @@ -57,6 +57,10 @@ func (v *View) SetRunningInAutomation(new bool) *View { return v } +func (v *View) RunningInAutomation() bool { + return v.runningInAutomation +} + // Configure applies the global view configuration flags. func (v *View) Configure(view *arguments.View) { v.colorize.Disable = view.NoColor