From a4b8570991ff4499b38763e6326c446da9d9961b Mon Sep 17 00:00:00 2001
From: Adrien Delorme <adrien.delorme@icloud.com>
Date: Tue, 7 May 2019 11:43:18 +0200
Subject: [PATCH] refactor arg parsing into it's own cfg maker & test it

---
 command/build.go      | 50 +++++++++++++++++++------------
 command/build_test.go | 70 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+), 19 deletions(-)

diff --git a/command/build.go b/command/build.go
index 98bbec293..aa54de632 100644
--- a/command/build.go
+++ b/command/build.go
@@ -52,12 +52,16 @@ func (c *BuildCommand) Run(args []string) int {
 	return c.RunContext(buildCtx, args)
 }
 
-func (c *BuildCommand) RunContext(buildCtx context.Context, args []string) int {
-	var cfg = struct {
-		Color, Debug, Force, Timestamp, Parallel bool
-		ParallelBuilds                           int64
-		OnError                                  string
-	}{}
+type Config struct {
+	Color, Debug, Force, Timestamp bool
+	ParallelBuilds                 int64
+	OnError                        string
+	Args                           []string
+}
+
+func (c *BuildCommand) ParseArgs(args []string) (Config, int) {
+	var cfg Config
+	var parallel bool
 	flags := c.Meta.FlagSet("build", FlagSetBuildFilter|FlagSetVars)
 	flags.Usage = func() { c.Ui.Say(c.Help()) }
 	flags.BoolVar(&cfg.Color, "color", true, "")
@@ -66,22 +70,37 @@ func (c *BuildCommand) RunContext(buildCtx context.Context, args []string) int {
 	flags.BoolVar(&cfg.Timestamp, "timestamp-ui", false, "")
 	flagOnError := enumflag.New(&cfg.OnError, "cleanup", "abort", "ask")
 	flags.Var(flagOnError, "on-error", "")
-	flags.BoolVar(&cfg.Parallel, "parallel", true, "")
+	flags.BoolVar(&parallel, "parallel", true, "")
 	flags.Int64Var(&cfg.ParallelBuilds, "parallel-builds", 0, "")
 	if err := flags.Parse(args); err != nil {
-		return 1
+		return cfg, 1
 	}
 
-	args = flags.Args()
-	if len(args) != 1 {
+	if parallel == false && cfg.ParallelBuilds == 0 {
+		cfg.ParallelBuilds = 1
+	}
+	if cfg.ParallelBuilds < 1 {
+		cfg.ParallelBuilds = math.MaxInt64
+	}
+
+	cfg.Args = flags.Args()
+	if len(cfg.Args) != 1 {
 		flags.Usage()
-		return 1
+		return cfg, 1
+	}
+	return cfg, 0
+}
+
+func (c *BuildCommand) RunContext(buildCtx context.Context, args []string) int {
+	cfg, ret := c.ParseArgs(args)
+	if ret != 0 {
+		return ret
 	}
 
 	// Parse the template
 	var tpl *template.Template
 	var err error
-	tpl, err = template.ParseFile(args[0])
+	tpl, err = template.ParseFile(cfg.Args[0])
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Failed to parse template: %s", err))
 		return 1
@@ -185,13 +204,6 @@ func (c *BuildCommand) RunContext(buildCtx context.Context, args []string) int {
 		m map[string]error
 	}{m: make(map[string]error)}
 
-	if cfg.ParallelBuilds < 1 {
-		cfg.ParallelBuilds = math.MaxInt64
-	}
-	if cfg.Parallel == false && cfg.ParallelBuilds == 0 {
-		cfg.ParallelBuilds = 1
-	}
-
 	limitParallel := semaphore.NewWeighted(cfg.ParallelBuilds)
 	for i := range builds {
 		if err := buildCtx.Err(); err != nil {
diff --git a/command/build_test.go b/command/build_test.go
index 8f3661b95..ac88d63b7 100644
--- a/command/build_test.go
+++ b/command/build_test.go
@@ -2,10 +2,13 @@ package command
 
 import (
 	"bytes"
+	"fmt"
+	"math"
 	"os"
 	"path/filepath"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/hashicorp/packer/builder/file"
 	"github.com/hashicorp/packer/packer"
 	shell_local "github.com/hashicorp/packer/post-processor/shell-local"
@@ -210,3 +213,70 @@ func cleanup() {
 	os.RemoveAll("lilas.txt")
 	os.RemoveAll("campanules.txt")
 }
+
+func TestBuildCommand_ParseArgs(t *testing.T) {
+	defaultMeta := testMetaFile(t)
+	type fields struct {
+		Meta Meta
+	}
+	type args struct {
+		args []string
+	}
+	tests := []struct {
+		fields       fields
+		args         args
+		wantCfg      Config
+		wantExitCode int
+	}{
+		{fields{defaultMeta},
+			args{[]string{"file.json"}},
+			Config{
+				Args:           []string{"file.json"},
+				ParallelBuilds: math.MaxInt64,
+				Color:          true,
+			},
+			0,
+		},
+		{fields{defaultMeta},
+			args{[]string{"-parallel=true", "file.json"}},
+			Config{
+				Args:           []string{"file.json"},
+				ParallelBuilds: math.MaxInt64,
+				Color:          true,
+			},
+			0,
+		},
+		{fields{defaultMeta},
+			args{[]string{"-parallel=false", "file.json"}},
+			Config{
+				Args:           []string{"file.json"},
+				ParallelBuilds: 1,
+				Color:          true,
+			},
+			0,
+		},
+		{fields{defaultMeta},
+			args{[]string{"-parallel-builds=5", "file.json"}},
+			Config{
+				Args:           []string{"file.json"},
+				ParallelBuilds: 5,
+				Color:          true,
+			},
+			0,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(fmt.Sprintf("%s", tt.args.args), func(t *testing.T) {
+			c := &BuildCommand{
+				Meta: tt.fields.Meta,
+			}
+			gotCfg, gotExitCode := c.ParseArgs(tt.args.args)
+			if diff := cmp.Diff(gotCfg, tt.wantCfg); diff != "" {
+				t.Fatalf("BuildCommand.ParseArgs() unexpected cfg %s", diff)
+			}
+			if gotExitCode != tt.wantExitCode {
+				t.Fatalf("BuildCommand.ParseArgs() gotExitCode = %v, want %v", gotExitCode, tt.wantExitCode)
+			}
+		})
+	}
+}