Merge pull request Masterminds#34 from Masterminds/fix/33

Fixed Masterminds#33: xml parsing errors were returned from lookups
tony · Apr 27, 2016 · 56971a5 · 56971a5
2 parents c92afd1 + 519b965
commit 56971a5
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 5 deletions.
diff --git a/vcs_remote_lookup.go b/vcs_remote_lookup.go
@@ -90,10 +90,6 @@ func detectVcsFromRemote(vcsURL string) (Type, string, error) {
 		return t, vcsURL, nil
 	}
 
-	// Need to test for vanity or paths like golang.org/x/
-
-	// TODO: Test for 3xx redirect codes and handle appropriately.
-
 	// Pages like https://golang.org/x/net provide an html document with
 	// meta tags containing a location to work with. The go tool uses
 	// a meta tag with the name go-import which is what we use here.
@@ -117,10 +113,14 @@ func detectVcsFromRemote(vcsURL string) (Type, string, error) {
 		return NoVCS, "", ErrCannotDetectVCS
 	}
 	defer resp.Body.Close()
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		return NoVCS, "", ErrCannotDetectVCS
+	}
 
 	t, nu, err := parseImportFromBody(u, resp.Body)
 	if err != nil {
-		return NoVCS, "", err
+		// TODO(mattfarina): Log the parsing error
+		return NoVCS, "", ErrCannotDetectVCS
 	} else if t == "" || nu == "" {
 		return NoVCS, "", ErrCannotDetectVCS
 	}
@@ -299,6 +299,7 @@ func get(url string) ([]byte, error) {
 	}
 	defer resp.Body.Close()
 	if resp.StatusCode != 200 {
+		// TODO(mattfarina): log the failed status
 		return nil, fmt.Errorf("%s: %s", url, resp.Status)
 	}
 	b, err := ioutil.ReadAll(resp.Body)

diff --git a/vcs_remote_lookup_test.go b/vcs_remote_lookup_test.go
@@ -13,6 +13,8 @@ func TestVCSLookup(t *testing.T) {
 		"https://github.com/masterminds":                                   {work: false, t: Git},
 		"https://github.com/Masterminds/VCSTestRepo":                       {work: true, t: Git},
 		"https://bitbucket.org/mattfarina/testhgrepo":                      {work: true, t: Hg},
+		"https://bitbucket.org/mattfarina/repo-does-not-exist":             {work: false, t: Hg},
+		"https://bitbucket.org/mattfarina/private-repo-for-vcs-testing":    {work: false, t: Hg},
 		"https://launchpad.net/govcstestbzrrepo/trunk":                     {work: true, t: Bzr},
 		"https://launchpad.net/~mattfarina/+junk/mygovcstestbzrrepo":       {work: true, t: Bzr},
 		"https://launchpad.net/~mattfarina/+junk/mygovcstestbzrrepo/trunk": {work: true, t: Bzr},
@@ -56,6 +58,10 @@ func TestVCSLookup(t *testing.T) {
 			t.Errorf("Error detecting VCS from URL(%s): %s", u, err)
 		}
 
+		if err != nil && err != ErrCannotDetectVCS && c.work == false {
+			t.Errorf("Unexpected error returned (%s): %s", u, err)
+		}
+
 		if c.work == true && ty != c.t {
 			t.Errorf("Incorrect VCS type returned(%s)", u)
 		}