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

@@ -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 <hostname> <ip> 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 <domain> 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
 	}

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)
 	}
 }

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":

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))
+	}
+}