Updating recursive formatter tests to be cleaner and table driven

command/fmt_test.go

@@ -1,6 +1,7 @@
 package command
 
 import (
+	"bytes"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -54,42 +55,50 @@ func TestFmt_unfomattedTemlateDirectory(t *testing.T) {
 	}
 }
 
-type RecursiveTestCase struct {
-	TestCaseName             string
-	Recursion                bool
-	TopLevelFilePreFormat    []byte
-	LowerLevelFilePreFormat  []byte
-	TopLevelFilePostFormat   []byte
-	LowerLevelFilePostFormat []byte
-}
-
 func TestFmt_Recursive(t *testing.T) {
-	unformattedData := []byte(`ami_filter_name ="amzn2-ami-hvm-*-x86_64-gp2"
+	unformattedData := `ami_filter_name ="amzn2-ami-hvm-*-x86_64-gp2"
 ami_filter_owners =[ "137112412989" ]
 
-`)
+`
 
-	formattedData := []byte(`ami_filter_name   = "amzn2-ami-hvm-*-x86_64-gp2"
+	formattedData := `ami_filter_name   = "amzn2-ami-hvm-*-x86_64-gp2"
 ami_filter_owners = ["137112412989"]
 
-`)
+`
 
-	recursiveTestCases := []RecursiveTestCase{
+	tests := []struct {
+		name                  string
+		formatArgs            []string // arguments passed to format
+		alreadyPresentContent map[string]string
+		expectedContent       map[string]string
+	}{
 		{
-			TestCaseName:             "With Recursive flag on",
+			name:       "nested formats recursively",
-			Recursion:                true,
+			formatArgs: []string{"-recursive=true"},
-			TopLevelFilePreFormat:    unformattedData,
+			alreadyPresentContent: map[string]string{
-			LowerLevelFilePreFormat:  unformattedData,
+				"foo/bar/baz":     unformattedData,
-			TopLevelFilePostFormat:   formattedData,
+				"foo/bar/baz/woo": unformattedData,
-			LowerLevelFilePostFormat: formattedData,
+				"":                unformattedData,
+			},
+			expectedContent: map[string]string{
+				"foo/bar/baz":     formattedData,
+				"foo/bar/baz/woo": formattedData,
+				"":                formattedData,
+			},
 		},
 		{
-			TestCaseName:             "With Recursive flag off",
+			name:       "nested no recursive format",
-			Recursion:                false,
+			formatArgs: []string{},
-			TopLevelFilePreFormat:    unformattedData,
+			alreadyPresentContent: map[string]string{
-			LowerLevelFilePreFormat:  unformattedData,
+				"foo/bar/baz":     unformattedData,
-			TopLevelFilePostFormat:   formattedData,
+				"foo/bar/baz/woo": unformattedData,
-			LowerLevelFilePostFormat: unformattedData,
+				"":                unformattedData,
+			},
+			expectedContent: map[string]string{
+				"foo/bar/baz":     unformattedData,
+				"foo/bar/baz/woo": unformattedData,
+				"":                formattedData,
+			},
 		},
 	}
 
@@ -97,65 +106,93 @@ ami_filter_owners = ["137112412989"]
 		Meta: testMeta(t),
 	}
 
-	for _, tc := range recursiveTestCases {
+	testFileName := "test.pkrvars.hcl"
-		executeRecursiveTestCase(t, tc, c)
-	}
-}
 
-func executeRecursiveTestCase(t *testing.T, tc RecursiveTestCase, c *FormatCommand) {
+	for _, tt := range tests {
-	// Creating temp directories and files
+		topDir, err := ioutil.TempDir("test-fixtures/fmt", "temp-dir")
-	topDir, err := ioutil.TempDir("test-fixtures/fmt", "top-dir")
 		if err != nil {
-		t.Fatalf("failed to create sub level recurisve directory for test case: %s, error: %s", tc.TestCaseName, err)
+			t.Fatalf("Failed to create TopDir for test case: %s, error: %v", tt.name, err)
 		}
 		defer os.Remove(topDir)
 
-	subDir, err := ioutil.TempDir(topDir, "sub-dir")
+		for testDir, content := range tt.alreadyPresentContent {
+			dir := filepath.Join(topDir, testDir)
+			err := os.MkdirAll(dir, 0777)
 			if err != nil {
-		t.Fatalf("failed to create sub level recurisve directory for test case: %s, error: %s", tc.TestCaseName, err)
+				os.RemoveAll(topDir)
+				t.Fatalf(
+					"Failed to create subDir: %s\n\n, for test case: %s\n\n, error: %v",
+					testDir,
+					tt.name,
+					err)
 			}
-	defer os.Remove(subDir)
 
-	topTempFile, err := ioutil.TempFile(topDir, "*.pkrvars.hcl")
+			file, err := os.Create(filepath.Join(dir, testFileName))
 			if err != nil {
-		t.Fatalf("failed to create top level tempfile for test case: %s, error: %s", tc.TestCaseName, err)
+				os.RemoveAll(topDir)
+				t.Fatalf("failed to create testfile at directory: %s\n\n, for test case: %s\n\n, error: %s",
+					testDir,
+					tt.name,
+					err)
 			}
-	defer os.Remove(topTempFile.Name())
 
-	_, _ = topTempFile.Write(tc.TopLevelFilePreFormat)
+			_, err = file.Write([]byte(content))
-	topTempFile.Close()
-
-	subTempFile, err := ioutil.TempFile(subDir, "*.pkrvars.hcl")
 			if err != nil {
-		t.Fatalf("failed to create sub level tempfile for test case: %s, error: %s", tc.TestCaseName, err)
+				os.RemoveAll(topDir)
-	}
+				t.Fatalf("failed to write to testfile at directory: %s\n\n, for test case: %s\n\n, error: %s",
-	defer os.Remove(subTempFile.Name())
+					testDir,
-
+					tt.name,
-	_, _ = subTempFile.Write(tc.LowerLevelFilePreFormat)
+					err)
-	subTempFile.Close()
-
-	var args []string
-	if tc.Recursion {
-		args = []string{"-recursive=true", topDir}
-	} else {
-		args = []string{topDir}
 			}
 
-	if code := c.Run(args); code != 0 {
+			err = file.Close()
-		fatalCommand(t, c.Meta)
-	}
-
-	validateFileIsFormatted(t, tc.TopLevelFilePostFormat, topTempFile, tc)
-	validateFileIsFormatted(t, tc.LowerLevelFilePostFormat, subTempFile, tc)
-}
-
-func validateFileIsFormatted(t *testing.T, formattedData []byte, testFile *os.File, tc RecursiveTestCase) {
-	data, err := ioutil.ReadFile(testFile.Name())
 			if err != nil {
-		t.Fatalf("failed to open the newly formatted fixture for test case: %s, error: %s", tc.TestCaseName, err)
+				os.RemoveAll(topDir)
+				t.Fatalf("failed to close testfile at directory: %s\n\n, for test case: %s\n\n, error: %s",
+					testDir,
+					tt.name,
+					err)
+			}
 		}
 
-	if diff := cmp.Diff(string(data), string(formattedData)); diff != "" {
+		testArgs := append(tt.formatArgs, topDir)
-		t.Errorf("Unexpected format tfData output for test case: %v, diff:  %s", tc.TestCaseName, diff)
+		if code := c.Run(testArgs); code != 0 {
+			os.RemoveAll(topDir)
+			ui := c.Meta.Ui.(*packersdk.BasicUi)
+			out := ui.Writer.(*bytes.Buffer)
+			err := ui.ErrorWriter.(*bytes.Buffer)
+			t.Fatalf(
+				"Bad exit code for test case: %s.\n\nStdout:\n\n%s\n\nStderr:\n\n%s",
+				tt.name,
+				out.String(),
+				err.String())
+		}
+
+		for expectedPath, expectedContent := range tt.expectedContent {
+			b, err := ioutil.ReadFile(filepath.Join(topDir, expectedPath, testFileName))
+			if err != nil {
+				os.RemoveAll(topDir)
+				t.Fatalf("ReadFile failed for test case: %s, error : %v", tt.name, err)
+			}
+			got := string(b)
+			if diff := cmp.Diff(got, expectedContent); diff != "" {
+				os.RemoveAll(topDir)
+				t.Errorf(
+					"format dir, unexpected result for test case: %s, path: %s,  Expected: %s, Got: %s",
+					tt.name,
+					expectedPath,
+					expectedContent,
+					got)
 			}
 		}
+
+		err = os.RemoveAll(topDir)
+		if err != nil {
+			t.Errorf(
+				"Failed to delete top level test directory for test case: %s, please clean before another test run. Error: %s",
+				tt.name,
+				err)
+		}
+	}
+
+}

command/test-fixtures/fmt/formatted.pkrvars.hcl

@@ -1,3 +0,0 @@
-ami_filter_name   = "amzn2-ami-hvm-*-x86_64-gp2"
-ami_filter_owners = ["137112412989"]
-

hcl2template/formatter_test.go

@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -38,17 +39,8 @@ func TestHCL2Formatter_Format(t *testing.T) {
 	}
 }
 
-type FormatterRecursiveTestCase struct {
-	TestCaseName             string
-	Recursion                bool
-	TopLevelFilePreFormat    []byte
-	LowerLevelFilePreFormat  []byte
-	TopLevelFilePostFormat   []byte
-	LowerLevelFilePostFormat []byte
-}
-
 func TestHCL2Formatter_Recursive(t *testing.T) {
-	unformattedData := []byte(`
+	unformattedData := `
 // starts resources to provision them.
 build {
     sources = [
@@ -197,9 +189,9 @@ build {
         }
     }
 }
-`)
+`
 
-	formattedData := []byte(`
+	formattedData := `
 // starts resources to provision them.
 build {
   sources = [
@@ -348,92 +340,133 @@ build {
     }
   }
 }
-`)
+`
+
+	tests := []struct {
+		name                  string
+		recursive             bool
+		alreadyPresentContent map[string]string
+		expectedContent       map[string]string
+	}{
+		{
+			name:      "nested formats recursively",
+			recursive: true,
+			alreadyPresentContent: map[string]string{
+				"foo/bar/baz":     unformattedData,
+				"foo/bar/baz/woo": unformattedData,
+				"":                unformattedData,
+			},
+			expectedContent: map[string]string{
+				"foo/bar/baz":     formattedData,
+				"foo/bar/baz/woo": formattedData,
+				"":                formattedData,
+			},
+		},
+		{
+			name:      "nested no recursive format",
+			recursive: false,
+			alreadyPresentContent: map[string]string{
+				"foo/bar/baz":     unformattedData,
+				"foo/bar/baz/woo": unformattedData,
+				"":                unformattedData,
+			},
+			expectedContent: map[string]string{
+				"foo/bar/baz":     unformattedData,
+				"foo/bar/baz/woo": unformattedData,
+				"":                formattedData,
+			},
+		},
+	}
 
 	var buf bytes.Buffer
 	f := NewHCL2Formatter()
 	f.Output = &buf
 	f.Write = true
 
-	recursiveTestCases := []FormatterRecursiveTestCase{
+	testFileName := "test.pkrvars.hcl"
-		{
-			TestCaseName:             "With Recursive flag on",
-			Recursion:                true,
-			TopLevelFilePreFormat:    unformattedData,
-			LowerLevelFilePreFormat:  unformattedData,
-			TopLevelFilePostFormat:   formattedData,
-			LowerLevelFilePostFormat: formattedData,
-		},
-		{
-			TestCaseName:             "With Recursive flag off",
-			Recursion:                false,
-			TopLevelFilePreFormat:    unformattedData,
-			LowerLevelFilePreFormat:  unformattedData,
-			TopLevelFilePostFormat:   formattedData,
-			LowerLevelFilePostFormat: unformattedData,
-		},
-	}
 
-	for _, tc := range recursiveTestCases {
+	for _, tt := range tests {
-		executeRecursiveTestCase(t, tc, f)
+		topDir, err := ioutil.TempDir("testdata/format", "temp-dir")
-	}
-}
-
-func executeRecursiveTestCase(t *testing.T, tc FormatterRecursiveTestCase, f *HCL2Formatter) {
-	f.Recursive = tc.Recursion
-
-	var topDir string
-	topDir, err := ioutil.TempDir("testdata/format", "top-dir")
 		if err != nil {
-		t.Fatalf("failed to create sub level recurisve directory for test case: %s, errors: %s", tc.TestCaseName, err)
+			t.Fatalf("Failed to create TopDir for test case: %s, error: %v", tt.name, err)
 		}
-	defer os.Remove(topDir)
 
-	var subDir string
+		for testDir, content := range tt.alreadyPresentContent {
-	subDir, err = ioutil.TempDir(topDir, "sub-dir")
+			dir := filepath.Join(topDir, testDir)
+			err := os.MkdirAll(dir, 0777)
 			if err != nil {
-		t.Fatalf("failed to create sub level recurisve directory for test case: %s, errors: %s", tc.TestCaseName, err)
+				os.RemoveAll(topDir)
+				t.Fatalf(
+					"Failed to create subDir: %s\n\n, for test case: %s\n\n, error: %v",
+					testDir,
+					tt.name,
+					err)
 			}
-	defer os.Remove(subDir)
 
-	topTempFile, err := ioutil.TempFile(topDir, "*.pkr.hcl")
+			file, err := os.Create(filepath.Join(dir, testFileName))
 			if err != nil {
-		t.Fatalf("failed to create top level tempfile for test case: %s, errors: %s", tc.TestCaseName, err)
+				os.RemoveAll(topDir)
+				t.Fatalf("failed to create testfile at directory: %s\n\n, for test case: %s\n\n, error: %s",
+					testDir,
+					tt.name,
+					err)
 			}
-	defer os.Remove(topTempFile.Name())
 
-	_, _ = topTempFile.Write(tc.TopLevelFilePreFormat)
+			_, err = file.Write([]byte(content))
-	topTempFile.Close()
-
-	subTempFile, err := ioutil.TempFile(subDir, "*.pkr.hcl")
 			if err != nil {
-		t.Fatalf("failed to create sub level tempfile for test case: %s, errors: %s", tc.TestCaseName, err)
+				os.RemoveAll(topDir)
+				t.Fatalf("failed to write to testfile at directory: %s\n\n, for test case: %s\n\n, error: %s",
+					testDir,
+					tt.name,
+					err)
 			}
-	defer os.Remove(subTempFile.Name())
 
-	_, _ = subTempFile.Write(tc.LowerLevelFilePreFormat)
+			err = file.Close()
-	subTempFile.Close()
+			if err != nil {
+				os.RemoveAll(topDir)
+				t.Fatalf("failed to close testfile at directory: %s\n\n, for test case: %s\n\n, error: %s",
+					testDir,
+					tt.name,
+					err)
+			}
+		}
 
+		f.Recursive = tt.recursive
 		_, diags := f.Format(topDir)
 		if diags.HasErrors() {
-		t.Fatalf("the call to Format failed unexpectedly for test case: %s, errors: %s", tc.TestCaseName, diags.Error())
+			os.RemoveAll(topDir)
+			t.Fatalf("the call to Format failed unexpectedly for test case: %s, errors: %s", tt.name, diags.Error())
 		}
 
-	validateFileIsFormatted(t, tc.TopLevelFilePostFormat, topTempFile, tc.TestCaseName)
+		for expectedPath, expectedContent := range tt.expectedContent {
-	validateFileIsFormatted(t, tc.LowerLevelFilePostFormat, subTempFile, tc.TestCaseName)
+			b, err := ioutil.ReadFile(filepath.Join(topDir, expectedPath, testFileName))
-}
-
-func validateFileIsFormatted(t *testing.T, formattedData []byte, testFile *os.File, testCaseName string) {
-	data, err := ioutil.ReadFile(testFile.Name())
 			if err != nil {
-		t.Fatalf("failed to open the newly formatted fixture for test case: %s, errors: %s", testCaseName, err)
+				os.RemoveAll(topDir)
+				t.Fatalf("ReadFile failed for test case: %s, error : %v", tt.name, err)
+			}
+			got := string(b)
+			if diff := cmp.Diff(got, expectedContent); diff != "" {
+				os.RemoveAll(topDir)
+				t.Errorf(
+					"format dir, unexpected result for test case: %s, path: %s,  Expected: %s, Got: %s",
+					tt.name,
+					expectedPath,
+					expectedContent,
+					got)
+			}
 		}
 
-	if diff := cmp.Diff(string(data), string(formattedData)); diff != "" {
+		err = os.RemoveAll(topDir)
-		t.Errorf("Unexpected format tfData output for test case: %s, errors: %s", testCaseName, diff)
+		if err != nil {
+			t.Errorf(
+				"Failed to delete top level test directory for test case: %s, please clean before another test run. Error: %s",
+				tt.name,
+				err)
 		}
 	}
 
+}
+
 func TestHCL2Formatter_Format_Write(t *testing.T) {
 
 	var buf bytes.Buffer