Add more unit-tests

This branch's aim is to improve the unit tests to get to > 80%
test coverage.

The only non-test / mechanical change is in the download logic:
this was changed to print all the errors encountered when failing
to download multiple assets as opposed to just the first one.
This is in `internal/lock.go`.

Makefile

@@ -30,7 +30,8 @@ build: dep
 	@go build -o grabit
 
 test: dep
-	@go test -race -coverprofile=coverage.out -v ./...
+	@go test -race -v -coverpkg=./... -coverprofile=coverage.out ./...
+	@go tool cover -func coverage.out | tail -n 1
 
 check: dep
 	@golangci-lint run --sort-results ./...

cmd/add.go

@@ -8,33 +8,47 @@ import (
 	"github.com/spf13/cobra"
 )
 
-func init() {
-	rootCmd.AddCommand(addCmd)
+func addAdd(cmd *cobra.Command) {
+	addCmd := &cobra.Command{
+		Use:   "add",
+		Short: "Add new resource",
+		Args:  cobra.MinimumNArgs(1),
+		RunE:  runAdd,
+	}
 	addCmd.Flags().String("algo", internal.RecommendedAlgo, "Integrity algorithm")
 	addCmd.Flags().String("filename", "", "Target file name to use when downloading the resource")
 	addCmd.Flags().StringArray("tag", []string{}, "Resource tags")
+	cmd.AddCommand(addCmd)
 }
 
-var addCmd = &cobra.Command{
-	Use:   "add",
-	Short: "Add new resource",
-	Args:  cobra.MinimumNArgs(1),
-	Run:   runAdd,
-}
-
-func runAdd(cmd *cobra.Command, args []string) {
+func runAdd(cmd *cobra.Command, args []string) error {
 	lockFile, err := cmd.Flags().GetString("lock-file")
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
 	lock, err := internal.NewLock(lockFile, true)
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
 	algo, err := cmd.Flags().GetString("algo")
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
 	tags, err := cmd.Flags().GetStringArray("tag")
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
 	filename, err := cmd.Flags().GetString("filename")
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
 	err = lock.AddResource(args, algo, tags, filename)
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
 	err = lock.Save()
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
+	return nil
 }

cmd/add_test.go

@@ -0,0 +1,25 @@
+package cmd
+
+import (
+	"fmt"
+	"net/http"
+	"testing"
+
+	"github.com/cisco-open/grabit/test"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRunAdd(t *testing.T) {
+	handler := func(w http.ResponseWriter, r *http.Request) {
+		_, err := w.Write([]byte(`abcdef`))
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+	port, server := test.HttpHandler(handler)
+	defer server.Close()
+	cmd := NewRootCmd()
+	cmd.SetArgs([]string{"-f", test.TmpFile(t, ""), "add", fmt.Sprintf("http://localhost:%d/test.html", port)})
+	err := cmd.Execute()
+	assert.Nil(t, err)
+}

cmd/delete.go

@@ -8,25 +8,31 @@ import (
 	"github.com/spf13/cobra"
 )
 
-func init() {
-	rootCmd.AddCommand(delCmd)
-}
-
-var delCmd = &cobra.Command{
-	Use:   "delete",
-	Short: "Delete existing resources",
-	Args:  cobra.MinimumNArgs(1),
-	Run:   runDel,
+func addDelete(cmd *cobra.Command) {
+	var delCmd = &cobra.Command{
+		Use:   "delete",
+		Short: "Delete existing resources",
+		Args:  cobra.MinimumNArgs(1),
+		RunE:  runDel,
+	}
+	cmd.AddCommand(delCmd)
 }
 
-func runDel(cmd *cobra.Command, args []string) {
+func runDel(cmd *cobra.Command, args []string) error {
 	lockFile, err := cmd.Flags().GetString("lock-file")
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
 	lock, err := internal.NewLock(lockFile, false)
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
 	for _, r := range args {
 		lock.DeleteResource(r)
 	}
 	err = lock.Save()
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
+	return nil
 }

cmd/delete_test.go

@@ -0,0 +1,21 @@
+package cmd
+
+import (
+	"testing"
+
+	"github.com/cisco-open/grabit/test"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRunDelete(t *testing.T) {
+	testfilepath := test.TmpFile(t, `
+	[[Resource]]
+	Urls = ['http://localhost:123456/test.html']
+	Integrity = 'sha256-asdasdasd'
+	Tags = ['tag1', 'tag2']
+`)
+	cmd := NewRootCmd()
+	cmd.SetArgs([]string{"-f", testfilepath, "delete", "http://localhost:123456/test.html"})
+	err := cmd.Execute()
+	assert.Nil(t, err)
+}

cmd/dowload_test.go

@@ -0,0 +1,118 @@
+package cmd
+
+import (
+	"fmt"
+	"net/http"
+	"os"
+	"testing"
+
+	"github.com/cisco-open/grabit/test"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRunDownload(t *testing.T) {
+	handler := func(w http.ResponseWriter, r *http.Request) {
+		_, err := w.Write([]byte(`abcdef`))
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+	port, server := test.HttpHandler(handler)
+	defer server.Close()
+	testfilepath := test.TmpFile(t, fmt.Sprintf(`
+	[[Resource]]
+	Urls = ['http://localhost:%d/test.html']
+	Integrity = 'sha256-vvV+x/U6bUC+tkCngKY5yDvCmsipgW8fxsXG3Nk8RyE='
+`, port))
+	outputDir := test.TmpDir(t)
+	cmd := NewRootCmd()
+	cmd.SetArgs([]string{"-f", testfilepath, "download", "--dir", outputDir})
+	err := cmd.Execute()
+	assert.Nil(t, err)
+}
+
+func TestRunDownloadWithTags(t *testing.T) {
+	handler := func(w http.ResponseWriter, r *http.Request) {
+		_, err := w.Write([]byte(`abcdef`))
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+	port, server := test.HttpHandler(handler)
+	defer server.Close()
+	testfilepath := test.TmpFile(t, fmt.Sprintf(`
+	[[Resource]]
+	Urls = ['http://localhost:%d/test.html']
+	Integrity = 'sha256-vvV+x/U6bUC+tkCngKY5yDvCmsipgW8fxsXG3Nk8RyE='
+	Tags = ['tag']
+
+	[[Resource]]
+	Urls = ['http://localhost:%d/test2.html']
+	Integrity = 'sha256-vvV+x/U6bUC+tkCngKY5yDvCmsipgW8fxsXG3Nk8RyE='
+	Tags = ['tag1', 'tag2']
+`, port, port))
+	outputDir := test.TmpDir(t)
+	cmd := NewRootCmd()
+	cmd.SetArgs([]string{"-f", testfilepath, "download", "--tag", "tag", "--dir", outputDir})
+	err := cmd.Execute()
+	assert.Nil(t, err)
+	files, err := os.ReadDir(outputDir)
+	assert.Nil(t, err)
+	actualFiles := []string{}
+	for _, file := range files {
+		actualFiles = append(actualFiles, file.Name())
+	}
+	assert.ElementsMatch(t, []string{"test.html"}, actualFiles)
+}
+
+func TestRunDownloadWithoutTags(t *testing.T) {
+	handler := func(w http.ResponseWriter, r *http.Request) {
+		_, err := w.Write([]byte(`abcdef`))
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+	port, server := test.HttpHandler(handler)
+	defer server.Close()
+	testfilepath := test.TmpFile(t, fmt.Sprintf(`
+	[[Resource]]
+	Urls = ['http://localhost:%d/test.html']
+	Integrity = 'sha256-vvV+x/U6bUC+tkCngKY5yDvCmsipgW8fxsXG3Nk8RyE='
+	Tags = ['tag']
+
+	[[Resource]]
+	Urls = ['http://localhost:%d/test2.html']
+	Integrity = 'sha256-vvV+x/U6bUC+tkCngKY5yDvCmsipgW8fxsXG3Nk8RyE='
+	Tags = ['tag1', 'tag2']
+`, port, port))
+	outputDir := test.TmpDir(t)
+	cmd := NewRootCmd()
+	cmd.SetArgs([]string{"-f", testfilepath, "download", "--notag", "tag", "--dir", outputDir})
+	err := cmd.Execute()
+	assert.Nil(t, err)
+	files, err := os.ReadDir(outputDir)
+	assert.Nil(t, err)
+	actualFiles := []string{}
+	for _, file := range files {
+		actualFiles = append(actualFiles, file.Name())
+	}
+	assert.ElementsMatch(t, []string{"test2.html"}, actualFiles)
+}
+
+func TestRunDownloadMultipleErrors(t *testing.T) {
+	testfilepath := test.TmpFile(t, `
+	[[Resource]]
+	Urls = ['http://localhost:1234/test.html']
+	Integrity = 'sha256-vvV+x/U6bUC+tkCngKY5yDvCmsipgW8fxsXG3Nk8RyE='
+
+	[[Resource]]
+	Urls = ['http://cannot-be-resolved.no:12/test.html']
+	Integrity = 'sha256-vvV+x/U6bUC+tkCngKY5yDvCmsipgW8fxsXG3Nk8RyE='
+`)
+	cmd := NewRootCmd()
+	cmd.SetArgs([]string{"-f", testfilepath, "download"})
+	err := cmd.Execute()
+	assert.NotNil(t, err)
+	assert.Contains(t, err.Error(), "failed to download")
+	assert.Contains(t, err.Error(), "lookup cannot-be-resolved.no: no such host")
+}

cmd/download.go

@@ -8,34 +8,48 @@ import (
 	"github.com/spf13/cobra"
 )
 
-func init() {
-	rootCmd.AddCommand(downloadCmd)
+func addDownload(cmd *cobra.Command) {
+	downloadCmd := &cobra.Command{
+		Use:   "download",
+		Short: "Download defined resources",
+		Args:  cobra.NoArgs,
+		RunE:  runFetch,
+	}
 	downloadCmd.Flags().String("dir", ".", "Target directory where to store the files")
 	downloadCmd.Flags().StringArray("tag", []string{}, "Only download the resources with the given tag")
 	downloadCmd.Flags().StringArray("notag", []string{}, "Only download the resources without the given tag")
 	downloadCmd.Flags().String("perm", "", "Optional permissions for the downloaded files (e.g. '644')")
+	cmd.AddCommand(downloadCmd)
 }
 
-var downloadCmd = &cobra.Command{
-	Use:   "download",
-	Short: "Download defined resources",
-	Args:  cobra.NoArgs,
-	Run:   runFetch,
-}
-
-func runFetch(cmd *cobra.Command, args []string) {
+func runFetch(cmd *cobra.Command, args []string) error {
 	lockFile, err := cmd.Flags().GetString("lock-file")
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
 	lock, err := internal.NewLock(lockFile, false)
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
 	dir, err := cmd.Flags().GetString("dir")
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
 	tags, err := cmd.Flags().GetStringArray("tag")
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
 	notags, err := cmd.Flags().GetStringArray("notag")
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
 	perm, err := cmd.Flags().GetString("perm")
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
 	err = lock.Download(dir, tags, notags, perm)
-	FatalIfNotNil(err)
+	if err != nil {
+		return err
+	}
+	return nil
 }