Skip to content

Commit

Permalink
command/views: Remove command-specific runningInAutomation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
apparentlymart committed May 10, 2021
1 parent 669ff45 commit 330acba
Show file tree
Hide file tree
Showing 10 changed files with 28 additions and 24 deletions.
2 changes: 1 addition & 1 deletion command/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion command/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion command/refresh.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions command/views/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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:
Expand Down
16 changes: 8 additions & 8 deletions command/views/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")},
Expand All @@ -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{})

Expand All @@ -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 {
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)},
Expand Down
4 changes: 2 additions & 2 deletions command/views/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions command/views/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions command/views/refresh.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions command/views/refresh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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")},
Expand All @@ -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{})

Expand All @@ -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)},
Expand Down
4 changes: 4 additions & 0 deletions command/views/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 330acba

Please sign in to comment.