Return []error from ValidateManifest and add tests

- Modifies ValidateManifest() to return []error, as a collection of all
the errors, instead of logging directly.
- Adds tests for ValidateManifest().

Author: darkowlzz

manifest.go

@@ -6,6 +6,7 @@ package dep
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"reflect"
 	"sort"
@@ -44,11 +45,12 @@ type rawProject struct {
 	Source   string `toml:"source,omitempty"`
 }
 
-func validateManifest(b *bytes.Buffer) error {
-	// Load the TomlTree from reader
-	tree, err := toml.Load(b.String())
+func validateManifest(s string) ([]error, error) {
+	var errs []error
+	// Load the TomlTree from string
+	tree, err := toml.Load(s)
 	if err != nil {
-		return errors.Wrap(err, "Unable to load TomlTree from string")
+		return errs, errors.Wrap(err, "Unable to load TomlTree from string")
 	}
 	// Convert tree to a map
 	manifest := tree.ToMap()
@@ -74,17 +76,17 @@ func validateManifest(b *bytes.Buffer) error {
 						}
 					}
 					if !isValidMetadata {
-						internal.Logf("WARNING: metadata should consist of key-value pairs")
+						errs = append(errs, errors.New("metadata should consist of key-value pairs"))
 					}
 				}
 			}
 		}
 		if !isValidField {
-			internal.Logf("WARNING: Unknown field in manifest: %v", prop)
+			errs = append(errs, fmt.Errorf("Unknown field in manifest: %v", prop))
 		}
 	}
 
-	return nil
+	return errs, nil
 }
 
 func readManifest(r io.Reader) (*Manifest, error) {
@@ -93,10 +95,15 @@ func readManifest(r io.Reader) (*Manifest, error) {
 	if err != nil {
 		return nil, errors.Wrap(err, "Unable to read byte stream")
 	}
-	err = validateManifest(buf)
+
+	// Validate manifest and log warnings
+	errs, err := validateManifest(buf.String())
 	if err != nil {
 		return nil, errors.Wrap(err, "Manifest validation failed")
 	}
+	for _, e := range errs {
+		internal.Logf("WARNING: %v", e)
+	}
 
 	raw := rawManifest{}
 	err = toml.Unmarshal(buf.Bytes(), &raw)

manifest_test.go

@@ -5,6 +5,7 @@
 package dep
 
 import (
+	"errors"
 	"reflect"
 	"strings"
 	"testing"
@@ -119,3 +120,74 @@ func TestReadManifestErrors(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateManifest(t *testing.T) {
+	cases := []struct {
+		tomlString string
+		want       []error
+	}{
+		{
+			tomlString: `
+			[[dependencies]]
+        name = "github.com/foo/bar"
+			`,
+			want: []error{},
+		},
+		{
+			tomlString: `
+			foo = "some-value"
+			version = 14
+
+			[[bar]]
+			  author = "xyz"
+
+			[[dependencies]]
+			  name = "github.com/foo/bar"
+			  version = ""
+			`,
+			want: []error{
+				errors.New("Unknown field in manifest: foo"),
+				errors.New("Unknown field in manifest: bar"),
+				errors.New("Unknown field in manifest: version"),
+			},
+		},
+		{
+			tomlString: `
+			metadata = "project-name"
+
+			[[dependencies]]
+			  name = "github.com/foo/bar"
+			`,
+			want: []error{errors.New("metadata should consist of key-value pairs")},
+		},
+	}
+
+	// constains for error
+	contains := func(s []error, e error) bool {
+		for _, a := range s {
+			if a.Error() == e.Error() {
+				return true
+			}
+		}
+		return false
+	}
+
+	for _, c := range cases {
+		errs, err := validateManifest(c.tomlString)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// compare length of error slice
+		if len(errs) != len(c.want) {
+			t.Fatalf("Number of manifest errors are not as expected: \n\t(GOT) %v errors(%v)\n\t(WNT) %v errors(%v).", len(errs), errs, len(c.want), c.want)
+		}
+
+		// check if the expected errors exist in actual errors slice
+		for _, er := range errs {
+			if !contains(c.want, er) {
+				t.Fatalf("Manifest errors are not as expected: \n\t(MISSING) %v\n\t(FROM) %v", er, c.want)
+			}
+		}
+	}
+}