Test all branches of GH URI-parsing code

When a user adds a registry (e.g., through a command like `ks registry
add`, or implicitly through `ks init`) we must parse a URI pointing at
registry hosted on github.com, since we currently only support the
`"github"` protocol.

This logic is somewhat complex, since we must

  1. infer the location of a `registry.yaml` file given the URI, and
  2. accept and parse a broad set of URIs a user might provide.

For example, consider the following valid URIs, and what we do to infer
the location of the `registry.yaml` file that specifies a registry:

  URIs with an explicit `registry.yaml`:
    github.com/exampleOrg/exampleRepo/blob/master/registry.yaml
    github.com/exampleOrg/exampleRepo/blob/master/incubator/registry.yaml

  URIs with an implicit `registry.yaml`:
    github.com/exampleOrg/exampleRepo/
    github.com/exampleOrg/exampleRepo/tree/master

  URIs with different protocols (or no protocol specified)
    github.com/exampleOrg/exampleRepo
    http://github....

metadata/registry_managers.go

@@ -26,6 +26,10 @@ const (
 	resolvedSHAField = "resolvedSHA"
 )
 
+var (
+	errInvalidURI = fmt.Errorf("Invalid GitHub URI: try navigating in GitHub to the URI of the folder containing the 'registry.yaml', and using that URI instead. Generally, this URI should be of the form 'github.com/{organization}/{repository}/tree/{branch}/[path-to-directory]'")
+)
+
 //
 // GitHub registry manager.
 //
@@ -275,6 +279,10 @@ func (gh *gitHubRegistryManager) registrySpecRawURL() string {
 	return strings.Join([]string{rawGitHubRoot, gh.org, gh.repo, gh.GitVersion.RefSpec, gh.registrySpecRepoPath}, "/")
 }
 
