From 1f7af74e2c06681507ca04cc53ec7aa61f5cecc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Kurid=C5=BEa?= Date: Thu, 26 May 2022 13:25:45 +0200 Subject: [PATCH] format: Output list and diff changes with --fail flag (#4508) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The change enables using --diff and --list together with --fail as discussed in #4508, for example: $ opa fmt [path [...]] --list --fail; exit $? path/to/file-1.rego path/to/file-2.rego unexpected diff 2 Previously, the same command returned only the error: $ opa fmt [path [...]] --list --fail; exit $? unexpected diff 2 Signed-off-by: David Kuridža --- cmd/fmt.go | 10 +++- cmd/fmt_test.go | 132 +++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 122 insertions(+), 20 deletions(-) diff --git a/cmd/fmt.go b/cmd/fmt.go index 2f6d62832c..7a9d69c28b 100644 --- a/cmd/fmt.go +++ b/cmd/fmt.go @@ -116,7 +116,7 @@ func formatFile(params *fmtCommandParams, out io.Writer, filename string, info o changed := !bytes.Equal(contents, formatted) - if params.fail { + if params.fail && !params.list && !params.diff { if changed { return newError("unexpected diff") } @@ -125,6 +125,10 @@ func formatFile(params *fmtCommandParams, out io.Writer, filename string, info o if params.list { if changed { fmt.Fprintln(out, filename) + + if params.fail { + return newError("unexpected diff") + } } return nil } @@ -138,6 +142,10 @@ func formatFile(params *fmtCommandParams, out io.Writer, filename string, info o } fmt.Fprintln(out, stdout.String()) + + if params.fail { + return newError("unexpected diff") + } } return nil } diff --git a/cmd/fmt_test.go b/cmd/fmt_test.go index af53296bbe..63228b1134 100644 --- a/cmd/fmt_test.go +++ b/cmd/fmt_test.go @@ -20,20 +20,20 @@ p { } ` +const unformatted = ` + package test + + p { a == 1; true + 1 + 3 + } + + +` + func TestFmtFormatFile(t *testing.T) { params := fmtCommandParams{} var stdout bytes.Buffer - unformatted := ` - package test - - p { a == 1; true - 1 + 3 - } - - - ` - files := map[string]string{ "policy.rego": unformatted, } @@ -76,6 +76,32 @@ func TestFmtFormatFileNoChanges(t *testing.T) { }) } +func TestFmtFailFormatFileNoChanges(t *testing.T) { + params := fmtCommandParams{ + fail: true, + diff: true, + } + var stdout bytes.Buffer + + files := map[string]string{ + "policy.rego": formatted, + } + + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(¶ms, &stdout, policyFile, info, err) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + actual := stdout.String() + if len(actual) > 0 { + t.Fatalf("Expected no output, got:\n%v\n\n", actual) + } + }) +} + func TestFmtFormatFileDiff(t *testing.T) { params := fmtCommandParams{ diff: true, @@ -128,6 +154,58 @@ func TestFmtFormatFileList(t *testing.T) { }) } +func TestFmtFailFormatFileList(t *testing.T) { + params := fmtCommandParams{ + fail: true, + list: true, + } + var stdout bytes.Buffer + + files := map[string]string{ + "policy.rego": formatted, + } + + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(¶ms, &stdout, policyFile, info, err) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + actual := strings.TrimSpace(stdout.String()) + if len(actual) > 0 { + t.Fatalf("Expected no output, got:\n%v\n\n", actual) + } + }) +} + +func TestFmtFailFormatFileChangesList(t *testing.T) { + params := fmtCommandParams{ + fail: true, + list: true, + } + var stdout bytes.Buffer + + files := map[string]string{ + "policy.rego": unformatted, + } + + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(¶ms, &stdout, policyFile, info, err) + if err == nil { + t.Fatalf("Unexpected error: %v", err) + } + + actual := strings.TrimSpace(stdout.String()) + if len(actual) == 0 { + t.Fatalf("Expected output, got:\n%v\n\n", actual) + } + }) +} + func TestFmtFailFileNoChanges(t *testing.T) { params := fmtCommandParams{ fail: true, @@ -152,15 +230,26 @@ func TestFmtFailFileChanges(t *testing.T) { fail: true, } - unformatted := ` - package test - - p { a == 1; true - 1 + 3 + files := map[string]string{ + "policy.rego": unformatted, } - - ` + test.WithTempFS(files, func(path string) { + policyFile := filepath.Join(path, "policy.rego") + info, err := os.Stat(policyFile) + err = formatFile(¶ms, ioutil.Discard, policyFile, info, err) + if err == nil { + t.Fatalf("Unexpected error: %s", err) + } + }) +} + +func TestFmtFailFileChangesDiff(t *testing.T) { + params := fmtCommandParams{ + diff: true, + fail: true, + } + var stdout bytes.Buffer files := map[string]string{ "policy.rego": unformatted, @@ -169,9 +258,14 @@ func TestFmtFailFileChanges(t *testing.T) { test.WithTempFS(files, func(path string) { policyFile := filepath.Join(path, "policy.rego") info, err := os.Stat(policyFile) - err = formatFile(¶ms, ioutil.Discard, policyFile, info, err) + err = formatFile(¶ms, &stdout, policyFile, info, err) if err == nil { - t.Fatalf("Unexpected error: %s", err) + t.Fatalf("Unexpected error: %v", err) + } + + actual := strings.TrimSpace(stdout.String()) + if len(actual) == 0 { + t.Fatalf("Expected output, got:\n%v\n\n", actual) } }) }