From 168ef0ce477e63f83e8ceb134073839c2453b3a7 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Fri, 13 Mar 2020 01:31:24 -0700 Subject: [PATCH] Update to fmt -n (preview) behavior - Fixing -n not being parsed by CLI - fmt -n will error on any duplicates or conflicts - fmt (without -n) will NOT error on duplicates; it will simply replace them - fmt (without -n) WILL error on conflicts, since it does not know how to fix them - fmt will exit 1 when an error state is reached --- commands.go | 46 ++++++++++++++++++++----------------- commands_test.go | 3 ++- main.go | 8 +++++-- main_test.go | 59 ++++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 90 insertions(+), 26 deletions(-) diff --git a/commands.go b/commands.go index 4ddadf0..b63d814 100644 --- a/commands.go +++ b/commands.go @@ -11,6 +11,8 @@ import ( "github.com/cbednarski/hostess/hostess" ) +var ErrParsingHostsFile = errors.New("Errors while parsing hostsfile. Please resolve any conflicts and try again.") + type Options struct { Preview bool } @@ -21,28 +23,30 @@ func PrintErrLn(err error) { } // LoadHostfile will try to load, parse, and return a Hostfile. If we -// encounter errors we will terminate, unless -f is passed. -func LoadHostfile() (*hostess.Hostfile, error) { +// encounter errors we will terminate. +func LoadHostfile(options *Options) (*hostess.Hostfile, error) { hosts, errs := hostess.LoadHostfile() - fatal := false + var err error if len(errs) > 0 { - for _, err := range errs { - PrintErrLn(err) + // If -n is passed, we'll always exit with an error code on duplicate. + // See issue 39 + if options.Preview { + err = ErrParsingHostsFile + } + + for _, currentErr := range errs { + PrintErrLn(currentErr) // If we find a duplicate we'll notify the user and continue. For // other errors we'll bail out. - if !strings.Contains(err.Error(), "duplicate hostname entry") { - fatal = true + if !strings.Contains(currentErr.Error(), "duplicate hostname entry") { + err = ErrParsingHostsFile } } } - if fatal { - return nil, errors.New("Errors while parsing hostsfile. Please resolve any conflicts and try again.") - } - - return hosts, nil + return hosts, err } // SaveOrPreview will display or write the Hostfile @@ -74,7 +78,7 @@ func StrPadRight(input string, length int) string { // Add command parses and adds or updates a hostname in the // hosts file func Add(options *Options, hostname, ip string) error { - hostsfile, err := LoadHostfile() + hostsfile, err := LoadHostfile(options) newHostname, err := hostess.NewHostname(hostname, ip, true) if err != nil { @@ -107,7 +111,7 @@ func Add(options *Options, hostname, ip string) error { // Remove command removes any hostname(s) matching from the hosts file func Remove(options *Options, hostname string) error { - hostsfile, err := LoadHostfile() + hostsfile, err := LoadHostfile(options) if err != nil { return err } @@ -128,7 +132,7 @@ func Remove(options *Options, hostname string) error { // Has command indicates whether a hostname is present in the hosts file func Has(options *Options, hostname string) error { - hostsfile, err := LoadHostfile() + hostsfile, err := LoadHostfile(options) if err != nil { return err } @@ -145,7 +149,7 @@ func Has(options *Options, hostname string) error { } func Enable(options *Options, hostname string) error { - hostsfile, err := LoadHostfile() + hostsfile, err := LoadHostfile(options) if err != nil { return err } @@ -164,7 +168,7 @@ func Enable(options *Options, hostname string) error { } func Disable(options *Options, hostname string) error { - hostsfile, err := LoadHostfile() + hostsfile, err := LoadHostfile(options) if err != nil { return err } @@ -191,7 +195,7 @@ func Disable(options *Options, hostname string) error { // List command shows a list of hostnames in the hosts file func List(options *Options) error { - hostsfile, err := LoadHostfile() + hostsfile, err := LoadHostfile(options) if err != nil { return err } @@ -222,7 +226,7 @@ func List(options *Options) error { // Format command removes duplicates from the hosts file func Format(options *Options) error { - hostsfile, err := LoadHostfile() + hostsfile, err := LoadHostfile(options) if err != nil { return err } @@ -237,7 +241,7 @@ func Format(options *Options) error { // Dump command outputs hosts file contents as JSON func Dump(options *Options) error { - hostsfile, err := LoadHostfile() + hostsfile, err := LoadHostfile(options) if err != nil { return err } @@ -258,7 +262,7 @@ func Apply(options *Options, filename string) error { return fmt.Errorf("Unable to read JSON from %s: %s", filename, err) } - hostfile, err := LoadHostfile() + hostfile, err := LoadHostfile(options) if err != nil { return err } diff --git a/commands_test.go b/commands_test.go index 465322c..0105966 100644 --- a/commands_test.go +++ b/commands_test.go @@ -31,8 +31,9 @@ func TestLoadHostfile(t *testing.T) { // Issue #39: This hosts file contains a duplicate. We should paper over it. os.Setenv("HOSTESS_PATH", filepath.Join("testdata", "issue39")) defer os.Unsetenv("HOSTESS_PATH") + options := &Options{} - if _, err := LoadHostfile(); err != nil { + if _, err := LoadHostfile(options); err != nil { t.Fatal(err) } } diff --git a/main.go b/main.go index 8e6c9fc..df384cb 100644 --- a/main.go +++ b/main.go @@ -70,7 +70,12 @@ func wrappedMain(args []string) error { fmt.Printf(help, hostess.GetHostsPath()) } - if err := cli.Parse(args[1:]); err != nil { + command := "" + if len(args) > 1 { + command = args[1] + } + + if err := cli.Parse(args[2:]); err != nil { return err } @@ -78,7 +83,6 @@ func wrappedMain(args []string) error { Preview: *preview, } - command := cli.Arg(0) switch command { case "-v", "--version", "version": diff --git a/main_test.go b/main_test.go index 550323a..4f72578 100644 --- a/main_test.go +++ b/main_test.go @@ -1,9 +1,11 @@ package main import ( + "bytes" "io" "io/ioutil" "os" + "path/filepath" "runtime" "strings" "testing" @@ -14,10 +16,20 @@ import ( // CopyHostsFile creates a temporary hosts file in the system temp directory, // sets the HOSTESS_PATH environment variable, and returns the file path and a // cleanup function -func CopyHostsFile(t *testing.T) (string, func()) { +func CopyHostsFile(t *testing.T, fixtureFiles ...string) (string, func()) { t.Helper() - fixture, err := os.Open("testdata/ubuntu.hosts") + fixtureFile := filepath.Join("testdata", "ubuntu.hosts") + + // This is an optional argument so we'll default to the ubuntu.hosts above + // and only accept arity 1 if the user passes in extra data + if len(fixtureFiles) > 1 { + t.Fatalf("%s supplied too many arguments to CopyHostsFile", t.Name()) + } else if len(fixtureFiles) == 1 { + fixtureFile = fixtureFiles[0] + } + + fixture, err := os.Open(fixtureFile) if err != nil { t.Fatal(err) } @@ -37,6 +49,7 @@ func CopyHostsFile(t *testing.T) (string, func()) { cleanup := func() { os.Remove(temp.Name()) + os.Unsetenv(hostess.EnvHostessPath) } return temp.Name(), cleanup @@ -227,3 +240,45 @@ ff02::2 ip6-allrouters t.Errorf("--- Expected ---\n%s\n--- Found ---\n%s\n", expected, output) } } + +func TestExitCodeFmt(t *testing.T) { + temp, cleanup := CopyHostsFile(t, filepath.Join("testdata", "issue39")) + defer cleanup() + + state1, err := ioutil.ReadFile(temp) + if err != nil { + t.Fatal(err) + } + + t.Logf("%s", state1) + + if err := wrappedMain([]string{"hostess", "fmt", "-n"}); err != ErrParsingHostsFile { + t.Fatalf(`Expected %q, found %v`, ErrParsingHostsFile, err) + } + + state2, err := ioutil.ReadFile(temp) + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal(state1, state2) { + t.Error("Expected hosts contents before and after fix -n to be the same") + } + + if err := wrappedMain([]string{"hostess", "fmt"}); err != nil { + t.Fatalf("Unexpected error: %s", err) + } + + finalExpected := `127.0.0.1 localhost kubernetes.docker.internal +::1 localhost +` + + state3, err := ioutil.ReadFile(temp) + if err != nil { + t.Fatal(err) + } + + if string(state3) != finalExpected { + t.Fatalf("---- Expected ----\n%s\n---- Found ----\n%s\n", finalExpected, string(state3)) + } +}