Add GolangCI based CI

.golangci.yml

@@ -0,0 +1,5 @@
+linters:
+  enable-all: true
+  disable:
+    - gochecknoglobals
+    - gochecknoinits

conf/Config_test.go

@@ -25,7 +25,7 @@ path /buh
 keep latest 10
 }`, "", &Config{
 			Plans: []Plan{
-				Plan{
+				{
 					Name:   "buh",
 					Paths:  []string{"/buh"},
 					Latest: 10,
@@ -41,12 +41,12 @@ keep 1d for 30d
 keep latest 10
 }`, "", &Config{
 			Plans: []Plan{
-				Plan{
+				{
 					Name:   "buh",
 					Paths:  []string{"/buh"},
 					Latest: 10,
 					Periods: []Period{
-						Period{
+						{
 							Frequency: 24 * time.Hour,
 							Age:       30 * 24 * time.Hour,
 						},
@@ -66,16 +66,16 @@ keep 1d   for 30d
 keep latest 10
     }`, "", &Config{
 			Plans: []Plan{
-				Plan{
+				{
 					Name:   "buh",
 					Paths:  []string{"/buh"},
 					Latest: 10,
 					Periods: []Period{
-						Period{
+						{
 							Frequency: 24 * time.Hour,
 							Age:       30 * 24 * time.Hour,
 						},
-						Period{
+						{
 							Frequency: time.Hour,
 							Age:       24 * time.Hour,
 						},
@@ -94,16 +94,16 @@ protect horse
 protect sheep
 }`, "", &Config{
 			Plans: []Plan{
-				Plan{
+				{
 					Name:   "buh",
 					Paths:  []string{"/buh", "/buh/2"},
 					Latest: 1,
 					Periods: []Period{
-						Period{
+						{
 							Frequency: 24 * time.Hour,
 							Age:       30 * 24 * time.Hour,
 						},
-						Period{
+						{
 							Frequency: time.Hour,
 							Age:       24 * time.Hour,
 						},

conf/Plan_test.go

@@ -65,7 +65,14 @@ func TestKeepError(t *testing.T) {
 		conf: c,
 	}
 
-	cases := []string{"keep -1d for 30d", "keep 1d for -30d", "keep -1d for -30d", "keep # comment", "keep 1d for 1s", "keep }"}
+	cases := []string{
+		"keep -1d for 30d",
+		"keep 1d for -30d",
+		"keep -1d for -30d",
+		"keep # comment",
+		"keep 1d for 1s",
+		"keep }",
+	}
 	for i, cc := range cases {
 		s.scanner = bufio.NewScanner(strings.NewReader(cc))
 		s.scanLine()

conf/state_test.go

@@ -75,7 +75,7 @@ func TestEnd(t *testing.T) {
 	p := &Plan{
 		Name:    "testplan",
 		Paths:   []string{"/"},
-		Periods: []Period{Period{Frequency: time.Second, Age: time.Hour}},
+		Periods: []Period{{Frequency: time.Second, Age: time.Hour}},
 	}
 
 	cases := []string{"}", "} # comment", "  } "}
@@ -102,7 +102,7 @@ func TestEndError(t *testing.T) {
 	p := &Plan{
 		Name:    "testplan",
 		conf:    c,
-		Periods: []Period{Period{Frequency: time.Second, Age: time.Hour}},
+		Periods: []Period{{Frequency: time.Second, Age: time.Hour}},
 	}
 
 	cases := []string{"}", "} # comment", "  } "}

main.go

@@ -38,7 +38,6 @@ var (
 
 	// Can be overridden when running tests.
 	stdout io.Writer = os.Stdout
-	stderr io.Writer = os.Stderr
 )
 
 func init() {
@@ -125,11 +124,14 @@ func clean(cmd *cobra.Command, args []string) error {
 	fd := int(confFile.Fd())
 	err = syscall.Flock(fd, syscall.LOCK_EX|syscall.LOCK_NB)
 	if err != nil {
-		return fmt.Errorf("Could not aquire lock on '%s'", confFile.Name())
+		return fmt.Errorf("could not aquire lock on '%s'", confFile.Name())
 	}
 
 	// make sure to unlock :)
-	defer syscall.Flock(fd, syscall.LOCK_UN)
+	defer func() {
+		// We can ignore errors here, we're exiting anyway.
+		_ = syscall.Flock(fd, syscall.LOCK_UN)
+	}()
 
 	lists, err := processAll(now, conf)
 	if err != nil {

main_test.go

@@ -15,16 +15,25 @@ func init() {
 
 	// Mute normal output when running tests.
 	stdout = ioutil.Discard
-	stderr = ioutil.Discard
 	rootCmd.SilenceUsage = true
 	rootCmd.SilenceErrors = true
 }
 
+const (
+	echoCommand = "echo"
+	failCommand = "false"
+)
+
 func TestGetList(t *testing.T) {
 	// We override the zfs command in tests, to avoid running the real zfs
 	// binary.
-	commandName = "echo"
-	commandArguments = []string{"-e", "-n", "playground/fs1@snap1\t1492989570\nplayground/fs1@snap2\t1492989572\nplayground/fs1@snap3\t1492989573\nplayground/fs1@snap4\t1492989574\nplayground/fs1@snap5\t1492989587\n"}
+	commandName = echoCommand
+	commandArguments = []string{"-e", "-n", `playground/fs1@snap1\t1492989570
+playground/fs1@snap2\t1492989572
+playground/fs1@snap3\t1492989573
+playground/fs1@snap4\t1492989574
+playground/fs1@snap5\t1492989587
+`}
 
 	list, err := getList("playground/fs1")
 	if err != nil {
@@ -50,7 +59,7 @@ func TestGetListMissingBinary(t *testing.T) {
 }
 
 func TestGetListError(t *testing.T) {
-	commandName = "false"
+	commandName = failCommand
 
 	list, err := getList("/pool/fs1")
 	if err == nil {
@@ -65,12 +74,12 @@ func TestGetListError(t *testing.T) {
 func TestReadConf(t *testing.T) {
 	expected := &conf.Config{
 		Plans: []conf.Plan{
-			conf.Plan{
+			{
 				Name:   "buh",
 				Paths:  []string{"/buh"},
 				Latest: 10,
 				Periods: []conf.Period{
-					conf.Period{
+					{
 						Frequency: 24 * time.Hour,
 						Age:       30 * 24 * time.Hour,
 					},
@@ -98,7 +107,7 @@ keep latest 10
 	}
 
 	defer tmpfile.Close()
-	tmpfile.Seek(0, 0)
+	_, _ = tmpfile.Seek(0, 0)
 
 	conf, err := readConf(tmpfile)
 	if err != nil {
@@ -135,7 +144,7 @@ func TestReadConfSyntaxError(t *testing.T) {
 	}
 
 	defer tmpfile.Close()
-	tmpfile.Seek(0, 0)
+	_, _ = tmpfile.Seek(0, 0)
 
 	conf, err := readConf(tmpfile)
 	if err == nil {
@@ -148,17 +157,22 @@ func TestReadConfSyntaxError(t *testing.T) {
 }
 
 func TestProcessAll(t *testing.T) {
-	commandName = "echo"
-	commandArguments = []string{"-e", "-n", "playground/fs1@snap1\t1492989570\nplayground/fs1@snap2\t1492989572\nplayground/fs1@snap3\t1492989573\nplayground/fs1@snap4\t1492989574\nplayground/fs1@snap5\t1492989587\n"}
+	commandName = echoCommand
+	commandArguments = []string{"-e", "-n", `playground/fs1@snap1\t1492989570
+playground/fs1@snap2\t1492989572
+playground/fs1@snap3\t1492989573
+playground/fs1@snap4\t1492989574
+playground/fs1@snap5\t1492989587
+`}
 
 	conf := &conf.Config{
 		Plans: []conf.Plan{
-			conf.Plan{
+			{
 				Name:   "buh",
 				Paths:  []string{"playground/fs1"},
 				Latest: 10,
 				Periods: []conf.Period{
-					conf.Period{
+					{
 						Frequency: 24 * time.Hour,
 						Age:       30 * 24 * time.Hour,
 					},
@@ -182,17 +196,22 @@ func TestProcessAll(t *testing.T) {
 }
 
 func TestProcessAllFail(t *testing.T) {
-	commandName = "false"
-	commandArguments = []string{"-e", "-n", "playground/fs1@snap1\t1492989570\nplayground/fs1@snap2\t1492989572\nplayground/fs1@snap3\t1492989573\nplayground/fs1@snap4\t1492989574\nplayground/fs1@snap5\t1492989587\n"}
+	commandName = failCommand
+	commandArguments = []string{"-e", "-n", `playground/fs1@snap1\t1492989570
+playground/fs1@snap2\t1492989572
+playground/fs1@snap3\t1492989573
+playground/fs1@snap4\t1492989574
+playground/fs1@snap5\t1492989587
+`}
 
 	conf := &conf.Config{
 		Plans: []conf.Plan{
-			conf.Plan{
+			{
 				Name:   "buh",
 				Paths:  []string{"playground/fs1"},
 				Latest: 10,
 				Periods: []conf.Period{
-					conf.Period{
+					{
 						Frequency: 24 * time.Hour,
 						Age:       30 * 24 * time.Hour,
 					},
@@ -234,7 +253,7 @@ func TestMainNoConfig(t *testing.T) {
 }
 
 func TestMainNoZFS(t *testing.T) {
-	commandName = "false"
+	commandName = failCommand
 
 	content := []byte(`
 plan buh {
@@ -271,8 +290,13 @@ keep latest 10
 
 func TestMainFull(t *testing.T) {
 	now = time.Unix(1492993419, 0)
-	commandName = "echo"
-	commandArguments = []string{"-e", "-n", "playground/fs1@snap1\t1492989570\nplayground/fs1@snap2\t1492989572\nplayground/fs1@snap3\t1492989573\nplayground/fs1@snap4\t1492989574\nplayground/fs1@snap5\t1492989587\n"}
+	commandName = echoCommand
+	commandArguments = []string{"-e", "-n", `playground/fs1@snap1\t1492989570
+playground/fs1@snap2\t1492989572
+playground/fs1@snap3\t1492989573
+playground/fs1@snap4\t1492989574
+playground/fs1@snap5\t1492989587
+`}
 	verbose = true
 	content := []byte(`
 plan buh {
@@ -313,14 +337,12 @@ func TestConcurrency(t *testing.T) {
 	// This will force clean() to wait for our mainWaitGroup.Done().
 	mainWaitGroup.Add(1)
 
+	var cleanErr error
 	go func() {
-		err := clean(nil, []string{tmpfile.Name()})
-		if err != nil {
-			t.Fatalf("clean() returned an error: %s", err.Error())
-		}
+		cleanErr = clean(nil, []string{tmpfile.Name()})
 	}()
 
-	// Give some time for the first clean() to aquire the lock.
+	// Give some time for the first clean() to acquire the lock.
 	time.Sleep(time.Millisecond * 100)
 
 	err := clean(nil, []string{tmpfile.Name()})
@@ -330,4 +352,8 @@ func TestConcurrency(t *testing.T) {
 
 	// Let the first clean() exit.
 	mainWaitGroup.Done()
+
+	if cleanErr != nil {
+		t.Fatalf("clean() returned an error: %s", cleanErr.Error())
+	}
 }

zfs/Snapshot.go

@@ -18,7 +18,7 @@ type (
 )
 
 var (
-	// ErrMalformedLine will be returned if output from zfs is unuseable.
+	// ErrMalformedLine will be returned if output from zfs is unusable.
 	ErrMalformedLine = errors.New("broken line")
 )
 

zfs/SnapshotList_test.go

@@ -122,13 +122,13 @@ func TestLatestEmpty(t *testing.T) {
 }
 
 func TestString(t *testing.T) {
-	out := fmt.Sprintf("%s", empty)
+	out := empty.String()
 
 	if out != "[ ]" {
 		t.Fatalf("String() returned wrong output for empty list")
 	}
 
-	out = fmt.Sprintf("%s", list)
+	out = list.String()
 
 	if len(out) < 10 {
 		t.Fatalf("String() returned wrong output for list")