Merge pull request cli#1155 from metalogical/FIX-detached-head

mislav · web-flow · commit ab903bdfc077 · 2020-06-24T17:01:22.000+02:00
fix regression in support for detached HEAD state
diff --git a/command/pr.go b/command/pr.go
@@ -131,7 +131,7 @@ func prStatus(cmd *cobra.Command, args []string) error {
 	repoOverride, _ := cmd.Flags().GetString("repo")
 	currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx, baseRepo)
 
-	if err != nil && repoOverride == "" && err.Error() != "git: not on any branch" {
+	if err != nil && repoOverride == "" && !errors.Is(err, git.ErrNotOnAnyBranch) {
 		return fmt.Errorf("could not query for pull request for current branch: %w", err)
 	}
 
diff --git a/command/pr_test.go b/command/pr_test.go
@@ -299,6 +299,38 @@ Requesting a code review from you
 	}
 }
 
+func TestPRStatus_detachedHead(t *testing.T) {
+	initBlankContext("", "OWNER/REPO", "")
+	http := initFakeHTTP()
+	http.StubRepoResponse("OWNER", "REPO")
+
+	http.StubResponse(200, bytes.NewBufferString(`
+	{ "data": {} }
+	`))
+
+	output, err := RunCommand("pr status")
+	if err != nil {
+		t.Errorf("error running command `pr status`: %v", err)
+	}
+
+	expected := `
+Relevant pull requests in OWNER/REPO
+
+Current branch
+  There is no current branch
+
+Created by you
+  You have no open pull requests
+
+Requesting a code review from you
+  You have no pull requests to review
+
+`
+	if output.String() != expected {
+		t.Errorf("expected %q, got %q", expected, output.String())
+	}
+}
+
 func TestPRList(t *testing.T) {
 	initBlankContext("", "OWNER/REPO", "master")
 	http := initFakeHTTP()
diff --git a/context/blank_context.go b/context/blank_context.go
@@ -40,7 +40,7 @@ func (c *blankContext) SetAuthToken(t string) {
 
 func (c *blankContext) Branch() (string, error) {
 	if c.branch == "" {
-		return "", fmt.Errorf("branch was not initialized")
+		return "", fmt.Errorf("branch was not initialized: %w", git.ErrNotOnAnyBranch)
 	}
 	return c.branch, nil
 }
diff --git a/git/git.go b/git/git.go
@@ -13,6 +13,9 @@ import (
 	"github.com/cli/cli/internal/run"
 )
 
+// ErrNotOnAnyBranch indicates that the users is in detached HEAD state
+var ErrNotOnAnyBranch = errors.New("git: not on any branch")
+
 // Ref represents a git commit reference
 type Ref struct {
 	Hash string
@@ -64,7 +67,7 @@ func CurrentBranch() (string, error) {
 	if errors.As(err, &cmdErr) {
 		if cmdErr.Stderr.Len() == 0 {
 			// Detached head
-			return "", errors.New("git: not on any branch")
+			return "", ErrNotOnAnyBranch
 		}
 	}
 
diff --git a/git/git_test.go b/git/git_test.go
@@ -67,9 +67,8 @@ func Test_CurrentBranch_detached_head(t *testing.T) {
 	if err == nil {
 		t.Errorf("expected an error")
 	}
-	expectedError := "git: not on any branch"
-	if err.Error() != expectedError {
-		t.Errorf("got unexpected error: %s instead of %s", err.Error(), expectedError)
+	if err != ErrNotOnAnyBranch {
+		t.Errorf("got unexpected error: %s instead of %s", err, ErrNotOnAnyBranch)
 	}
 	if len(cs.Calls) != 1 {
 		t.Errorf("expected 1 git call, saw %d", len(cs.Calls))

Original file line number	Diff line number	Diff line change
`@@ -131,7 +131,7 @@ func prStatus(cmd *cobra.Command, args []string) error {`
`131`	`131`	`repoOverride, _ := cmd.Flags().GetString("repo")`
`132`	`132`	`currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx, baseRepo)`
`133`	`133`
`134`		`- if err != nil && repoOverride == "" && err.Error() != "git: not on any branch" {`
	`134`	`+ if err != nil && repoOverride == "" && !errors.Is(err, git.ErrNotOnAnyBranch) {`
`135`	`135`	`return fmt.Errorf("could not query for pull request for current branch: %w", err)`
`136`	`136`	`}`
`137`	`137`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ func (c *blankContext) SetAuthToken(t string) {`
`40`	`40`
`41`	`41`	`func (c *blankContext) Branch() (string, error) {`
`42`	`42`	`if c.branch == "" {`
`43`		`- return "", fmt.Errorf("branch was not initialized")`
	`43`	`+ return "", fmt.Errorf("branch was not initialized: %w", git.ErrNotOnAnyBranch)`
`44`	`44`	`}`
`45`	`45`	`return c.branch, nil`
`46`	`46`	`}`
Original file line number	Diff line number	Diff line change
`@@ -13,6 +13,9 @@ import (`
`13`	`13`	`"github.com/cli/cli/internal/run"`
`14`	`14`	`)`
`15`	`15`
	`16`	`+// ErrNotOnAnyBranch indicates that the users is in detached HEAD state`
	`17`	`+var ErrNotOnAnyBranch = errors.New("git: not on any branch")`
	`18`	`+`
`16`	`19`	`// Ref represents a git commit reference`
`17`	`20`	`type Ref struct {`
`18`	`21`	`Hash string`
`@@ -64,7 +67,7 @@ func CurrentBranch() (string, error) {`
`64`	`67`	`if errors.As(err, &cmdErr) {`
`65`	`68`	`if cmdErr.Stderr.Len() == 0 {`
`66`	`69`	`// Detached head`
`67`		`- return "", errors.New("git: not on any branch")`
	`70`	`+ return "", ErrNotOnAnyBranch`
`68`	`71`	`}`
`69`	`72`	`}`
`70`	`73`
Original file line number	Diff line number	Diff line change
`@@ -67,9 +67,8 @@ func Test_CurrentBranch_detached_head(t *testing.T) {`
`67`	`67`	`if err == nil {`
`68`	`68`	`t.Errorf("expected an error")`
`69`	`69`	`}`
`70`		`- expectedError := "git: not on any branch"`
`71`		`- if err.Error() != expectedError {`
`72`		`- t.Errorf("got unexpected error: %s instead of %s", err.Error(), expectedError)`
	`70`	`+ if err != ErrNotOnAnyBranch {`
	`71`	`+ t.Errorf("got unexpected error: %s instead of %s", err, ErrNotOnAnyBranch)`
`73`	`72`	`}`
`74`	`73`	`if len(cs.Calls) != 1 {`
`75`	`74`	`t.Errorf("expected 1 git call, saw %d", len(cs.Calls))`