+//
+// Helper functions.
+//
+
 func parseGitHubURI(uri string) (org, repo, refSpec, regRepoPath, regSpecRepoPath string, err error) {
 	// Normalize URI.
 	uri = strings.TrimSpace(uri)
@@ -310,7 +318,7 @@ func parseGitHubURI(uri string) (org, repo, refSpec, regRepoPath, regSpecRepoPat
 	//   * URI points at a directory inside the respoitory, e.g.,
 	//     'http://github.com/ksonnet/parts/tree/master/incubator'
 	//   * URI points at an 'app.yaml', e.g.,
-	//     'http://github.com/ksonnet/parts/blob/master/app.yaml'
+	//     'http://github.com/ksonnet/parts/blob/master/registry.yaml'
 	//   * URI points at a repository root, e.g.,
 	//     'http://github.com/ksonnet/parts'
 	//
@@ -324,13 +332,13 @@ func parseGitHubURI(uri string) (org, repo, refSpec, regRepoPath, regSpecRepoPat
 
 		// See note above about first component being blank.
 		if components[3] == "tree" {
-			regRepoPath = strings.Join(components[5:], "/")
-
-			// If we have a trailing '/' character, last component will be
-			// blank.
+			// If we have a trailing '/' character, last component will be blank. Make
+			// sure that `regRepoPath` does not contain a trailing `/`.
 			if components[len-1] == "" {
+				regRepoPath = strings.Join(components[5:len-1], "/")
 				components[len-1] = registryYAMLFile
 			} else {
+				regRepoPath = strings.Join(components[5:], "/")
 				components = append(components, registryYAMLFile)
 			}
 			regSpecRepoPath = strings.Join(components[5:], "/")
@@ -341,7 +349,7 @@ func parseGitHubURI(uri string) (org, repo, refSpec, regRepoPath, regSpecRepoPat
 			regSpecRepoPath = strings.Join(components[5:], "/")
 			return
 		} else {
-			return "", "", "", "", "", fmt.Errorf("Invalid GitHub URI: try navigating in GitHub to the URI of the folder containing the 'app.yaml', and using that URI instead. Generally, this URI should be of the form 'github.com/{organization}/{repository}/tree/{branch}/[path-to-directory]'")
+			return "", "", "", "", "", errInvalidURI
 		}
 	} else {
 		refSpec = defaultGitHubBranch

metadata/registry_test.go

@@ -2,12 +2,164 @@ package metadata
 
 import (
 	"path"
+	"testing"
 
 	"github.com/ksonnet/ksonnet/metadata/app"
 	"github.com/ksonnet/ksonnet/metadata/parts"
 	"github.com/ksonnet/ksonnet/metadata/registry"
 )
 
+func TestParseGiHubRegistryURITest(t *testing.T) {
+	tests := []struct {
+		// Specification to parse.
+		uri string
+
+		// Optional error to check.
+		targetErr error
+
+		// Optional results to verify.
+		targetOrg                  string
+		targetRepo                 string
+		targetRefSpec              string
+		targetRegistryRepoPath     string
+		targetRegistrySpecRepoPath string
+	}{
+		//
+		// `parseGitHubURI` should correctly parse org, repo, and refspec. Does not
+		// test path parsing.
+		//
+		{
+			uri: "github.com/exampleOrg1/exampleRepo1",
+
+			targetOrg:                  "exampleOrg1",
+			targetRepo:                 "exampleRepo1",
+			targetRefSpec:              "master",
+			targetRegistryRepoPath:     "",
+			targetRegistrySpecRepoPath: "registry.yaml",
+		},
+		{
+			uri: "github.com/exampleOrg2/exampleRepo2/tree/master",
+
+			targetOrg:                  "exampleOrg2",
+			targetRepo:                 "exampleRepo2",
+			targetRefSpec:              "master",
+			targetRegistryRepoPath:     "",
+			targetRegistrySpecRepoPath: "registry.yaml",
+		},
+		{
+			uri: "github.com/exampleOrg3/exampleRepo3/tree/exampleBranch1",
+
+			targetOrg:                  "exampleOrg3",
+			targetRepo:                 "exampleRepo3",
+			targetRefSpec:              "exampleBranch1",
+			targetRegistryRepoPath:     "",
+			targetRegistrySpecRepoPath: "registry.yaml",
+		},
+		{
+			// Fails because `blob` refers to a file, but this refers to a directory.
+			uri:       "github.com/exampleOrg4/exampleRepo4/blob/master",
+			targetErr: errInvalidURI,
+		},
+		{
+			uri: "github.com/exampleOrg4/exampleRepo4/tree/exampleBranch2",
+
+			targetOrg:                  "exampleOrg4",
+			targetRepo:                 "exampleRepo4",
+			targetRefSpec:              "exampleBranch2",
+			targetRegistryRepoPath:     "",
+			targetRegistrySpecRepoPath: "registry.yaml",
+		},
+
+		//
+		// Parsing URIs with paths.
+		//
+		{
+			// Fails because referring to a directory requires a URI with
+			// `tree/{branchName}` prepending the path.
+			uri:       "github.com/exampleOrg6/exampleRepo6/path/to/some/registry",
+			targetErr: errInvalidURI,
+		},
+		{
+			uri: "github.com/exampleOrg5/exampleRepo5/tree/master/path/to/some/registry",
+
+			targetOrg:                  "exampleOrg5",
+			targetRepo:                 "exampleRepo5",
+			targetRefSpec:              "master",
+			targetRegistryRepoPath:     "path/to/some/registry",
+			targetRegistrySpecRepoPath: "path/to/some/registry/registry.yaml",
+		},
+		{
+			uri: "github.com/exampleOrg6/exampleRepo6/tree/exampleBranch3/path/to/some/registry",
+
+			targetOrg:                  "exampleOrg6",
+			targetRepo:                 "exampleRepo6",
+			targetRefSpec:              "exampleBranch3",
+			targetRegistryRepoPath:     "path/to/some/registry",
+			targetRegistrySpecRepoPath: "path/to/some/registry/registry.yaml",
+		},
+		{
+			// Fails because `blob` refers to a file, but this refers to a directory.
+			uri:       "github.com/exampleOrg7/exampleRepo7/blob/master",
+			targetErr: errInvalidURI,
+		},
+		{
+			// Fails because `blob` refers to a file, but this refers to a directory.
+			uri:       "github.com/exampleOrg5/exampleRepo5/blob/exampleBranch2",
+			targetErr: errInvalidURI,
+		},
+	}
+
+	for _, test := range tests {
+		// Make sure we correctly parse each URN as a bare-domain URI, as well as
+		// with 'http://' and 'https://' as prefixes.
+		for _, prefix := range []string{"http://", "https://", "http://www.", "https://www.", "www.", ""} {
+			// Make sure we correctly parse each URI even if it has the optional
+			// trailing `/` character.
+			for _, suffix := range []string{"/", ""} {
+				uri := prefix + test.uri + suffix
+
+				t.Run(uri, func(t *testing.T) {
+					org, repo, refspec, registryRepoPath, registrySpecRepoPath, err := parseGitHubURI(uri)
+					if test.targetErr != nil {
+						if err != test.targetErr {
+							t.Fatalf("Expected URI '%s' parse to fail with err '%v', got: '%v'", uri, test.targetErr, err)
+						}
+						return
+					}
+
+					if err != nil {
+						t.Fatalf("Expected parse to succeed, but failed with error '%v'", err)
+					}
+
+					if org != test.targetOrg {
+						t.Errorf("Expected org '%s', got '%s'", test.targetOrg, org)
+					}
+
+					if repo != test.targetRepo {
+						t.Errorf("Expected repo '%s', got '%s'", test.targetRepo, repo)
+					}
+
+					if refspec != test.targetRefSpec {
+						t.Errorf("Expected refspec '%s', got '%s'", test.targetRefSpec, refspec)
+					}
+
+					if registryRepoPath != test.targetRegistryRepoPath {
+						t.Errorf("Expected registryRepoPath '%s', got '%s'", test.targetRegistryRepoPath, registryRepoPath)
+					}
+
+					if registrySpecRepoPath != test.targetRegistrySpecRepoPath {
+						t.Errorf("Expected targetRegistrySpecRepoPath '%s', got '%s'", test.targetRegistrySpecRepoPath, registrySpecRepoPath)
+					}
+				})
+			}
+		}
+	}
+}
+
+//
+// Mock registry manager for end-to-end tests.
+//
+
 type mockRegistryManager struct {
 	*app.RegistryRefSpec
 	registryDir string