From ab4b3a8465db9e63042253a933dda462f8bea317 Mon Sep 17 00:00:00 2001 From: teddylear Date: Fri, 8 Jan 2021 21:22:26 -0500 Subject: [PATCH 01/19] Adding recursive flag to formatter to format subdirectories --- command/cli.go | 4 +- command/fmt.go | 16 +- command/fmt_test.go | 12 ++ hcl2template/formatter.go | 108 ++++++++----- hcl2template/formatter_test.go | 9 +- .../format/sub_directory/unformatted.pkr.hcl | 149 ++++++++++++++++++ .../sub_directory/unformatted.pkrvars.hcl | 3 + 7 files changed, 256 insertions(+), 45 deletions(-) create mode 100644 hcl2template/testdata/format/sub_directory/unformatted.pkr.hcl create mode 100644 hcl2template/testdata/format/sub_directory/unformatted.pkrvars.hcl diff --git a/command/cli.go b/command/cli.go index 56d598d7f..85a24dad6 100644 --- a/command/cli.go +++ b/command/cli.go @@ -147,12 +147,12 @@ func (va *FormatArgs) AddFlagSets(flags *flag.FlagSet) { flags.BoolVar(&va.Check, "check", false, "check if the input is formatted") flags.BoolVar(&va.Diff, "diff", false, "display the diff of formatting changes") flags.BoolVar(&va.Write, "write", true, "overwrite source files instead of writing to stdout") - + flags.BoolVar(&va.Recursive, "recursive", true, "Also process files in subdirectories") va.MetaArgs.AddFlagSets(flags) } // FormatArgs represents a parsed cli line for `packer fmt` type FormatArgs struct { MetaArgs - Check, Diff, Write bool + Check, Diff, Write, Recursive bool } diff --git a/command/fmt.go b/command/fmt.go index 5b15ed772..6cf896381 100644 --- a/command/fmt.go +++ b/command/fmt.go @@ -48,9 +48,10 @@ func (c *FormatCommand) RunContext(ctx context.Context, cla *FormatArgs) int { } formatter := hclutils.HCL2Formatter{ - ShowDiff: cla.Diff, - Write: cla.Write, - Output: os.Stdout, + ShowDiff: cla.Diff, + Write: cla.Write, + Output: os.Stdout, + Recursive: cla.Recursive, } bytesModified, diags := formatter.Format(cla.Path) @@ -90,6 +91,8 @@ Options: -write=false Don't write to source files (always disabled if using -check) + -recursive Also process files in subdirectories. By default, only the + given directory (or current directory) is processed. ` return strings.TrimSpace(helpText) @@ -105,8 +108,9 @@ func (*FormatCommand) AutocompleteArgs() complete.Predictor { func (*FormatCommand) AutocompleteFlags() complete.Flags { return complete.Flags{ - "-check": complete.PredictNothing, - "-diff": complete.PredictNothing, - "-write": complete.PredictNothing, + "-check": complete.PredictNothing, + "-diff": complete.PredictNothing, + "-write": complete.PredictNothing, + "-recursive": complete.PredictNothing, } } diff --git a/command/fmt_test.go b/command/fmt_test.go index 242845f38..ee7ef9832 100644 --- a/command/fmt_test.go +++ b/command/fmt_test.go @@ -50,3 +50,15 @@ func TestFmt_unfomattedTemlateDirectory(t *testing.T) { fatalCommand(t, c.Meta) } } + +func TestFmt_Recursive(t *testing.T) { + c := &FormatCommand{ + Meta: testMeta(t), + } + + args := []string{"-check=true", "-recursive=true", filepath.Join(testFixture("fmt"), "")} + + if code := c.Run(args); code != 3 { + fatalCommand(t, c.Meta) + } +} diff --git a/hcl2template/formatter.go b/hcl2template/formatter.go index d02d421a4..aa275a35e 100644 --- a/hcl2template/formatter.go +++ b/hcl2template/formatter.go @@ -7,6 +7,8 @@ import ( "io/ioutil" "os" "os/exec" + "path/filepath" + "strings" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclparse" @@ -14,9 +16,9 @@ import ( ) type HCL2Formatter struct { - ShowDiff, Write bool - Output io.Writer - parser *hclparse.Parser + ShowDiff, Write, Recursive bool + Output io.Writer + parser *hclparse.Parser } // NewHCL2Formatter creates a new formatter, ready to format configuration files. @@ -26,55 +28,88 @@ func NewHCL2Formatter() *HCL2Formatter { } } +func isHcl2FileOrVarFile(path string) bool { + if strings.HasSuffix(path, hcl2FileExt) || strings.HasSuffix(path, hcl2VarFileExt) { + return true + } + return false +} + +func (f *HCL2Formatter) formatFile(path string, diags hcl.Diagnostics, bytesModified int) (int, hcl.Diagnostics) { + data, err := f.processFile(path) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("encountered an error while formatting %s", path), + Detail: err.Error(), + }) + } + bytesModified += len(data) + return bytesModified, diags +} + // Format all HCL2 files in path and return the total bytes formatted. // If any error is encountered, zero bytes will be returned. // // Path can be a directory or a file. func (f *HCL2Formatter) Format(path string) (int, hcl.Diagnostics) { + var diags hcl.Diagnostics + var bytesModified int - var allHclFiles []string - var diags []*hcl.Diagnostic - - if path == "-" { - allHclFiles = []string{"-"} - } else { - hclFiles, _, diags := GetHCL2Files(path, hcl2FileExt, hcl2JsonFileExt) - if diags.HasErrors() { - return 0, diags - } - - hclVarFiles, _, diags := GetHCL2Files(path, hcl2VarFileExt, hcl2VarJsonFileExt) - if diags.HasErrors() { - return 0, diags - } - - allHclFiles = append(hclFiles, hclVarFiles...) - - if len(allHclFiles) == 0 { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: fmt.Sprintf("Cannot tell whether %s contains HCL2 configuration data", path), - }) - - return 0, diags - } + if path == "" { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "path is empty, cannot format", + Detail: "path is empty, cannot format", + }) + return bytesModified, diags } if f.parser == nil { f.parser = hclparse.NewParser() } - var bytesModified int - for _, fn := range allHclFiles { - data, err := f.processFile(fn) + isDir, err := isDir(path) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Cannot tell wether " + path + " is a directory", + Detail: err.Error(), + }) + return bytesModified, diags + } + + if !isDir { + bytesModified, diags = f.formatFile(path, diags, bytesModified) + } else { + fileInfos, err := ioutil.ReadDir(path) if err != nil { - diags = append(diags, &hcl.Diagnostic{ + diag := &hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: fmt.Sprintf("encountered an error while formatting %s", fn), + Summary: "Cannot read hcl directory", Detail: err.Error(), - }) + } + diags = append(diags, diag) + return bytesModified, diags + } + + for _, fileInfo := range fileInfos { + filename := filepath.Join(path, fileInfo.Name()) + if fileInfo.IsDir() { + if f.Recursive { + var tempDiags hcl.Diagnostics + var tempBytesModified int + tempBytesModified, tempDiags = f.Format(filename) + bytesModified += tempBytesModified + diags = diags.Extend(tempDiags) + } else { + continue + } + } + if isHcl2FileOrVarFile(filename) { + bytesModified, diags = f.formatFile(filename, diags, bytesModified) + } } - bytesModified += len(data) } return bytesModified, diags @@ -84,6 +119,7 @@ func (f *HCL2Formatter) Format(path string) (int, hcl.Diagnostics) { // overwriting the contents of the original when the f.Write is true; a diff of the changes // will be outputted if f.ShowDiff is true. func (f *HCL2Formatter) processFile(filename string) ([]byte, error) { + if f.Output == nil { f.Output = os.Stdout } diff --git a/hcl2template/formatter_test.go b/hcl2template/formatter_test.go index 4d1474f71..bb31e002d 100644 --- a/hcl2template/formatter_test.go +++ b/hcl2template/formatter_test.go @@ -32,11 +32,18 @@ func TestHCL2Formatter_Format(t *testing.T) { if diags.HasErrors() { t.Fatalf("the call to Format failed unexpectedly %s", diags.Error()) } - if buf.String() != "" && tc.FormatExpected == false { t.Errorf("Format(%q) should contain the name of the formatted file(s), but got %q", tc.Path, buf.String()) } + } +} +func TestHCL2Formatter_Recursive(t *testing.T) { + f := NewHCL2Formatter() + f.Recursive = true + _, diags := f.Format("testdata/format") + if diags.HasErrors() { + t.Fatalf("the call to Format failed unexpectedly %s", diags.Error()) } } diff --git a/hcl2template/testdata/format/sub_directory/unformatted.pkr.hcl b/hcl2template/testdata/format/sub_directory/unformatted.pkr.hcl new file mode 100644 index 000000000..86aa3a154 --- /dev/null +++ b/hcl2template/testdata/format/sub_directory/unformatted.pkr.hcl @@ -0,0 +1,149 @@ + +// starts resources to provision them. +build { + sources = [ + "source.amazon-ebs.ubuntu-1604", + "source.virtualbox-iso.ubuntu-1204", + ] + + provisioner "shell" { + string = coalesce(null, "", "string") + int = "${41 + 1}" + int64 = "${42 + 1}" + bool = "true" + trilean = true + duration = "${9 + 1}s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a","b"], + ["c","d"] + ] + + nested { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a","b"], + ["c","d"] + ] + } + + nested_slice { + } + } + + provisioner "file" { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a","b"], + ["c","d"] + ] + + nested { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a","b"], + ["c","d"] + ] + } + + nested_slice { + } + } + + post-processor "amazon-import" { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a","b"], + ["c","d"] + ] + + nested { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a","b"], + ["c","d"] + ] + } + + nested_slice { + } + } +} diff --git a/hcl2template/testdata/format/sub_directory/unformatted.pkrvars.hcl b/hcl2template/testdata/format/sub_directory/unformatted.pkrvars.hcl new file mode 100644 index 000000000..7ddc9df79 --- /dev/null +++ b/hcl2template/testdata/format/sub_directory/unformatted.pkrvars.hcl @@ -0,0 +1,3 @@ +ami_filter_name ="amzn2-ami-hvm-*-x86_64-gp2" +ami_filter_owners =[ "137112412989" ] + From 261abe0caefba9bd62235b52f60170759dd6a05d Mon Sep 17 00:00:00 2001 From: teddylear Date: Mon, 11 Jan 2021 21:53:27 -0500 Subject: [PATCH 02/19] Setting recursive fmt to false, updatting recursive fmt test to validate formatted files --- command/cli.go | 2 +- command/fmt_test.go | 75 ++++++++- .../test-fixtures/fmt/formatted.pkrvars.hcl | 3 + hcl2template/formatter_test.go | 63 +++++++- .../format/sub_directory/unformatted.pkr.hcl | 149 ------------------ .../sub_directory/unformatted.pkrvars.hcl | 3 - 6 files changed, 139 insertions(+), 156 deletions(-) create mode 100644 command/test-fixtures/fmt/formatted.pkrvars.hcl delete mode 100644 hcl2template/testdata/format/sub_directory/unformatted.pkr.hcl delete mode 100644 hcl2template/testdata/format/sub_directory/unformatted.pkrvars.hcl diff --git a/command/cli.go b/command/cli.go index 85a24dad6..39e329502 100644 --- a/command/cli.go +++ b/command/cli.go @@ -147,7 +147,7 @@ func (va *FormatArgs) AddFlagSets(flags *flag.FlagSet) { flags.BoolVar(&va.Check, "check", false, "check if the input is formatted") flags.BoolVar(&va.Diff, "diff", false, "display the diff of formatting changes") flags.BoolVar(&va.Write, "write", true, "overwrite source files instead of writing to stdout") - flags.BoolVar(&va.Recursive, "recursive", true, "Also process files in subdirectories") + flags.BoolVar(&va.Recursive, "recursive", false, "Also process files in subdirectories") va.MetaArgs.AddFlagSets(flags) } diff --git a/command/fmt_test.go b/command/fmt_test.go index ee7ef9832..a57383999 100644 --- a/command/fmt_test.go +++ b/command/fmt_test.go @@ -1,10 +1,14 @@ package command import ( + "io/ioutil" + "os" "path/filepath" + "runtime" "strings" "testing" + "github.com/google/go-cmp/cmp" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" "github.com/stretchr/testify/assert" ) @@ -56,9 +60,76 @@ func TestFmt_Recursive(t *testing.T) { Meta: testMeta(t), } - args := []string{"-check=true", "-recursive=true", filepath.Join(testFixture("fmt"), "")} + unformattedData, err := ioutil.ReadFile("test-fixtures/fmt/unformatted.pkrvars.hcl") + if err != nil { + t.Fatalf("failed to open the unformatted fixture %s", err) + } - if code := c.Run(args); code != 3 { + var subDir string + subDir, err = ioutil.TempDir("test-fixtures/fmt", "sub_dir") + if err != nil { + t.Fatalf("failed to create sub level recurisve directory for test %s", err) + } + defer os.Remove(subDir) + + var superSubDir string + superSubDir, err = ioutil.TempDir(subDir, "super_sub_dir") + if err != nil { + t.Fatalf("failed to create sub level recurisve directory for test %s", err) + } + defer os.Remove(superSubDir) + + tf, err := ioutil.TempFile(subDir, "*.pkrvars.hcl") + if err != nil { + t.Fatalf("failed to create top level tempfile for test %s", err) + } + defer os.Remove(tf.Name()) + + _, _ = tf.Write(unformattedData) + tf.Close() + + subTf, err := ioutil.TempFile(superSubDir, "*.pkrvars.hcl") + if err != nil { + t.Fatalf("failed to create sub level tempfile for test %s", err) + } + defer os.Remove(subTf.Name()) + + _, _ = subTf.Write(unformattedData) + subTf.Close() + + var directoryDelimiter string + if runtime.GOOS == "windows" { + directoryDelimiter = "\\" + } else { + directoryDelimiter = "/" + } + + dirSplit := strings.Split(subDir, directoryDelimiter) + // Need just last bit to of top level temp directory to call command + subDirIsolated := dirSplit[len(dirSplit)-1] + args := []string{"-recursive=true", filepath.Join(testFixture("fmt"), subDirIsolated)} + + if code := c.Run(args); code != 0 { fatalCommand(t, c.Meta) } + + formattedData, err := ioutil.ReadFile("test-fixtures/fmt/formatted.pkrvars.hcl") + if err != nil { + t.Fatalf("failed to open the formatted fixture %s", err) + } + + validateFileIsFormatted(t, formattedData, tf) + validateFileIsFormatted(t, formattedData, subTf) +} + +func validateFileIsFormatted(t *testing.T, formattedData []byte, testFile *os.File) { + //lets re-read the tempfile which should now be formatted + data, err := ioutil.ReadFile(testFile.Name()) + if err != nil { + t.Fatalf("failed to open the newly formatted fixture %s", err) + } + + if diff := cmp.Diff(string(data), string(formattedData)); diff != "" { + t.Errorf("Unexpected format tfData output %s", diff) + } } diff --git a/command/test-fixtures/fmt/formatted.pkrvars.hcl b/command/test-fixtures/fmt/formatted.pkrvars.hcl new file mode 100644 index 000000000..a85fa0aee --- /dev/null +++ b/command/test-fixtures/fmt/formatted.pkrvars.hcl @@ -0,0 +1,3 @@ +ami_filter_name = "amzn2-ami-hvm-*-x86_64-gp2" +ami_filter_owners = ["137112412989"] + diff --git a/hcl2template/formatter_test.go b/hcl2template/formatter_test.go index bb31e002d..b084f634a 100644 --- a/hcl2template/formatter_test.go +++ b/hcl2template/formatter_test.go @@ -39,12 +39,73 @@ func TestHCL2Formatter_Format(t *testing.T) { } func TestHCL2Formatter_Recursive(t *testing.T) { + var buf bytes.Buffer f := NewHCL2Formatter() + f.Output = &buf + f.Write = true f.Recursive = true - _, diags := f.Format("testdata/format") + + unformattedData, err := ioutil.ReadFile("testdata/format/unformatted.pkr.hcl") + if err != nil { + t.Fatalf("failed to open the unformatted fixture %s", err) + } + + var subDir string + subDir, err = ioutil.TempDir("testdata/format", "sub_dir") + if err != nil { + t.Fatalf("failed to create sub level recurisve directory for test %s", err) + } + defer os.Remove(subDir) + + var superSubDir string + superSubDir, err = ioutil.TempDir(subDir, "super_sub_dir") + if err != nil { + t.Fatalf("failed to create sub level recurisve directory for test %s", err) + } + defer os.Remove(superSubDir) + + tf, err := ioutil.TempFile(subDir, "*.pkr.hcl") + if err != nil { + t.Fatalf("failed to create top level tempfile for test %s", err) + } + defer os.Remove(tf.Name()) + + _, _ = tf.Write(unformattedData) + tf.Close() + + subTf, err := ioutil.TempFile(superSubDir, "*.pkr.hcl") + if err != nil { + t.Fatalf("failed to create sub level tempfile for test %s", err) + } + defer os.Remove(subTf.Name()) + + _, _ = subTf.Write(unformattedData) + subTf.Close() + + _, diags := f.Format(subDir) if diags.HasErrors() { t.Fatalf("the call to Format failed unexpectedly %s", diags.Error()) } + + formattedData, err := ioutil.ReadFile("testdata/format/formatted.pkr.hcl") + if err != nil { + t.Fatalf("failed to open the formatted fixture %s", err) + } + + validateFileIsFormatted(t, formattedData, tf) + validateFileIsFormatted(t, formattedData, subTf) +} + +func validateFileIsFormatted(t *testing.T, formattedData []byte, testFile *os.File) { + //lets re-read the tempfile which should now be formatted + data, err := ioutil.ReadFile(testFile.Name()) + if err != nil { + t.Fatalf("failed to open the newly formatted fixture %s", err) + } + + if diff := cmp.Diff(string(data), string(formattedData)); diff != "" { + t.Errorf("Unexpected format tfData output %s", diff) + } } func TestHCL2Formatter_Format_Write(t *testing.T) { diff --git a/hcl2template/testdata/format/sub_directory/unformatted.pkr.hcl b/hcl2template/testdata/format/sub_directory/unformatted.pkr.hcl deleted file mode 100644 index 86aa3a154..000000000 --- a/hcl2template/testdata/format/sub_directory/unformatted.pkr.hcl +++ /dev/null @@ -1,149 +0,0 @@ - -// starts resources to provision them. -build { - sources = [ - "source.amazon-ebs.ubuntu-1604", - "source.virtualbox-iso.ubuntu-1204", - ] - - provisioner "shell" { - string = coalesce(null, "", "string") - int = "${41 + 1}" - int64 = "${42 + 1}" - bool = "true" - trilean = true - duration = "${9 + 1}s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a","b"], - ["c","d"] - ] - - nested { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a","b"], - ["c","d"] - ] - } - - nested_slice { - } - } - - provisioner "file" { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a","b"], - ["c","d"] - ] - - nested { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a","b"], - ["c","d"] - ] - } - - nested_slice { - } - } - - post-processor "amazon-import" { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a","b"], - ["c","d"] - ] - - nested { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a","b"], - ["c","d"] - ] - } - - nested_slice { - } - } -} diff --git a/hcl2template/testdata/format/sub_directory/unformatted.pkrvars.hcl b/hcl2template/testdata/format/sub_directory/unformatted.pkrvars.hcl deleted file mode 100644 index 7ddc9df79..000000000 --- a/hcl2template/testdata/format/sub_directory/unformatted.pkrvars.hcl +++ /dev/null @@ -1,3 +0,0 @@ -ami_filter_name ="amzn2-ami-hvm-*-x86_64-gp2" -ami_filter_owners =[ "137112412989" ] - From 6adf1f6659fbb4522be557eec0ca4313eea74cd7 Mon Sep 17 00:00:00 2001 From: teddylear Date: Mon, 18 Jan 2021 15:05:18 -0500 Subject: [PATCH 03/19] Fixing recursive fmt tests syntax and adding test case when recursive option is off --- command/fmt_test.go | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/command/fmt_test.go b/command/fmt_test.go index a57383999..cb20c5a9d 100644 --- a/command/fmt_test.go +++ b/command/fmt_test.go @@ -4,7 +4,6 @@ import ( "io/ioutil" "os" "path/filepath" - "runtime" "strings" "testing" @@ -97,17 +96,7 @@ func TestFmt_Recursive(t *testing.T) { _, _ = subTf.Write(unformattedData) subTf.Close() - var directoryDelimiter string - if runtime.GOOS == "windows" { - directoryDelimiter = "\\" - } else { - directoryDelimiter = "/" - } - - dirSplit := strings.Split(subDir, directoryDelimiter) - // Need just last bit to of top level temp directory to call command - subDirIsolated := dirSplit[len(dirSplit)-1] - args := []string{"-recursive=true", filepath.Join(testFixture("fmt"), subDirIsolated)} + args := []string{"-recursive=true", subDir} if code := c.Run(args); code != 0 { fatalCommand(t, c.Meta) @@ -120,6 +109,34 @@ func TestFmt_Recursive(t *testing.T) { validateFileIsFormatted(t, formattedData, tf) validateFileIsFormatted(t, formattedData, subTf) + + //Testing with recursive flag off that sub directories are not formatted + tf, err = ioutil.TempFile(subDir, "*.pkrvars.hcl") + if err != nil { + t.Fatalf("failed to create top level tempfile for test %s", err) + } + defer os.Remove(tf.Name()) + + _, _ = tf.Write(unformattedData) + tf.Close() + + subTf, err = ioutil.TempFile(superSubDir, "*.pkrvars.hcl") + if err != nil { + t.Fatalf("failed to create sub level tempfile for test %s", err) + } + defer os.Remove(subTf.Name()) + + _, _ = subTf.Write(unformattedData) + subTf.Close() + + args = []string{subDir} + + if code := c.Run(args); code != 0 { + fatalCommand(t, c.Meta) + } + + validateFileIsFormatted(t, formattedData, tf) + validateFileIsFormatted(t, unformattedData, subTf) } func validateFileIsFormatted(t *testing.T, formattedData []byte, testFile *os.File) { From 93df53a2753e69d47fed6274bf4e2e37384d7c44 Mon Sep 17 00:00:00 2001 From: teddylear Date: Thu, 28 Jan 2021 19:56:51 -0500 Subject: [PATCH 04/19] Refactor recursive formatting test cases to be table driven --- command/fmt_test.go | 112 ++++++----- hcl2template/formatter.go | 3 +- hcl2template/formatter_test.go | 356 +++++++++++++++++++++++++++++++-- 3 files changed, 406 insertions(+), 65 deletions(-) diff --git a/command/fmt_test.go b/command/fmt_test.go index cb20c5a9d..a7cf9b70c 100644 --- a/command/fmt_test.go +++ b/command/fmt_test.go @@ -1,6 +1,7 @@ package command import ( + "fmt" "io/ioutil" "os" "path/filepath" @@ -54,25 +55,63 @@ 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" +ami_filter_owners =[ "137112412989" ] + +`) + + formattedData := []byte(`ami_filter_name = "amzn2-ami-hvm-*-x86_64-gp2" +ami_filter_owners = ["137112412989"] + +`) + + recursiveTestCases := []RecursiveTestCase{ + { + 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, + }, + } + c := &FormatCommand{ Meta: testMeta(t), } - unformattedData, err := ioutil.ReadFile("test-fixtures/fmt/unformatted.pkrvars.hcl") - if err != nil { - t.Fatalf("failed to open the unformatted fixture %s", err) + for _, tc := range recursiveTestCases { + executeRecursiveTestCase(t, tc, c) } +} - var subDir string - subDir, err = ioutil.TempDir("test-fixtures/fmt", "sub_dir") +func executeRecursiveTestCase(t *testing.T, tc RecursiveTestCase, c *FormatCommand) { + // Creating temp directories and files + subDir, err := ioutil.TempDir("test-fixtures/fmt", "sub_dir") if err != nil { t.Fatalf("failed to create sub level recurisve directory for test %s", err) } defer os.Remove(subDir) - var superSubDir string - superSubDir, err = ioutil.TempDir(subDir, "super_sub_dir") + superSubDir, err := ioutil.TempDir(subDir, "super_sub_dir") if err != nil { t.Fatalf("failed to create sub level recurisve directory for test %s", err) } @@ -84,69 +123,46 @@ func TestFmt_Recursive(t *testing.T) { } defer os.Remove(tf.Name()) - _, _ = tf.Write(unformattedData) + _, _ = tf.Write(tc.TopLevelFilePreFormat) tf.Close() + data, err := ioutil.ReadFile(tf.Name()) + if err != nil { + t.Fatalf("failed to open the newly formatted fixture %s", err) + } + fmt.Println(fmt.Sprintf("top level data: %v", data)) + subTf, err := ioutil.TempFile(superSubDir, "*.pkrvars.hcl") if err != nil { t.Fatalf("failed to create sub level tempfile for test %s", err) } defer os.Remove(subTf.Name()) - _, _ = subTf.Write(unformattedData) + _, _ = subTf.Write(tc.LowerLevelFilePreFormat) subTf.Close() - args := []string{"-recursive=true", subDir} + var args []string + if tc.Recursion { + args = []string{"-recursive=true", subDir} + } else { + args = []string{subDir} + } if code := c.Run(args); code != 0 { fatalCommand(t, c.Meta) } - formattedData, err := ioutil.ReadFile("test-fixtures/fmt/formatted.pkrvars.hcl") - if err != nil { - t.Fatalf("failed to open the formatted fixture %s", err) - } - - validateFileIsFormatted(t, formattedData, tf) - validateFileIsFormatted(t, formattedData, subTf) - - //Testing with recursive flag off that sub directories are not formatted - tf, err = ioutil.TempFile(subDir, "*.pkrvars.hcl") - if err != nil { - t.Fatalf("failed to create top level tempfile for test %s", err) - } - defer os.Remove(tf.Name()) - - _, _ = tf.Write(unformattedData) - tf.Close() - - subTf, err = ioutil.TempFile(superSubDir, "*.pkrvars.hcl") - if err != nil { - t.Fatalf("failed to create sub level tempfile for test %s", err) - } - defer os.Remove(subTf.Name()) - - _, _ = subTf.Write(unformattedData) - subTf.Close() - - args = []string{subDir} - - if code := c.Run(args); code != 0 { - fatalCommand(t, c.Meta) - } - - validateFileIsFormatted(t, formattedData, tf) - validateFileIsFormatted(t, unformattedData, subTf) + validateFileIsFormatted(t, tc.TopLevelFilePostFormat, tf, tc) + validateFileIsFormatted(t, tc.LowerLevelFilePostFormat, subTf, tc) } -func validateFileIsFormatted(t *testing.T, formattedData []byte, testFile *os.File) { - //lets re-read the tempfile which should now be formatted +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 %s", err) } if diff := cmp.Diff(string(data), string(formattedData)); diff != "" { - t.Errorf("Unexpected format tfData output %s", diff) + t.Errorf("Unexpected format tfData output on tc: %v, diff: %s", tc.TestCaseName, diff) } } diff --git a/hcl2template/formatter.go b/hcl2template/formatter.go index aa275a35e..42534805d 100644 --- a/hcl2template/formatter.go +++ b/hcl2template/formatter.go @@ -102,9 +102,8 @@ func (f *HCL2Formatter) Format(path string) (int, hcl.Diagnostics) { tempBytesModified, tempDiags = f.Format(filename) bytesModified += tempBytesModified diags = diags.Extend(tempDiags) - } else { - continue } + continue } if isHcl2FileOrVarFile(filename) { bytesModified, diags = f.formatFile(filename, diags, bytesModified) diff --git a/hcl2template/formatter_test.go b/hcl2template/formatter_test.go index b084f634a..d30413069 100644 --- a/hcl2template/formatter_test.go +++ b/hcl2template/formatter_test.go @@ -38,20 +38,352 @@ 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(` +// starts resources to provision them. +build { + sources = [ + "source.amazon-ebs.ubuntu-1604", + "source.virtualbox-iso.ubuntu-1204", + ] + + provisioner "shell" { + string = coalesce(null, "", "string") + int = "${41 + 1}" + int64 = "${42 + 1}" + bool = "true" + trilean = true + duration = "${9 + 1}s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a","b"], + ["c","d"] + ] + + nested { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a","b"], + ["c","d"] + ] + } + + nested_slice { + } + } + + provisioner "file" { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a","b"], + ["c","d"] + ] + + nested { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a","b"], + ["c","d"] + ] + } + + nested_slice { + } + } + + post-processor "amazon-import" { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a","b"], + ["c","d"] + ] + + nested { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a","b"], + ["c","d"] + ] + } + + nested_slice { + } + } +} +`) + + formattedData := []byte(` +// starts resources to provision them. +build { + sources = [ + "source.amazon-ebs.ubuntu-1604", + "source.virtualbox-iso.ubuntu-1204", + ] + + provisioner "shell" { + string = coalesce(null, "", "string") + int = "${41 + 1}" + int64 = "${42 + 1}" + bool = "true" + trilean = true + duration = "${9 + 1}s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a", "b"], + ["c", "d"] + ] + + nested { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a", "b"], + ["c", "d"] + ] + } + + nested_slice { + } + } + + provisioner "file" { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a", "b"], + ["c", "d"] + ] + + nested { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a", "b"], + ["c", "d"] + ] + } + + nested_slice { + } + } + + post-processor "amazon-import" { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a", "b"], + ["c", "d"] + ] + + nested { + string = "string" + int = 42 + int64 = 43 + bool = true + trilean = true + duration = "10s" + map_string_string = { + a = "b" + c = "d" + } + slice_string = [ + "a", + "b", + "c", + ] + slice_slice_string = [ + ["a", "b"], + ["c", "d"] + ] + } + + nested_slice { + } + } +} +`) + var buf bytes.Buffer f := NewHCL2Formatter() f.Output = &buf f.Write = true - f.Recursive = true - unformattedData, err := ioutil.ReadFile("testdata/format/unformatted.pkr.hcl") - if err != nil { - t.Fatalf("failed to open the unformatted fixture %s", err) + recursiveTestCases := []FormatterRecursiveTestCase{ + { + 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 { + executeRecursiveTestCase(t, tc, f) + } +} + +func executeRecursiveTestCase(t *testing.T, tc FormatterRecursiveTestCase, f *HCL2Formatter) { + f.Recursive = tc.Recursion + var subDir string - subDir, err = ioutil.TempDir("testdata/format", "sub_dir") + subDir, err := ioutil.TempDir("testdata/format", "sub_dir") if err != nil { t.Fatalf("failed to create sub level recurisve directory for test %s", err) } @@ -70,7 +402,7 @@ func TestHCL2Formatter_Recursive(t *testing.T) { } defer os.Remove(tf.Name()) - _, _ = tf.Write(unformattedData) + _, _ = tf.Write(tc.TopLevelFilePreFormat) tf.Close() subTf, err := ioutil.TempFile(superSubDir, "*.pkr.hcl") @@ -79,7 +411,7 @@ func TestHCL2Formatter_Recursive(t *testing.T) { } defer os.Remove(subTf.Name()) - _, _ = subTf.Write(unformattedData) + _, _ = subTf.Write(tc.LowerLevelFilePreFormat) subTf.Close() _, diags := f.Format(subDir) @@ -87,17 +419,11 @@ func TestHCL2Formatter_Recursive(t *testing.T) { t.Fatalf("the call to Format failed unexpectedly %s", diags.Error()) } - formattedData, err := ioutil.ReadFile("testdata/format/formatted.pkr.hcl") - if err != nil { - t.Fatalf("failed to open the formatted fixture %s", err) - } - - validateFileIsFormatted(t, formattedData, tf) - validateFileIsFormatted(t, formattedData, subTf) + validateFileIsFormatted(t, tc.TopLevelFilePostFormat, tf) + validateFileIsFormatted(t, tc.LowerLevelFilePostFormat, subTf) } func validateFileIsFormatted(t *testing.T, formattedData []byte, testFile *os.File) { - //lets re-read the tempfile which should now be formatted data, err := ioutil.ReadFile(testFile.Name()) if err != nil { t.Fatalf("failed to open the newly formatted fixture %s", err) From 40a97e29db3176b9b9897351777a875040afbe28 Mon Sep 17 00:00:00 2001 From: teddylear Date: Fri, 29 Jan 2021 22:11:41 -0500 Subject: [PATCH 05/19] Clean up recursive format tests to be more accurate --- command/fmt_test.go | 57 +++++++++++++++------------------- hcl2template/formatter_test.go | 56 ++++++++++++++++----------------- 2 files changed, 53 insertions(+), 60 deletions(-) diff --git a/command/fmt_test.go b/command/fmt_test.go index a7cf9b70c..c66777a9e 100644 --- a/command/fmt_test.go +++ b/command/fmt_test.go @@ -1,7 +1,6 @@ package command import ( - "fmt" "io/ioutil" "os" "path/filepath" @@ -105,64 +104,58 @@ ami_filter_owners = ["137112412989"] func executeRecursiveTestCase(t *testing.T, tc RecursiveTestCase, c *FormatCommand) { // Creating temp directories and files - subDir, err := ioutil.TempDir("test-fixtures/fmt", "sub_dir") + topDir, err := ioutil.TempDir("test-fixtures/fmt", "top-dir") if err != nil { - t.Fatalf("failed to create sub level recurisve directory for test %s", err) + t.Fatalf("failed to create sub level recurisve directory for test case: %s, error: %s", tc.TestCaseName, err) + } + defer os.Remove(topDir) + + subDir, err := ioutil.TempDir(topDir, "sub-dir") + if err != nil { + t.Fatalf("failed to create sub level recurisve directory for test case: %s, error: %s", tc.TestCaseName, err) } defer os.Remove(subDir) - superSubDir, err := ioutil.TempDir(subDir, "super_sub_dir") + topTempFile, err := ioutil.TempFile(topDir, "*.pkrvars.hcl") if err != nil { - t.Fatalf("failed to create sub level recurisve directory for test %s", err) + t.Fatalf("failed to create top level tempfile for test case: %s, error: %s", tc.TestCaseName, err) } - defer os.Remove(superSubDir) + defer os.Remove(topTempFile.Name()) - tf, err := ioutil.TempFile(subDir, "*.pkrvars.hcl") + _, _ = topTempFile.Write(tc.TopLevelFilePreFormat) + topTempFile.Close() + + subTempFile, err := ioutil.TempFile(subDir, "*.pkrvars.hcl") if err != nil { - t.Fatalf("failed to create top level tempfile for test %s", err) + t.Fatalf("failed to create sub level tempfile for test case: %s, error: %s", tc.TestCaseName, err) } - defer os.Remove(tf.Name()) + defer os.Remove(subTempFile.Name()) - _, _ = tf.Write(tc.TopLevelFilePreFormat) - tf.Close() - - data, err := ioutil.ReadFile(tf.Name()) - if err != nil { - t.Fatalf("failed to open the newly formatted fixture %s", err) - } - fmt.Println(fmt.Sprintf("top level data: %v", data)) - - subTf, err := ioutil.TempFile(superSubDir, "*.pkrvars.hcl") - if err != nil { - t.Fatalf("failed to create sub level tempfile for test %s", err) - } - defer os.Remove(subTf.Name()) - - _, _ = subTf.Write(tc.LowerLevelFilePreFormat) - subTf.Close() + _, _ = subTempFile.Write(tc.LowerLevelFilePreFormat) + subTempFile.Close() var args []string if tc.Recursion { - args = []string{"-recursive=true", subDir} + args = []string{"-recursive=true", topDir} } else { - args = []string{subDir} + args = []string{topDir} } if code := c.Run(args); code != 0 { fatalCommand(t, c.Meta) } - validateFileIsFormatted(t, tc.TopLevelFilePostFormat, tf, tc) - validateFileIsFormatted(t, tc.LowerLevelFilePostFormat, subTf, tc) + 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 %s", err) + t.Fatalf("failed to open the newly formatted fixture for test case: %s, error: %s", tc.TestCaseName, err) } if diff := cmp.Diff(string(data), string(formattedData)); diff != "" { - t.Errorf("Unexpected format tfData output on tc: %v, diff: %s", tc.TestCaseName, diff) + t.Errorf("Unexpected format tfData output for test case: %v, diff: %s", tc.TestCaseName, diff) } } diff --git a/hcl2template/formatter_test.go b/hcl2template/formatter_test.go index d30413069..469237887 100644 --- a/hcl2template/formatter_test.go +++ b/hcl2template/formatter_test.go @@ -382,55 +382,55 @@ build { func executeRecursiveTestCase(t *testing.T, tc FormatterRecursiveTestCase, f *HCL2Formatter) { f.Recursive = tc.Recursion - var subDir string - subDir, err := ioutil.TempDir("testdata/format", "sub_dir") + 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 %s", err) + t.Fatalf("failed to create sub level recurisve directory for test case: %s, errors: %s", tc.TestCaseName, err) + } + defer os.Remove(topDir) + + var subDir string + subDir, err = ioutil.TempDir(topDir, "sub-dir") + if err != nil { + t.Fatalf("failed to create sub level recurisve directory for test case: %s, errors: %s", tc.TestCaseName, err) } defer os.Remove(subDir) - var superSubDir string - superSubDir, err = ioutil.TempDir(subDir, "super_sub_dir") + topTempFile, err := ioutil.TempFile(topDir, "*.pkr.hcl") if err != nil { - t.Fatalf("failed to create sub level recurisve directory for test %s", err) + t.Fatalf("failed to create top level tempfile for test case: %s, errors: %s", tc.TestCaseName, err) } - defer os.Remove(superSubDir) + defer os.Remove(topTempFile.Name()) - tf, err := ioutil.TempFile(subDir, "*.pkr.hcl") + _, _ = topTempFile.Write(tc.TopLevelFilePreFormat) + topTempFile.Close() + + subTempFile, err := ioutil.TempFile(subDir, "*.pkr.hcl") if err != nil { - t.Fatalf("failed to create top level tempfile for test %s", err) + t.Fatalf("failed to create sub level tempfile for test case: %s, errors: %s", tc.TestCaseName, err) } - defer os.Remove(tf.Name()) + defer os.Remove(subTempFile.Name()) - _, _ = tf.Write(tc.TopLevelFilePreFormat) - tf.Close() + _, _ = subTempFile.Write(tc.LowerLevelFilePreFormat) + subTempFile.Close() - subTf, err := ioutil.TempFile(superSubDir, "*.pkr.hcl") - if err != nil { - t.Fatalf("failed to create sub level tempfile for test %s", err) - } - defer os.Remove(subTf.Name()) - - _, _ = subTf.Write(tc.LowerLevelFilePreFormat) - subTf.Close() - - _, diags := f.Format(subDir) + _, diags := f.Format(topDir) if diags.HasErrors() { - t.Fatalf("the call to Format failed unexpectedly %s", diags.Error()) + t.Fatalf("the call to Format failed unexpectedly for test case: %s, errors: %s", tc.TestCaseName, diags.Error()) } - validateFileIsFormatted(t, tc.TopLevelFilePostFormat, tf) - validateFileIsFormatted(t, tc.LowerLevelFilePostFormat, subTf) + validateFileIsFormatted(t, tc.TopLevelFilePostFormat, topTempFile, tc.TestCaseName) + validateFileIsFormatted(t, tc.LowerLevelFilePostFormat, subTempFile, tc.TestCaseName) } -func validateFileIsFormatted(t *testing.T, formattedData []byte, testFile *os.File) { +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 %s", err) + t.Fatalf("failed to open the newly formatted fixture for test case: %s, errors: %s", testCaseName, err) } if diff := cmp.Diff(string(data), string(formattedData)); diff != "" { - t.Errorf("Unexpected format tfData output %s", diff) + t.Errorf("Unexpected format tfData output for test case: %s, errors: %s", testCaseName, diff) } } From ab7e89781a73d7aceef0393f18d74e25e3f8b3df Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Fri, 5 Feb 2021 14:05:37 +0100 Subject: [PATCH 06/19] empty commit to check if I can push From d3754e302195651b08f11219e2fc78155990a830 Mon Sep 17 00:00:00 2001 From: teddylear Date: Mon, 1 Feb 2021 21:33:04 -0500 Subject: [PATCH 07/19] Updating recursive formatter tests to be cleaner and table driven --- command/fmt_test.go | 209 +++++++++++------- .../test-fixtures/fmt/formatted.pkrvars.hcl | 3 - hcl2template/formatter_test.go | 209 ++++++++++-------- 3 files changed, 244 insertions(+), 177 deletions(-) delete mode 100644 command/test-fixtures/fmt/formatted.pkrvars.hcl diff --git a/command/fmt_test.go b/command/fmt_test.go index c66777a9e..8539fafb6 100644 --- a/command/fmt_test.go +++ b/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", - Recursion: true, - TopLevelFilePreFormat: unformattedData, - LowerLevelFilePreFormat: unformattedData, - TopLevelFilePostFormat: formattedData, - LowerLevelFilePostFormat: formattedData, + name: "nested formats recursively", + formatArgs: []string{"-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, + }, }, { - TestCaseName: "With Recursive flag off", - Recursion: false, - TopLevelFilePreFormat: unformattedData, - LowerLevelFilePreFormat: unformattedData, - TopLevelFilePostFormat: formattedData, - LowerLevelFilePostFormat: unformattedData, + name: "nested no recursive format", + formatArgs: []string{}, + 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, + }, }, } @@ -97,65 +106,93 @@ ami_filter_owners = ["137112412989"] Meta: testMeta(t), } - for _, tc := range recursiveTestCases { - executeRecursiveTestCase(t, tc, c) - } -} - -func executeRecursiveTestCase(t *testing.T, tc RecursiveTestCase, c *FormatCommand) { - // Creating temp directories and files - 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) - } - defer os.Remove(topDir) - - subDir, err := ioutil.TempDir(topDir, "sub-dir") - if err != nil { - t.Fatalf("failed to create sub level recurisve directory for test case: %s, error: %s", tc.TestCaseName, err) - } - defer os.Remove(subDir) - - topTempFile, err := ioutil.TempFile(topDir, "*.pkrvars.hcl") - if err != nil { - t.Fatalf("failed to create top level tempfile for test case: %s, error: %s", tc.TestCaseName, err) - } - defer os.Remove(topTempFile.Name()) - - _, _ = topTempFile.Write(tc.TopLevelFilePreFormat) - 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) - } - defer os.Remove(subTempFile.Name()) - - _, _ = subTempFile.Write(tc.LowerLevelFilePreFormat) - subTempFile.Close() - - var args []string - if tc.Recursion { - args = []string{"-recursive=true", topDir} - } else { - args = []string{topDir} - } - - if code := c.Run(args); code != 0 { - 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) - } - - if diff := cmp.Diff(string(data), string(formattedData)); diff != "" { - t.Errorf("Unexpected format tfData output for test case: %v, diff: %s", tc.TestCaseName, diff) + testFileName := "test.pkrvars.hcl" + + for _, tt := range tests { + topDir, err := ioutil.TempDir("test-fixtures/fmt", "temp-dir") + if err != nil { + t.Fatalf("Failed to create TopDir for test case: %s, error: %v", tt.name, err) + } + defer os.Remove(topDir) + + for testDir, content := range tt.alreadyPresentContent { + dir := filepath.Join(topDir, testDir) + err := os.MkdirAll(dir, 0777) + if err != nil { + os.RemoveAll(topDir) + t.Fatalf( + "Failed to create subDir: %s\n\n, for test case: %s\n\n, error: %v", + testDir, + tt.name, + err) + } + + file, err := os.Create(filepath.Join(dir, testFileName)) + if err != nil { + 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) + } + + _, err = file.Write([]byte(content)) + if err != nil { + 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) + } + + err = file.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) + } + } + + testArgs := append(tt.formatArgs, topDir) + 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) + } } + } diff --git a/command/test-fixtures/fmt/formatted.pkrvars.hcl b/command/test-fixtures/fmt/formatted.pkrvars.hcl deleted file mode 100644 index a85fa0aee..000000000 --- a/command/test-fixtures/fmt/formatted.pkrvars.hcl +++ /dev/null @@ -1,3 +0,0 @@ -ami_filter_name = "amzn2-ami-hvm-*-x86_64-gp2" -ami_filter_owners = ["137112412989"] - diff --git a/hcl2template/formatter_test.go b/hcl2template/formatter_test.go index 469237887..f59db4c0e 100644 --- a/hcl2template/formatter_test.go +++ b/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,90 +340,131 @@ 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{ - { - 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, - }, + testFileName := "test.pkrvars.hcl" + + for _, tt := range tests { + topDir, err := ioutil.TempDir("testdata/format", "temp-dir") + if err != nil { + t.Fatalf("Failed to create TopDir for test case: %s, error: %v", tt.name, err) + } + + for testDir, content := range tt.alreadyPresentContent { + dir := filepath.Join(topDir, testDir) + err := os.MkdirAll(dir, 0777) + if err != nil { + os.RemoveAll(topDir) + t.Fatalf( + "Failed to create subDir: %s\n\n, for test case: %s\n\n, error: %v", + testDir, + tt.name, + err) + } + + file, err := os.Create(filepath.Join(dir, testFileName)) + if err != nil { + 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) + } + + _, err = file.Write([]byte(content)) + if err != nil { + 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) + } + + err = file.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() { + os.RemoveAll(topDir) + t.Fatalf("the call to Format failed unexpectedly for test case: %s, errors: %s", tt.name, diags.Error()) + } + + 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) + } } - for _, tc := range recursiveTestCases { - executeRecursiveTestCase(t, tc, f) - } -} - -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) - } - defer os.Remove(topDir) - - var subDir string - subDir, err = ioutil.TempDir(topDir, "sub-dir") - if err != nil { - t.Fatalf("failed to create sub level recurisve directory for test case: %s, errors: %s", tc.TestCaseName, err) - } - defer os.Remove(subDir) - - topTempFile, err := ioutil.TempFile(topDir, "*.pkr.hcl") - if err != nil { - t.Fatalf("failed to create top level tempfile for test case: %s, errors: %s", tc.TestCaseName, err) - } - defer os.Remove(topTempFile.Name()) - - _, _ = topTempFile.Write(tc.TopLevelFilePreFormat) - 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) - } - defer os.Remove(subTempFile.Name()) - - _, _ = subTempFile.Write(tc.LowerLevelFilePreFormat) - subTempFile.Close() - - _, 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()) - } - - validateFileIsFormatted(t, tc.TopLevelFilePostFormat, topTempFile, tc.TestCaseName) - validateFileIsFormatted(t, tc.LowerLevelFilePostFormat, subTempFile, tc.TestCaseName) -} - -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) - } - - if diff := cmp.Diff(string(data), string(formattedData)); diff != "" { - t.Errorf("Unexpected format tfData output for test case: %s, errors: %s", testCaseName, diff) - } } func TestHCL2Formatter_Format_Write(t *testing.T) { From 0637601eda8a42a3a1297a75e73465c2f3568b94 Mon Sep 17 00:00:00 2001 From: teddylear Date: Wed, 10 Feb 2021 21:10:34 -0500 Subject: [PATCH 08/19] Fixing recursive formatting tests to work on all platforms --- command/fmt_test.go | 66 +++++++++-------------------- hcl2template/formatter_test.go | 76 ++++++++++------------------------ 2 files changed, 42 insertions(+), 100 deletions(-) diff --git a/command/fmt_test.go b/command/fmt_test.go index 8539fafb6..e190e7a15 100644 --- a/command/fmt_test.go +++ b/command/fmt_test.go @@ -106,58 +106,40 @@ ami_filter_owners = ["137112412989"] Meta: testMeta(t), } - testFileName := "test.pkrvars.hcl" + testDir := "test-fixtures/fmt" for _, tt := range tests { - topDir, err := ioutil.TempDir("test-fixtures/fmt", "temp-dir") + tempFileNames := make(map[string]string) + + tempDirectory, err := ioutil.TempDir(testDir, "test-dir-*") if err != nil { - t.Fatalf("Failed to create TopDir for test case: %s, error: %v", tt.name, err) + t.Fatalf("Failed to create temp dir for test case: %s, error: %v", tt.name, err) } - defer os.Remove(topDir) + defer os.RemoveAll(tempDirectory) - for testDir, content := range tt.alreadyPresentContent { - dir := filepath.Join(topDir, testDir) - err := os.MkdirAll(dir, 0777) + for subDir, content := range tt.alreadyPresentContent { + dir := filepath.Join(tempDirectory, subDir) + err = os.MkdirAll(dir, 0700) if err != nil { - os.RemoveAll(topDir) - t.Fatalf( - "Failed to create subDir: %s\n\n, for test case: %s\n\n, error: %v", - testDir, - tt.name, - err) + t.Fatalf("Failed to create directory for test case: %s, error: %v", tt.name, err) } - file, err := os.Create(filepath.Join(dir, testFileName)) + tempFile, err := ioutil.TempFile(dir, "*.pkrvars.hcl") if err != nil { - 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) + t.Fatalf("Failed to create temp file for test case: %s, error: %v", tt.name, err) } - _, err = file.Write([]byte(content)) + _, err = tempFile.Write([]byte(content)) if err != nil { - 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) - } - - err = file.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) + t.Fatalf("Failed to write temp file for test case: %s, error: %v", tt.name, err) } + tempFileNames[subDir] = tempFile.Name() + tempFile.Close() } - testArgs := append(tt.formatArgs, topDir) + testArgs := append(tt.formatArgs, tempDirectory) if code := c.Run(testArgs); code != 0 { - os.RemoveAll(topDir) + os.RemoveAll(tempDirectory) ui := c.Meta.Ui.(*packersdk.BasicUi) out := ui.Writer.(*bytes.Buffer) err := ui.ErrorWriter.(*bytes.Buffer) @@ -169,14 +151,12 @@ ami_filter_owners = ["137112412989"] } for expectedPath, expectedContent := range tt.expectedContent { - b, err := ioutil.ReadFile(filepath.Join(topDir, expectedPath, testFileName)) + b, err := ioutil.ReadFile(tempFileNames[expectedPath]) 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, @@ -185,14 +165,6 @@ ami_filter_owners = ["137112412989"] 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) - } } } diff --git a/hcl2template/formatter_test.go b/hcl2template/formatter_test.go index f59db4c0e..4fede302e 100644 --- a/hcl2template/formatter_test.go +++ b/hcl2template/formatter_test.go @@ -341,6 +341,10 @@ build { } } ` + var buf bytes.Buffer + f := NewHCL2Formatter() + f.Output = &buf + f.Write = true tests := []struct { name string @@ -378,75 +382,50 @@ build { }, } - var buf bytes.Buffer - f := NewHCL2Formatter() - f.Output = &buf - f.Write = true - - testFileName := "test.pkrvars.hcl" + testDir := "testdata/format" for _, tt := range tests { - topDir, err := ioutil.TempDir("testdata/format", "temp-dir") + tempFileNames := make(map[string]string) + + tempDirectory, err := ioutil.TempDir(testDir, "test-dir-*") if err != nil { - t.Fatalf("Failed to create TopDir for test case: %s, error: %v", tt.name, err) + t.Fatalf("Failed to create temp dir for test case: %s, error: %v", tt.name, err) } + defer os.RemoveAll(tempDirectory) - for testDir, content := range tt.alreadyPresentContent { - dir := filepath.Join(topDir, testDir) - err := os.MkdirAll(dir, 0777) + for subDir, content := range tt.alreadyPresentContent { + dir := filepath.Join(tempDirectory, subDir) + err = os.MkdirAll(dir, 0700) if err != nil { - os.RemoveAll(topDir) - t.Fatalf( - "Failed to create subDir: %s\n\n, for test case: %s\n\n, error: %v", - testDir, - tt.name, - err) + t.Fatalf("Failed to create directory for test case: %s, error: %v", tt.name, err) } - file, err := os.Create(filepath.Join(dir, testFileName)) + tempFile, err := ioutil.TempFile(dir, "*.pkrvars.hcl") if err != nil { - 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) + t.Fatalf("Failed to create temp file for test case: %s, error: %v", tt.name, err) } - _, err = file.Write([]byte(content)) + _, err = tempFile.Write([]byte(content)) if err != nil { - 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) - } - - err = file.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) + t.Fatalf("Failed to write temp file for test case: %s, error: %v", tt.name, err) } + tempFileNames[subDir] = tempFile.Name() + tempFile.Close() } f.Recursive = tt.recursive - _, diags := f.Format(topDir) + _, diags := f.Format(tempDirectory) if diags.HasErrors() { - os.RemoveAll(topDir) - t.Fatalf("the call to Format failed unexpectedly for test case: %s, errors: %s", tt.name, diags.Error()) + t.Fatalf("Call to Format failed unexpectedly for test case: %s, errors: %s", tt.name, diags.Error()) } for expectedPath, expectedContent := range tt.expectedContent { - b, err := ioutil.ReadFile(filepath.Join(topDir, expectedPath, testFileName)) + b, err := ioutil.ReadFile(tempFileNames[expectedPath]) 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, @@ -455,16 +434,7 @@ build { 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) - } } - } func TestHCL2Formatter_Format_Write(t *testing.T) { From a115b428ac25b19e022cd701cf9dd73959b98773 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Fri, 12 Mar 2021 11:07:26 +0100 Subject: [PATCH 09/19] simplify fmt test case a little --- command/build_test.go | 24 +++------ command/fmt_test.go | 116 ++++++++++++++++-------------------------- command/utils_test.go | 8 +++ 3 files changed, 59 insertions(+), 89 deletions(-) diff --git a/command/build_test.go b/command/build_test.go index 812b5c4e2..de7988a5c 100644 --- a/command/build_test.go +++ b/command/build_test.go @@ -389,7 +389,7 @@ func TestBuild(t *testing.T) { t.Run(tt.name, func(t *testing.T) { defer tt.cleanup(t) run(t, tt.args, tt.expectedCode) - tt.fileCheck.verify(t) + tt.fileCheck.verify(t, "") }) } } @@ -732,7 +732,7 @@ func TestHCL2PostProcessorForceFlag(t *testing.T) { if code := c.Run(args); code != 0 { fatalCommand(t, c.Meta) } - fCheck.verify(t) + fCheck.verify(t, "") // Second build should override previous manifest UUID, _ = uuid.GenerateUUID() @@ -766,7 +766,7 @@ func TestHCL2PostProcessorForceFlag(t *testing.T) { if code := c.Run(args); code != 0 { fatalCommand(t, c.Meta) } - fCheck.verify(t) + fCheck.verify(t, "") } func TestBuildCommand_HCLOnlyExceptOptions(t *testing.T) { @@ -887,19 +887,19 @@ func (fc fileCheck) expectedFiles() []string { return expected } -func (fc fileCheck) verify(t *testing.T) { +func (fc fileCheck) verify(t *testing.T, dir string) { for _, f := range fc.expectedFiles() { - if !fileExists(f) { - t.Errorf("Expected to find %s", f) + if _, err := os.Stat(filepath.Join(dir, f)); err != nil { + t.Errorf("Expected to find %s: %v", f, err) } } for _, f := range fc.notExpected { - if fileExists(f) { + if _, err := os.Stat(filepath.Join(dir, f)); err == nil { t.Errorf("Expected to not find %s", f) } } for file, expectedContent := range fc.expectedContent { - content, err := ioutil.ReadFile(file) + content, err := ioutil.ReadFile(filepath.Join(dir, file)) if err != nil { t.Fatalf("ioutil.ReadFile: %v", err) } @@ -909,14 +909,6 @@ func (fc fileCheck) verify(t *testing.T) { } } -// fileExists returns true if the filename is found -func fileExists(filename string) bool { - if _, err := os.Stat(filename); err == nil { - return true - } - return false -} - // testCoreConfigBuilder creates a packer CoreConfig that has a file builder // available. This allows us to test a builder that writes files to disk. func testCoreConfigBuilder(t *testing.T) *packer.CoreConfig { diff --git a/command/fmt_test.go b/command/fmt_test.go index e190e7a15..9efd4a213 100644 --- a/command/fmt_test.go +++ b/command/fmt_test.go @@ -8,7 +8,6 @@ import ( "strings" "testing" - "github.com/google/go-cmp/cmp" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" "github.com/stretchr/testify/assert" ) @@ -56,12 +55,14 @@ func TestFmt_unfomattedTemlateDirectory(t *testing.T) { } func TestFmt_Recursive(t *testing.T) { - unformattedData := `ami_filter_name ="amzn2-ami-hvm-*-x86_64-gp2" + unformattedData := ` +ami_filter_name ="amzn2-ami-hvm-*-x86_64-gp2" ami_filter_owners =[ "137112412989" ] ` - formattedData := `ami_filter_name = "amzn2-ami-hvm-*-x86_64-gp2" + formattedData := ` +ami_filter_name = "amzn2-ami-hvm-*-x86_64-gp2" ami_filter_owners = ["137112412989"] ` @@ -70,35 +71,41 @@ ami_filter_owners = ["137112412989"] name string formatArgs []string // arguments passed to format alreadyPresentContent map[string]string - expectedContent map[string]string + fileCheck }{ { name: "nested formats recursively", formatArgs: []string{"-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, + "foo/bar/baz.pkr.hcl": unformattedData, + "foo/bar/baz/woo.pkrvars.hcl": unformattedData, + "potato": unformattedData, + "foo/bar/potato": unformattedData, + "bar.pkr.hcl": unformattedData, }, + fileCheck: fileCheck{ + expectedContent: map[string]string{ + "foo/bar/baz.pkr.hcl": formattedData, + "foo/bar/baz/woo.pkrvars.hcl": formattedData, + "potato": unformattedData, + "foo/bar/potato": unformattedData, + "bar.pkr.hcl": formattedData, + }}, }, { name: "nested no recursive format", formatArgs: []string{}, 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, + "foo/bar/baz.pkr.hcl": unformattedData, + "foo/bar/baz/woo.pkrvars.hcl": unformattedData, + "bar.pkr.hcl": unformattedData, }, + fileCheck: fileCheck{ + expectedContent: map[string]string{ + "foo/bar/baz.pkr.hcl": unformattedData, + "foo/bar/baz/woo.pkrvars.hcl": unformattedData, + "bar.pkr.hcl": formattedData, + }}, }, } @@ -109,62 +116,25 @@ ami_filter_owners = ["137112412989"] testDir := "test-fixtures/fmt" for _, tt := range tests { - tempFileNames := make(map[string]string) + t.Run(tt.name, func(t *testing.T) { + tempDirectory := mustString(ioutil.TempDir(testDir, "test-dir-*")) + defer os.RemoveAll(tempDirectory) - tempDirectory, err := ioutil.TempDir(testDir, "test-dir-*") - if err != nil { - t.Fatalf("Failed to create temp dir for test case: %s, error: %v", tt.name, err) - } - defer os.RemoveAll(tempDirectory) + createFiles(tempDirectory, tt.alreadyPresentContent) - for subDir, content := range tt.alreadyPresentContent { - dir := filepath.Join(tempDirectory, subDir) - err = os.MkdirAll(dir, 0700) - if err != nil { - t.Fatalf("Failed to create directory for test case: %s, error: %v", tt.name, err) - } - - tempFile, err := ioutil.TempFile(dir, "*.pkrvars.hcl") - if err != nil { - t.Fatalf("Failed to create temp file for test case: %s, error: %v", tt.name, err) - } - - _, err = tempFile.Write([]byte(content)) - if err != nil { - t.Fatalf("Failed to write temp file for test case: %s, error: %v", tt.name, err) - } - tempFileNames[subDir] = tempFile.Name() - tempFile.Close() - } - - testArgs := append(tt.formatArgs, tempDirectory) - if code := c.Run(testArgs); code != 0 { - os.RemoveAll(tempDirectory) - 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(tempFileNames[expectedPath]) - if err != nil { - t.Fatalf("ReadFile failed for test case: %s, error : %v", tt.name, err) - } - got := string(b) - if diff := cmp.Diff(got, expectedContent); diff != "" { - t.Errorf( - "format dir, unexpected result for test case: %s, path: %s, Expected: %s, Got: %s", + testArgs := append(tt.formatArgs, tempDirectory) + if code := c.Run(testArgs); code != 0 { + 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, - expectedPath, - expectedContent, - got) + out.String(), + err.String()) } - } - } + tt.fileCheck.verify(t, tempDirectory) + }) + } } diff --git a/command/utils_test.go b/command/utils_test.go index f381b2e98..dfb0a036b 100644 --- a/command/utils_test.go +++ b/command/utils_test.go @@ -39,3 +39,11 @@ func (c *configDirSingleton) dir(key string) string { c.dirs[key] = mustString(ioutil.TempDir("", "pkr-test-cfg-dir-"+key)) return c.dirs[key] } + +// fileExists returns true if the filename is found +func fileExists(filename string) bool { + if _, err := os.Stat(filename); err == nil { + return true + } + return false +} From 7d30a5d79d88381a49dfc79fdaf0ff7eaed2ebef Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Fri, 12 Mar 2021 11:18:38 +0100 Subject: [PATCH 10/19] remove duplicate tests --- hcl2template/formatter_test.go | 475 --------------------------------- 1 file changed, 475 deletions(-) diff --git a/hcl2template/formatter_test.go b/hcl2template/formatter_test.go index 4fede302e..36067f29e 100644 --- a/hcl2template/formatter_test.go +++ b/hcl2template/formatter_test.go @@ -2,14 +2,7 @@ package hcl2template import ( "bytes" - "io/ioutil" - "os" - "os/exec" - "path/filepath" - "strings" "testing" - - "github.com/google/go-cmp/cmp" ) func TestHCL2Formatter_Format(t *testing.T) { @@ -38,471 +31,3 @@ func TestHCL2Formatter_Format(t *testing.T) { } } } - -func TestHCL2Formatter_Recursive(t *testing.T) { - unformattedData := ` -// starts resources to provision them. -build { - sources = [ - "source.amazon-ebs.ubuntu-1604", - "source.virtualbox-iso.ubuntu-1204", - ] - - provisioner "shell" { - string = coalesce(null, "", "string") - int = "${41 + 1}" - int64 = "${42 + 1}" - bool = "true" - trilean = true - duration = "${9 + 1}s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a","b"], - ["c","d"] - ] - - nested { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a","b"], - ["c","d"] - ] - } - - nested_slice { - } - } - - provisioner "file" { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a","b"], - ["c","d"] - ] - - nested { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a","b"], - ["c","d"] - ] - } - - nested_slice { - } - } - - post-processor "amazon-import" { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a","b"], - ["c","d"] - ] - - nested { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a","b"], - ["c","d"] - ] - } - - nested_slice { - } - } -} -` - - formattedData := ` -// starts resources to provision them. -build { - sources = [ - "source.amazon-ebs.ubuntu-1604", - "source.virtualbox-iso.ubuntu-1204", - ] - - provisioner "shell" { - string = coalesce(null, "", "string") - int = "${41 + 1}" - int64 = "${42 + 1}" - bool = "true" - trilean = true - duration = "${9 + 1}s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a", "b"], - ["c", "d"] - ] - - nested { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a", "b"], - ["c", "d"] - ] - } - - nested_slice { - } - } - - provisioner "file" { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a", "b"], - ["c", "d"] - ] - - nested { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a", "b"], - ["c", "d"] - ] - } - - nested_slice { - } - } - - post-processor "amazon-import" { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a", "b"], - ["c", "d"] - ] - - nested { - string = "string" - int = 42 - int64 = 43 - bool = true - trilean = true - duration = "10s" - map_string_string = { - a = "b" - c = "d" - } - slice_string = [ - "a", - "b", - "c", - ] - slice_slice_string = [ - ["a", "b"], - ["c", "d"] - ] - } - - nested_slice { - } - } -} -` - var buf bytes.Buffer - f := NewHCL2Formatter() - f.Output = &buf - f.Write = true - - 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, - }, - }, - } - - testDir := "testdata/format" - - for _, tt := range tests { - tempFileNames := make(map[string]string) - - tempDirectory, err := ioutil.TempDir(testDir, "test-dir-*") - if err != nil { - t.Fatalf("Failed to create temp dir for test case: %s, error: %v", tt.name, err) - } - defer os.RemoveAll(tempDirectory) - - for subDir, content := range tt.alreadyPresentContent { - dir := filepath.Join(tempDirectory, subDir) - err = os.MkdirAll(dir, 0700) - if err != nil { - t.Fatalf("Failed to create directory for test case: %s, error: %v", tt.name, err) - } - - tempFile, err := ioutil.TempFile(dir, "*.pkrvars.hcl") - if err != nil { - t.Fatalf("Failed to create temp file for test case: %s, error: %v", tt.name, err) - } - - _, err = tempFile.Write([]byte(content)) - if err != nil { - t.Fatalf("Failed to write temp file for test case: %s, error: %v", tt.name, err) - } - tempFileNames[subDir] = tempFile.Name() - tempFile.Close() - } - - f.Recursive = tt.recursive - _, diags := f.Format(tempDirectory) - if diags.HasErrors() { - t.Fatalf("Call to Format failed unexpectedly for test case: %s, errors: %s", tt.name, diags.Error()) - } - - for expectedPath, expectedContent := range tt.expectedContent { - b, err := ioutil.ReadFile(tempFileNames[expectedPath]) - if err != nil { - t.Fatalf("ReadFile failed for test case: %s, error : %v", tt.name, err) - } - got := string(b) - if diff := cmp.Diff(got, expectedContent); diff != "" { - t.Errorf( - "format dir, unexpected result for test case: %s, path: %s, Expected: %s, Got: %s", - tt.name, - expectedPath, - expectedContent, - got) - } - } - } -} - -func TestHCL2Formatter_Format_Write(t *testing.T) { - - var buf bytes.Buffer - f := NewHCL2Formatter() - f.Output = &buf - f.Write = true - - unformattedData, err := ioutil.ReadFile("testdata/format/unformatted.pkr.hcl") - if err != nil { - t.Fatalf("failed to open the unformatted fixture %s", err) - } - - tf, err := ioutil.TempFile("", "*.pkr.hcl") - if err != nil { - t.Fatalf("failed to create tempfile for test %s", err) - } - defer os.Remove(tf.Name()) - - _, _ = tf.Write(unformattedData) - tf.Close() - - _, diags := f.Format(tf.Name()) - if diags.HasErrors() { - t.Fatalf("the call to Format failed unexpectedly %s", diags.Error()) - } - - //lets re-read the tempfile which should now be formatted - data, err := ioutil.ReadFile(tf.Name()) - if err != nil { - t.Fatalf("failed to open the newly formatted fixture %s", err) - } - - formattedData, err := ioutil.ReadFile("testdata/format/formatted.pkr.hcl") - if err != nil { - t.Fatalf("failed to open the formatted fixture %s", err) - } - - if diff := cmp.Diff(string(data), string(formattedData)); diff != "" { - t.Errorf("Unexpected format output %s", diff) - } -} - -func TestHCL2Formatter_Format_ShowDiff(t *testing.T) { - - if _, err := exec.LookPath("diff"); err != nil { - t.Skip("Skipping test because diff is not in the executable PATH") - } - - var buf bytes.Buffer - f := HCL2Formatter{ - Output: &buf, - ShowDiff: true, - } - - _, diags := f.Format("testdata/format/unformatted.pkr.hcl") - if diags.HasErrors() { - t.Fatalf("the call to Format failed unexpectedly %s", diags.Error()) - } - - diffHeader := ` ---- old/testdata/format/unformatted.pkr.hcl -+++ new/testdata/format/unformatted.pkr.hcl -@@ -1,149 +1,149 @@ -` - if !strings.Contains(buf.String(), diffHeader) { - t.Errorf("expected buf to contain a file diff, but instead we got %s", buf.String()) - } - -} From be7d7313c517660fcf58321506b20e30ee6c9ec6 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Fri, 12 Mar 2021 11:25:10 +0100 Subject: [PATCH 11/19] add tests for piping fmt --- command/fmt_test.go | 72 ++++++++++++++++++++++++++++++++------------- 1 file changed, 52 insertions(+), 20 deletions(-) diff --git a/command/fmt_test.go b/command/fmt_test.go index 9efd4a213..66eca802d 100644 --- a/command/fmt_test.go +++ b/command/fmt_test.go @@ -2,12 +2,14 @@ package command import ( "bytes" + "fmt" "io/ioutil" "os" "path/filepath" "strings" "testing" + "github.com/google/go-cmp/cmp" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" "github.com/stretchr/testify/assert" ) @@ -54,18 +56,20 @@ func TestFmt_unfomattedTemlateDirectory(t *testing.T) { } } -func TestFmt_Recursive(t *testing.T) { - unformattedData := ` +const ( + unformattedHCL = ` ami_filter_name ="amzn2-ami-hvm-*-x86_64-gp2" ami_filter_owners =[ "137112412989" ] ` - - formattedData := ` + formattedHCL = ` ami_filter_name = "amzn2-ami-hvm-*-x86_64-gp2" ami_filter_owners = ["137112412989"] ` +) + +func TestFmt_Recursive(t *testing.T) { tests := []struct { name string @@ -77,34 +81,34 @@ ami_filter_owners = ["137112412989"] name: "nested formats recursively", formatArgs: []string{"-recursive=true"}, alreadyPresentContent: map[string]string{ - "foo/bar/baz.pkr.hcl": unformattedData, - "foo/bar/baz/woo.pkrvars.hcl": unformattedData, - "potato": unformattedData, - "foo/bar/potato": unformattedData, - "bar.pkr.hcl": unformattedData, + "foo/bar/baz.pkr.hcl": unformattedHCL, + "foo/bar/baz/woo.pkrvars.hcl": unformattedHCL, + "potato": unformattedHCL, + "foo/bar/potato": unformattedHCL, + "bar.pkr.hcl": unformattedHCL, }, fileCheck: fileCheck{ expectedContent: map[string]string{ - "foo/bar/baz.pkr.hcl": formattedData, - "foo/bar/baz/woo.pkrvars.hcl": formattedData, - "potato": unformattedData, - "foo/bar/potato": unformattedData, - "bar.pkr.hcl": formattedData, + "foo/bar/baz.pkr.hcl": formattedHCL, + "foo/bar/baz/woo.pkrvars.hcl": formattedHCL, + "potato": unformattedHCL, + "foo/bar/potato": unformattedHCL, + "bar.pkr.hcl": formattedHCL, }}, }, { name: "nested no recursive format", formatArgs: []string{}, alreadyPresentContent: map[string]string{ - "foo/bar/baz.pkr.hcl": unformattedData, - "foo/bar/baz/woo.pkrvars.hcl": unformattedData, - "bar.pkr.hcl": unformattedData, + "foo/bar/baz.pkr.hcl": unformattedHCL, + "foo/bar/baz/woo.pkrvars.hcl": unformattedHCL, + "bar.pkr.hcl": unformattedHCL, }, fileCheck: fileCheck{ expectedContent: map[string]string{ - "foo/bar/baz.pkr.hcl": unformattedData, - "foo/bar/baz/woo.pkrvars.hcl": unformattedData, - "bar.pkr.hcl": formattedData, + "foo/bar/baz.pkr.hcl": unformattedHCL, + "foo/bar/baz/woo.pkrvars.hcl": unformattedHCL, + "bar.pkr.hcl": formattedHCL, }}, }, } @@ -138,3 +142,31 @@ ami_filter_owners = ["137112412989"] }) } } + +func Test_fmt_pipe(t *testing.T) { + + tc := []struct { + piped string + command []string + env []string + expected string + }{ + {unformattedHCL, []string{"fmt", "-"}, nil, formattedHCL + "\n"}, + } + + for _, tc := range tc { + t.Run(fmt.Sprintf("echo %q | packer %s", tc.piped, tc.command), func(t *testing.T) { + p := helperCommand(t, tc.command...) + p.Stdin = strings.NewReader(tc.piped) + p.Env = append(p.Env, tc.env...) + bs, err := p.Output() + if err != nil { + t.Fatalf("%v: %s", err, bs) + } + + if diff := cmp.Diff(tc.expected, string(bs)); diff != "" { + t.Fatalf("%s", diff) + } + }) + } +} From 22d373c2794f491b97b7982a1d1b54c519cab6f3 Mon Sep 17 00:00:00 2001 From: teddylear Date: Sun, 14 Mar 2021 19:42:53 -0400 Subject: [PATCH 12/19] Adding more debug logic --- command/fmt_test.go | 6 +++--- hcl2template/formatter.go | 9 +++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/command/fmt_test.go b/command/fmt_test.go index 66eca802d..fe539490b 100644 --- a/command/fmt_test.go +++ b/command/fmt_test.go @@ -159,13 +159,13 @@ func Test_fmt_pipe(t *testing.T) { p := helperCommand(t, tc.command...) p.Stdin = strings.NewReader(tc.piped) p.Env = append(p.Env, tc.env...) + fmt.Println(fmt.Sprintf("Path: %s", p.Path)) bs, err := p.Output() if err != nil { - t.Fatalf("%v: %s", err, bs) + t.Fatalf("Error occurred running command %v: %s", err, bs) } - if diff := cmp.Diff(tc.expected, string(bs)); diff != "" { - t.Fatalf("%s", diff) + t.Fatalf("Error in diff: %s", diff) } }) } diff --git a/hcl2template/formatter.go b/hcl2template/formatter.go index 42534805d..286d1b1e5 100644 --- a/hcl2template/formatter.go +++ b/hcl2template/formatter.go @@ -69,6 +69,15 @@ func (f *HCL2Formatter) Format(path string) (int, hcl.Diagnostics) { f.parser = hclparse.NewParser() } + fmt.Println(fmt.Sprintf("Path is %s", path)) + if path == "-" { + fmt.Println("Found right statement!!!!!") + bytesModified, diags = f.formatFile(path, diags, bytesModified) + return bytesModified, diags + } else { + fmt.Println("Did not hit the correct statement") + } + isDir, err := isDir(path) if err != nil { diags = append(diags, &hcl.Diagnostic{ From 832c0f2b2a36da65d837f66e752c9534a12c971e Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Mar 2021 11:41:14 +0100 Subject: [PATCH 13/19] actually run fmt tests and then remove debug statements --- command/exec_test.go | 2 ++ command/fmt_test.go | 2 +- hcl2template/formatter.go | 2 -- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/command/exec_test.go b/command/exec_test.go index fdad36c26..4986a4bf3 100644 --- a/command/exec_test.go +++ b/command/exec_test.go @@ -87,6 +87,8 @@ func TestHelperProcess(*testing.T) { switch cmd { case "console": os.Exit((&ConsoleCommand{Meta: commandMeta()}).Run(args)) + case "fmt": + os.Exit((&FormatCommand{Meta: commandMeta()}).Run(args)) case "inspect": os.Exit((&InspectCommand{Meta: commandMeta()}).Run(args)) case "build": diff --git a/command/fmt_test.go b/command/fmt_test.go index fe539490b..e7011dba9 100644 --- a/command/fmt_test.go +++ b/command/fmt_test.go @@ -151,7 +151,7 @@ func Test_fmt_pipe(t *testing.T) { env []string expected string }{ - {unformattedHCL, []string{"fmt", "-"}, nil, formattedHCL + "\n"}, + {unformattedHCL, []string{"fmt", "-"}, nil, formattedHCL}, } for _, tc := range tc { diff --git a/hcl2template/formatter.go b/hcl2template/formatter.go index 286d1b1e5..7f0f65a98 100644 --- a/hcl2template/formatter.go +++ b/hcl2template/formatter.go @@ -69,9 +69,7 @@ func (f *HCL2Formatter) Format(path string) (int, hcl.Diagnostics) { f.parser = hclparse.NewParser() } - fmt.Println(fmt.Sprintf("Path is %s", path)) if path == "-" { - fmt.Println("Found right statement!!!!!") bytesModified, diags = f.formatFile(path, diags, bytesModified) return bytesModified, diags } else { From a6321ac1372e7ab5056f428ae5804176d283b176 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Mar 2021 11:46:07 +0100 Subject: [PATCH 14/19] remove debug line --- hcl2template/formatter.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/hcl2template/formatter.go b/hcl2template/formatter.go index 7f0f65a98..3ea5f1505 100644 --- a/hcl2template/formatter.go +++ b/hcl2template/formatter.go @@ -72,8 +72,6 @@ func (f *HCL2Formatter) Format(path string) (int, hcl.Diagnostics) { if path == "-" { bytesModified, diags = f.formatFile(path, diags, bytesModified) return bytesModified, diags - } else { - fmt.Println("Did not hit the correct statement") } isDir, err := isDir(path) From 25ee6a19a63afabc1b1bee0b77dc79a80173c0bf Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Mar 2021 11:48:13 +0100 Subject: [PATCH 15/19] flatten if a little --- hcl2template/formatter.go | 52 +++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/hcl2template/formatter.go b/hcl2template/formatter.go index 3ea5f1505..059f91d28 100644 --- a/hcl2template/formatter.go +++ b/hcl2template/formatter.go @@ -85,34 +85,34 @@ func (f *HCL2Formatter) Format(path string) (int, hcl.Diagnostics) { } if !isDir { - bytesModified, diags = f.formatFile(path, diags, bytesModified) - } else { - fileInfos, err := ioutil.ReadDir(path) - if err != nil { - diag := &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Cannot read hcl directory", - Detail: err.Error(), - } - diags = append(diags, diag) - return bytesModified, diags - } + return f.formatFile(path, diags, bytesModified) + } - for _, fileInfo := range fileInfos { - filename := filepath.Join(path, fileInfo.Name()) - if fileInfo.IsDir() { - if f.Recursive { - var tempDiags hcl.Diagnostics - var tempBytesModified int - tempBytesModified, tempDiags = f.Format(filename) - bytesModified += tempBytesModified - diags = diags.Extend(tempDiags) - } - continue - } - if isHcl2FileOrVarFile(filename) { - bytesModified, diags = f.formatFile(filename, diags, bytesModified) + fileInfos, err := ioutil.ReadDir(path) + if err != nil { + diag := &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Cannot read hcl directory", + Detail: err.Error(), + } + diags = append(diags, diag) + return bytesModified, diags + } + + for _, fileInfo := range fileInfos { + filename := filepath.Join(path, fileInfo.Name()) + if fileInfo.IsDir() { + if f.Recursive { + var tempDiags hcl.Diagnostics + var tempBytesModified int + tempBytesModified, tempDiags = f.Format(filename) + bytesModified += tempBytesModified + diags = diags.Extend(tempDiags) } + continue + } + if isHcl2FileOrVarFile(filename) { + bytesModified, diags = f.formatFile(filename, diags, bytesModified) } } From 8ee8420408e4fc0f52ae34503bd9bd99d95d6bfe Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Mar 2021 11:49:03 +0100 Subject: [PATCH 16/19] simplify return --- hcl2template/formatter.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hcl2template/formatter.go b/hcl2template/formatter.go index 059f91d28..d59a9eb8f 100644 --- a/hcl2template/formatter.go +++ b/hcl2template/formatter.go @@ -70,8 +70,7 @@ func (f *HCL2Formatter) Format(path string) (int, hcl.Diagnostics) { } if path == "-" { - bytesModified, diags = f.formatFile(path, diags, bytesModified) - return bytesModified, diags + return f.formatFile(path, diags, bytesModified) } isDir, err := isDir(path) From fe21ff905d12a1a915f5c467ef7100e0cd8b5e14 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Mar 2021 11:50:43 +0100 Subject: [PATCH 17/19] test that folders containing a - file won't hang forever --- command/fmt_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/command/fmt_test.go b/command/fmt_test.go index e7011dba9..b63ca5fb9 100644 --- a/command/fmt_test.go +++ b/command/fmt_test.go @@ -86,6 +86,7 @@ func TestFmt_Recursive(t *testing.T) { "potato": unformattedHCL, "foo/bar/potato": unformattedHCL, "bar.pkr.hcl": unformattedHCL, + "-": unformattedHCL, }, fileCheck: fileCheck{ expectedContent: map[string]string{ @@ -94,6 +95,7 @@ func TestFmt_Recursive(t *testing.T) { "potato": unformattedHCL, "foo/bar/potato": unformattedHCL, "bar.pkr.hcl": formattedHCL, + "-": unformattedHCL, }}, }, { @@ -103,12 +105,14 @@ func TestFmt_Recursive(t *testing.T) { "foo/bar/baz.pkr.hcl": unformattedHCL, "foo/bar/baz/woo.pkrvars.hcl": unformattedHCL, "bar.pkr.hcl": unformattedHCL, + "-": unformattedHCL, }, fileCheck: fileCheck{ expectedContent: map[string]string{ "foo/bar/baz.pkr.hcl": unformattedHCL, "foo/bar/baz/woo.pkrvars.hcl": unformattedHCL, "bar.pkr.hcl": formattedHCL, + "-": unformattedHCL, }}, }, } From e0fe579837e86fefb2943708cda93f2e92753ca1 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Mar 2021 12:00:19 +0100 Subject: [PATCH 18/19] un-remove tests --- hcl2template/formatter_test.go | 76 ++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/hcl2template/formatter_test.go b/hcl2template/formatter_test.go index 36067f29e..139308882 100644 --- a/hcl2template/formatter_test.go +++ b/hcl2template/formatter_test.go @@ -2,7 +2,13 @@ package hcl2template import ( "bytes" + "io/ioutil" + "os" + "os/exec" + "strings" "testing" + + "github.com/google/go-cmp/cmp" ) func TestHCL2Formatter_Format(t *testing.T) { @@ -31,3 +37,73 @@ func TestHCL2Formatter_Format(t *testing.T) { } } } + +func TestHCL2Formatter_Format_Write(t *testing.T) { + + var buf bytes.Buffer + f := NewHCL2Formatter() + f.Output = &buf + f.Write = true + + unformattedData, err := ioutil.ReadFile("testdata/format/unformatted.pkr.hcl") + if err != nil { + t.Fatalf("failed to open the unformatted fixture %s", err) + } + + tf, err := ioutil.TempFile("", "*.pkr.hcl") + if err != nil { + t.Fatalf("failed to create tempfile for test %s", err) + } + defer os.Remove(tf.Name()) + + _, _ = tf.Write(unformattedData) + tf.Close() + + _, diags := f.Format(tf.Name()) + if diags.HasErrors() { + t.Fatalf("the call to Format failed unexpectedly %s", diags.Error()) + } + + //lets re-read the tempfile which should now be formatted + data, err := ioutil.ReadFile(tf.Name()) + if err != nil { + t.Fatalf("failed to open the newly formatted fixture %s", err) + } + + formattedData, err := ioutil.ReadFile("testdata/format/formatted.pkr.hcl") + if err != nil { + t.Fatalf("failed to open the formatted fixture %s", err) + } + + if diff := cmp.Diff(string(data), string(formattedData)); diff != "" { + t.Errorf("Unexpected format output %s", diff) + } +} + +func TestHCL2Formatter_Format_ShowDiff(t *testing.T) { + + if _, err := exec.LookPath("diff"); err != nil { + t.Skip("Skipping test because diff is not in the executable PATH") + } + + var buf bytes.Buffer + f := HCL2Formatter{ + Output: &buf, + ShowDiff: true, + } + + _, diags := f.Format("testdata/format/unformatted.pkr.hcl") + if diags.HasErrors() { + t.Fatalf("the call to Format failed unexpectedly %s", diags.Error()) + } + + diffHeader := ` +--- old/testdata/format/unformatted.pkr.hcl ++++ new/testdata/format/unformatted.pkr.hcl +@@ -1,149 +1,149 @@ +` + if !strings.Contains(buf.String(), diffHeader) { + t.Errorf("expected buf to contain a file diff, but instead we got %s", buf.String()) + } + +} From 160d932ccef5aafd74cc25d342a941f8c1ff8178 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Mar 2021 12:04:16 +0100 Subject: [PATCH 19/19] remove weir "Cannot tell wether " + path + " is a directory" error --- hcl2template/formatter.go | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/hcl2template/formatter.go b/hcl2template/formatter.go index d59a9eb8f..34d63e8de 100644 --- a/hcl2template/formatter.go +++ b/hcl2template/formatter.go @@ -69,21 +69,7 @@ func (f *HCL2Formatter) Format(path string) (int, hcl.Diagnostics) { f.parser = hclparse.NewParser() } - if path == "-" { - return f.formatFile(path, diags, bytesModified) - } - - isDir, err := isDir(path) - if err != nil { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Cannot tell wether " + path + " is a directory", - Detail: err.Error(), - }) - return bytesModified, diags - } - - if !isDir { + if s, err := os.Stat(path); err != nil || !s.IsDir() { return f.formatFile(path, diags, bytesModified) }