rkt: refactor fetch, add tests

Unfortunately the recent fetch refactoring inverted the logic from what
it should be; we want to first check if the supplied image name is a
valid URL, and, if so, we should just pass it through. Otherwise,
newDiscoveryApp will return bogus results because
discovery.NewAppFromString does not expect to get URLs.

Also adds a few test cases for NewDiscoveryApp.

rkt/fetch.go

@@ -75,16 +75,15 @@ func runFetch(args []string) (exit int) {
 // fetchImage will take an image as either a URL or a name string and import it
 // into the store if found.
 func fetchImage(img string, ds *cas.Store) (string, error) {
-	var u *url.URL
-	var err error
-	if app := newDiscoveryApp(img); app != nil {
-		fmt.Printf("rkt: starting to discover app img %s\n", img)
-		u, err = discover(app)
-		if err != nil {
-			return "", fmt.Errorf("fetch: %v", err)
+	u, err := url.Parse(img)
+	if err == nil && u.Scheme == "" {
+		if app := newDiscoveryApp(img); app != nil {
+			fmt.Printf("rkt: starting to discover app img %s\n", img)
+			u, err = discover(app)
+			if err != nil {
+				return "", err
+			}
 		}
-	} else {
-		u, err = url.Parse(img)
 	}
 	if err != nil {
 		return "", fmt.Errorf("not a valid URL (%s)", img)

rkt/fetch_test.go

@@ -0,0 +1,87 @@
+package main
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/appc/spec/discovery"
+)
+
+func TestNewDiscoveryApp(t *testing.T) {
+	tests := []struct {
+		in string
+
+		w *discovery.App
+	}{
+		// not a valid AC name
+		{
+			"bad AC name",
+			nil,
+		},
+		// simple case - default arch, os should be substituted
+		{
+			"foo.com/bar",
+			&discovery.App{
+				Name: "foo.com/bar",
+				Labels: map[string]string{
+					"arch": defaultArch,
+					"os":   defaultOS,
+				},
+			},
+		},
+		// overriding arch, os should work
+		{
+			"www.abc.xyz/my/app,os=freebsd,arch=i386",
+			&discovery.App{
+				Name: "www.abc.xyz/my/app",
+				Labels: map[string]string{
+					"arch": "i386",
+					"os":   "freebsd",
+				},
+			},
+		},
+		// setting version should work
+		{
+			"yes.com/no:v1.2.3",
+			&discovery.App{
+				Name: "yes.com/no",
+				Labels: map[string]string{
+					"version": "v1.2.3",
+					"arch":    defaultArch,
+					"os":      defaultOS,
+				},
+			},
+		},
+		// arbitrary user-supplied labels
+		{
+			"example.com/foo/haha,val=one",
+			&discovery.App{
+				Name: "example.com/foo/haha",
+				Labels: map[string]string{
+					"val":  "one",
+					"arch": defaultArch,
+					"os":   defaultOS,
+				},
+			},
+		},
+		// combinations
+		{
+			"one.two/:three,os=four,foo=five,arch=six",
+			&discovery.App{
+				Name: "one.two/",
+				Labels: map[string]string{
+					"version": "three",
+					"os":      "four",
+					"foo":     "five",
+					"arch":    "six",
+				},
+			},
+		},
+	}
+	for i, tt := range tests {
+		g := newDiscoveryApp(tt.in)
+		if !reflect.DeepEqual(g, tt.w) {
+			t.Errorf("#%d: got %v, want %v", i, g, tt.w)
+		}
+	}
+}

test

@@ -14,8 +14,8 @@ COVER=${COVER:-"-cover"}
 
 source ./build
 
-TESTABLE_AND_FORMATTABLE="cas pkg/keystore pkg/lock pkg/tar stage1/init"
-FORMATTABLE="$TESTABLE_AND_FORMATTABLE metadatasvc path pkg/io pkg/proc rkt stage0/run.go version"
+TESTABLE_AND_FORMATTABLE="cas pkg/keystore pkg/lock pkg/tar rkt stage1/init"
+FORMATTABLE="$TESTABLE_AND_FORMATTABLE metadatasvc path pkg/io pkg/proc stage0/run.go version"
 
 # user has not provided PKG override
 if [ -z "$PKG" ]; then