Repository: seachicken/gh-poi
Pull request #11 (2021-11-26 14:20:12)

Add error handling for SAML enforcement error
Added error handling because pull requests related to branches are not displayed when SAML is enabled.

Related PR:
https://github.com/cli/cli/pull/4241

codecov[bot]: # [Codecov](https://codecov.io/gh/seachicken/gh-poi/pull/11?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Seito+Tanaka) Report
> Merging [#11](https://codecov.io/gh/seachicken/gh-poi/pull/11?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Seito+Tanaka) (be600b3) into [main](https://codecov.io/gh/seachicken/gh-poi/commit/f6a4dedd4b6b02784dc625497ab3cce0be6795e1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Seito+Tanaka) (f6a4ded) will **decrease** coverage by `2.02%`.
> The diff coverage is `75.00%`.

[![Impacted file tree graph](https://codecov.io/gh/seachicken/gh-poi/pull/11/graphs/tree.svg?width=650&height=150&src=pr&token=tcPxPgst2q&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Seito+Tanaka)](https://codecov.io/gh/seachicken/gh-poi/pull/11?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Seito+Tanaka)

```diff
@@            Coverage Diff             @@
##             main      #11      +/-   ##
==========================================
- Coverage   82.02%   80.00%   -2.03%     
==========================================
  Files           3        3              
  Lines         395      415      +20     
==========================================
+ Hits          324      332       +8     
- Misses         61       69       +8     
- Partials       10       14       +4     
```


| [Impacted Files](https://codecov.io/gh/seachicken/gh-poi/pull/11?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Seito+Tanaka) | Coverage Δ | |
|---|---|---|
| [poi.go](https://codecov.io/gh/seachicken/gh-poi/pull/11/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Seito+Tanaka#diff-cG9pLmdv) | `85.92% <75.00%> (-3.58%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/seachicken/gh-poi/pull/11?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Seito+Tanaka).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Seito+Tanaka)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/seachicken/gh-poi/pull/11?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Seito+Tanaka). Last update [f6a4ded...be600b3](https://codecov.io/gh/seachicken/gh-poi/pull/11?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Seito+Tanaka). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Seito+Tanaka).


Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 57001be..c060630 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -26,7 +26,9 @@ jobs:
       run: go build -v ./...
 
     - name: Test
-      run: go test ./... -v -race -coverprofile=coverage.out -covermode=atomic
+      run: |
+        echo ${{ secrets.GITHUB_TOKEN }} | gh auth login --with-token
+        go test ./... -v -race -coverprofile=coverage.out -covermode=atomic
 
     - name: Upload coverage to Codecov
       uses: codecov/codecov-action@v2
diff --git a/connmock/stub.go b/connmock/stub.go
index 75195b4..5470462 100644
--- a/connmock/stub.go
+++ b/connmock/stub.go
@@ -38,6 +38,18 @@ func NewConf(times *Times) *Conf {
 	}
 }
 
+func (s *Stub) CheckRepos(err error, conf *Conf) *Stub {
+	s.t.Helper()
+	configure(
+		s.Conn.
+			EXPECT().
+			CheckRepos(gomock.Any()).
+			Return(err),
+		conf,
+	)
+	return s
+}
+
 func (s *Stub) GetRepoNames(path string, err error, conf *Conf) *Stub {
 	s.t.Helper()
 	configure(
diff --git a/mocks/poi_mock.go b/mocks/poi_mock.go
index 391bc86..8e5f4a8 100644
--- a/mocks/poi_mock.go
+++ b/mocks/poi_mock.go
@@ -33,6 +33,20 @@ func (m *MockConnection) EXPECT() *MockConnectionMockRecorder {
 	return m.recorder
 }
 
+// CheckRepos mocks base method.
+func (m *MockConnection) CheckRepos(repoNames []string) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "CheckRepos", repoNames)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// CheckRepos indicates an expected call of CheckRepos.
+func (mr *MockConnectionMockRecorder) CheckRepos(repoNames interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckRepos", reflect.TypeOf((*MockConnection)(nil).CheckRepos), repoNames)
+}
+
 // DeleteBranches mocks base method.
 func (m *MockConnection) DeleteBranches(branchNames []string) (string, error) {
 	m.ctrl.T.Helper()
diff --git a/poi.go b/poi.go
index 92f2515..7113c8b 100644
--- a/poi.go
+++ b/poi.go
@@ -14,6 +14,7 @@ import (
 
 type (
 	Connection interface {
+		CheckRepos(repoNames []string) error
 		GetRepoNames() (string, error)
 		GetBrancheNames() (string, error)
 		GetPullRequests(hostname string, repoNames []string, queryHashes string) (string, error)
@@ -66,13 +67,19 @@ const (
 var ErrNotFound = errors.New("not found")
 
 func GetBranches(conn Connection) ([]Branch, error) {
-	var hostname, origin, upstream string
+	var hostname string
+	var repoNames []string
 	if json, err := conn.GetRepoNames(); err == nil {
-		hostname, origin, upstream, err = getRepo(json)
+		hostname, repoNames, _ = getRepo(json)
 	} else {
 		return nil, err
 	}
 
+	err := conn.CheckRepos(repoNames)
+	if err != nil {
+		return nil, err
+	}
+
 	var branches []Branch
 	if names, err := conn.GetBrancheNames(); err == nil {
 		branches = toBranch(strings.Split(names, "\n"))
@@ -82,7 +89,7 @@ func GetBranches(conn Connection) ([]Branch, error) {
 
 	prs := []PullRequest{}
 	for _, queryHashes := range GetQueryHashes(branches) {
-		json, err := conn.GetPullRequests(hostname, []string{origin, upstream}, queryHashes)
+		json, err := conn.GetPullRequests(hostname, repoNames, queryHashes)
 		if err != nil {
 			return nil, err
 		}
@@ -173,7 +180,7 @@ func toBranch(branchNames []string) []Branch {
 	return results
 }
 
-func getRepo(jsonResp string) (string, string, string, error) {
+func getRepo(jsonResp string) (string, []string, error) {
 	type response struct {
 		Name  string
 		Owner struct {
@@ -190,13 +197,17 @@ func getRepo(jsonResp string) (string, string, string, error) {
 
 	var resp response
 	if err := json.Unmarshal([]byte(jsonResp), &resp); err != nil {
-		return "", "", "", fmt.Errorf("error unmarshaling response: %w", err)
+		return "", nil, fmt.Errorf("error unmarshaling response: %w", err)
 	}
 
-	return getHostname(resp.Url),
+	repoNames := []string{
 		resp.Owner.Login + "/" + resp.Name,
-		resp.Parent.Owner.Login + "/" + resp.Parent.Name,
-		nil
+	}
+	if len(resp.Parent.Name) > 0 {
+		repoNames = append(repoNames, resp.Parent.Owner.Login+"/"+resp.Parent.Name)
+	}
+
+	return getHostname(resp.Url), repoNames, nil
 }
 
 func getHostname(url string) string {
@@ -309,6 +320,18 @@ func branchNameExists(branchName string, branches []Branch) bool {
 	return false
 }
 
+func (conn *ConnectionImpl) CheckRepos(repoNames []string) error {
+	for _, name := range repoNames {
+		args := []string{
+			"api", "repos/" + name, "--silent",
+		}
+		if _, err := run("gh", args); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func (conn *ConnectionImpl) GetRepoNames() (string, error) {
 	args := []string{
 		"repo", "view",
@@ -383,5 +406,9 @@ func run(name string, args []string) (string, error) {
 	}
 	cmd.Wait()
 
+	if stderr.Len() > 0 {
+		return "", fmt.Errorf("failed to run external command: %s\n%s", name, stderr.String())
+	}
+
 	return stdout.String(), nil
 }


Test patch:
diff --git a/poi_test.go b/poi_test.go
index c04e8c9..c7c8ad4 100644
--- a/poi_test.go
+++ b/poi_test.go
@@ -14,6 +14,7 @@ func Test_ShouldBeDeletableWhenBranchesAssociatedWithMergedPR(t *testing.T) {
 	defer ctrl.Finish()
 
 	s := connmock.Setup(ctrl).
+		CheckRepos(nil, nil).
 		GetRepoNames("origin", nil, nil).
 		GetBrancheNames("@main_issue1", nil, nil).
 		GetPullRequests("issue1Merged", nil, nil)
@@ -39,6 +40,7 @@ func Test_ShouldBeDeletableWhenBranchesAssociatedWithUpstreamMergedPR(t *testing
 	defer ctrl.Finish()
 
 	s := connmock.Setup(ctrl).
+		CheckRepos(nil, nil).
 		GetRepoNames("origin_upstream", nil, nil).
 		GetBrancheNames("@main_issue1", nil, nil).
 		GetPullRequests("issue1UpMerged", nil, nil)
@@ -64,6 +66,7 @@ func Test_ShouldNotDeletableWhenBranchesAssociatedWithClosedPR(t *testing.T) {
 	defer ctrl.Finish()
 
 	s := connmock.Setup(ctrl).
+		CheckRepos(nil, nil).
 		GetRepoNames("origin", nil, nil).
 		GetBrancheNames("@main_issue1", nil, nil).
 		GetPullRequests("issue1Closed", nil, nil)
@@ -89,6 +92,7 @@ func Test_ShouldBeDeletableWhenBranchesAssociatedWithMergedAndClosedPRs(t *testi
 	defer ctrl.Finish()
 
 	s := connmock.Setup(ctrl).
+		CheckRepos(nil, nil).
 		GetRepoNames("origin", nil, nil).
 		GetBrancheNames("@main_issue1", nil, nil).
 		GetPullRequests("issue1Merged_issue1Closed", nil, nil)
@@ -122,11 +126,25 @@ func Test_ReturnsAnErrorWhenGetRepoNamesFails(t *testing.T) {
 	assert.NotNil(t, err)
 }
 
+func Test_ReturnsAnErrorWhenCheckReposFails(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	s := connmock.Setup(ctrl).
+		CheckRepos(errors.New("failed to run external command: gh"), nil).
+		GetRepoNames("origin", nil, nil)
+
+	_, err := GetBranches(s.Conn)
+
+	assert.NotNil(t, err)
+}
+
 func Test_ReturnsAnErrorWhenGetBrancheNamesFails(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
 	s := connmock.Setup(ctrl).
+		CheckRepos(nil, nil).
 		GetRepoNames("origin", nil, nil).
 		GetBrancheNames("@main_issue1", errors.New("failed to run external command: gh"), nil)
 
@@ -140,6 +158,7 @@ func Test_ReturnsAnErrorWhenGetPullRequestsFails(t *testing.T) {
 	defer ctrl.Finish()
 
 	s := connmock.Setup(ctrl).
+		CheckRepos(nil, nil).
 		GetRepoNames("origin", nil, nil).
 		GetBrancheNames("@main_issue1", nil, nil).
 		GetPullRequests("issue1Merged", errors.New("failed to run external command: gh"), nil)