Repository: google/go-github
Pull request #749 (2017-10-11 17:57:24)

Enhancement/add remove repository
Added support for preview installations API for the following:
- adding a repository to an installation
- removing a repository from an installation

shurcooL: > I've attempted to tackle this issue but my changes have failed in travis checks. Can someone please point me in the right direction, the changes are located in PR #749. I am new to Go and this github client in general, so I may be missing something obvious. Thank you everyone for your patience and advice! (: @shurcooL @gmlewis 

@MorrisLaw, you can click on the icon here to see the CI results:

![image](https://user-images.githubusercontent.com/1924134/31517363-aea6e982-af69-11e7-84d8-9ce7d51475d3.png)

It links to the [CI page](https://travis-ci.org/google/go-github/builds/286640168). There, you can see all 4 jobs failed. Pick the first one ([Go 1.9.x](https://travis-ci.org/google/go-github/jobs/286640169) job), and look at the log. If you scan towards the bottom, you'll see something in red, that's the first failure:

```
$ go test -v -race ./...
# github.com/google/go-github/github
import cycle not allowed in test
package github.com/google/go-github/github (test)
	imports github.com/google/go-github/github
FAIL	github.com/google/go-github/github [setup failed]
?   	github.com/google/go-github/example/appengine	[no test files]
?   	github.com/google/go-github/example/basicauth	[no test files]
?   	github.com/google/go-github/test/fields	[no test files]
?   	github.com/google/go-github/test/integration	[no test files]
The command "go test -v -race ./..." exited with 1.
```

The command that failed was `go test -v -race ./...`. You will need to fix that issue.
googlebot: We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s).  If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA ([login here](https://cla.developers.google.com/) to double check)?  If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

<!-- need_author_cla -->

Patch:
diff --git a/AUTHORS b/AUTHORS
index c15c465a..e97e992e 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -77,6 +77,7 @@ i2bskn <i2bskn@gmail.com>
 Isao Jonas <isao.jonas@gmail.com>
 isqua <isqua@isqua.ru>
 Jameel Haffejee <RC1140@republiccommandos.co.za>
+Jeremy Morris <jeremylevanmorris@gmail.com>
 Jan Kosecki <jan.kosecki91@gmail.com>
 Jihoon Chung <j.c@navercorp.com>
 Jimmi Dyson <jimmidyson@gmail.com>
diff --git a/github/apps_installation.go b/github/apps_installation.go
index 6a277995..5c932919 100644
--- a/github/apps_installation.go
+++ b/github/apps_installation.go
@@ -5,7 +5,10 @@
 
 package github
 
-import "context"
+import (
+	"context"
+	"fmt"
+)
 
 // Installation represents a GitHub Apps installation.
 type Installation struct {
@@ -47,3 +50,35 @@ func (s *AppsService) ListRepos(ctx context.Context, opt *ListOptions) ([]*Repos
 
 	return r.Repositories, resp, nil
 }
+
+// AddRepository adds a single repository to an installation.
+//
+// GitHub API docs: https://developer.github.com/v3/apps/installations/#add-repository-to-installation
+func (s *AppsService) AddRepository(ctx context.Context, instID, repoID int) (*Repository, *Response, error) {
+	u := fmt.Sprintf("apps/installations/%v/repositories/%v", instID, repoID)
+	req, err := s.client.NewRequest("PUT", u, nil)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	r := new(Repository)
+	resp, err := s.client.Do(ctx, req, r)
+	if err != nil {
+		return nil, resp, err
+	}
+
+	return r, resp, nil
+}
+
+// RemoveRepository removes a single repository from an installation.
+//
+// GitHub docs: https://developer.github.com/v3/apps/installations/#remove-repository-from-installation
+func (s *AppsService) RemoveRepository(ctx context.Context, instID, repoID int) (*Response, error) {
+	u := fmt.Sprintf("apps/installations/%v/repositories/%v", instID, repoID)
+	req, err := s.client.NewRequest("DELETE", u, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	return s.client.Do(ctx, req, nil)
+}
diff --git a/github/github.go b/github/github.go
index ae37ba8f..54769bc3 100644
--- a/github/github.go
+++ b/github/github.go
@@ -27,7 +27,7 @@ import (
 )
 
 const (
-	libraryVersion = "12"
+	libraryVersion = "13"
 	defaultBaseURL = "https://api.github.com/"
 	uploadBaseURL  = "https://uploads.github.com/"
 	userAgent      = "go-github/" + libraryVersion
diff --git a/github/projects.go b/github/projects.go
index 89685c9d..a9b143df 100644
--- a/github/projects.go
+++ b/github/projects.go
@@ -65,6 +65,12 @@ type ProjectOptions struct {
 	Name string `json:"name,omitempty"`
 	// The body of the project. (Optional.)
 	Body string `json:"body,omitempty"`
+
+	// The following field(s) are only applicable for update.
+	// They should be left with zero values for creation.
+
+	// State of the project. Either "open" or "closed". (Optional.)
+	State string `json:"state,omitempty"`
 }
 
 // UpdateProject updates a repository project.
diff --git a/github/repos.go b/github/repos.go
index b6d98cc1..5f444535 100644
--- a/github/repos.go
+++ b/github/repos.go
@@ -232,8 +232,6 @@ func (s *RepositoriesService) ListByOrg(ctx context.Context, org string, opt *Re
 type RepositoryListAllOptions struct {
 	// ID of the last repository seen
 	Since int `url:"since,omitempty"`
-
-	ListOptions
 }
 
 // ListAll lists all GitHub repositories in the order that they were created.


Test patch:
diff --git a/github/apps_installation_test.go b/github/apps_installation_test.go
index 462d15a3..69598cfe 100644
--- a/github/apps_installation_test.go
+++ b/github/apps_installation_test.go
@@ -38,3 +38,38 @@ func TestAppsService_ListRepos(t *testing.T) {
 		t.Errorf("Apps.ListRepos returned %+v, want %+v", repositories, want)
 	}
 }
+
+func TestAppsService_AddRepository(t *testing.T) {
+	setup()
+	defer teardown()
+
+	mux.HandleFunc("/apps/installations/1/repositories/1", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "PUT")
+		fmt.Fprint(w, `{"id":1,"name":"n","description":"d","owner":{"login":"l"},"license":{"key":"mit"}}`)
+	})
+
+	repo, _, err := client.Apps.AddRepository(context.Background(), 1, 1)
+	if err != nil {
+		t.Errorf("Apps.AddRepository returned error: %v", err)
+	}
+
+	want := &Repository{ID: Int(1), Name: String("n"), Description: String("d"), Owner: &User{Login: String("l")}, License: &License{Key: String("mit")}}
+	if !reflect.DeepEqual(repo, want) {
+		t.Errorf("AddRepository returned %+v, want %+v", repo, want)
+	}
+}
+
+func TestAppsService_RemoveRepository(t *testing.T) {
+	setup()
+	defer teardown()
+
+	mux.HandleFunc("/apps/installations/1/repositories/1", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "DELETE")
+		w.WriteHeader(http.StatusNoContent)
+	})
+
+	_, err := client.Apps.RemoveRepository(context.Background(), 1, 1)
+	if err != nil {
+		t.Errorf("Apps.RemoveRepository returned error: %v", err)
+	}
+}
diff --git a/github/github_test.go b/github/github_test.go
index 74d14759..aff436a6 100644
--- a/github/github_test.go
+++ b/github/github_test.go
@@ -33,17 +33,39 @@ var (
 	server *httptest.Server
 )
 
+const (
+	// baseURLPath is a non-empty Client.BaseURL path to use during tests,
+	// to ensure relative URLs are used for all endpoints. See issue #752.
+	baseURLPath = "/api-v3"
+)
+
 // setup sets up a test HTTP server along with a github.Client that is
 // configured to talk to that test server. Tests should register handlers on
 // mux which provide mock responses for the API method being tested.
 func setup() {
 	// test server
 	mux = http.NewServeMux()
-	server = httptest.NewServer(mux)
+
+	// We want to ensure that tests catch mistakes where the endpoint URL is
+	// specified as absolute rather than relative. It only makes a difference
+	// when there's a non-empty base URL path. So, use that. See issue #752.
+	apiHandler := http.NewServeMux()
+	apiHandler.Handle(baseURLPath+"/", http.StripPrefix(baseURLPath, mux))
+	apiHandler.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
+		fmt.Fprintln(os.Stderr, "FAIL: Client.BaseURL path prefix is not preserved in the request URL:")
+		fmt.Fprintln(os.Stderr)
+		fmt.Fprintln(os.Stderr, "\t"+req.URL.String())
+		fmt.Fprintln(os.Stderr)
+		fmt.Fprintln(os.Stderr, "\tDid you accidentally use an absolute endpoint URL rather than relative?")
+		fmt.Fprintln(os.Stderr, "\tSee https://github.com/google/go-github/issues/752 for information.")
+		http.Error(w, "Client.BaseURL path prefix is not preserved in the request URL.", http.StatusInternalServerError)
+	})
+
+	server = httptest.NewServer(apiHandler)
 
 	// github client configured to use test server
 	client = NewClient(nil)
-	url, _ := url.Parse(server.URL + "/")
+	url, _ := url.Parse(server.URL + baseURLPath + "/")
 	client.BaseURL = url
 	client.UploadURL = url
 }
@@ -201,7 +223,7 @@ func TestNewRequest_invalidJSON(t *testing.T) {
 	type T struct {
 		A map[interface{}]interface{}
 	}
-	_, err := c.NewRequest("GET", "/", &T{})
+	_, err := c.NewRequest("GET", ".", &T{})
 
 	if err == nil {
 		t.Error("Expected error to be returned.")
@@ -222,7 +244,7 @@ func TestNewRequest_badURL(t *testing.T) {
 func TestNewRequest_emptyUserAgent(t *testing.T) {
 	c := NewClient(nil)
 	c.UserAgent = ""
-	req, err := c.NewRequest("GET", "/", nil)
+	req, err := c.NewRequest("GET", ".", nil)
 	if err != nil {
 		t.Fatalf("NewRequest returned unexpected error: %v", err)
 	}
@@ -239,7 +261,7 @@ func TestNewRequest_emptyUserAgent(t *testing.T) {
 // subtle errors.
 func TestNewRequest_emptyBody(t *testing.T) {
 	c := NewClient(nil)
-	req, err := c.NewRequest("GET", "/", nil)
+	req, err := c.NewRequest("GET", ".", nil)
 	if err != nil {
 		t.Fatalf("NewRequest returned unexpected error: %v", err)
 	}
@@ -374,7 +396,7 @@ func TestDo(t *testing.T) {
 		fmt.Fprint(w, `{"A":"a"}`)
 	})
 
-	req, _ := client.NewRequest("GET", "/", nil)
+	req, _ := client.NewRequest("GET", ".", nil)
 	body := new(foo)
 	client.Do(context.Background(), req, body)
 
@@ -392,11 +414,14 @@ func TestDo_httpError(t *testing.T) {
 		http.Error(w, "Bad Request", 400)
 	})
 
-	req, _ := client.NewRequest("GET", "/", nil)
-	_, err := client.Do(context.Background(), req, nil)
+	req, _ := client.NewRequest("GET", ".", nil)
+	resp, err := client.Do(context.Background(), req, nil)
 
 	if err == nil {
-		t.Error("Expected HTTP 400 error.")
+		t.Fatal("Expected HTTP 400 error, got no error.")
+	}
+	if resp.StatusCode != 400 {
+		t.Errorf("Expected HTTP 400 error, got %d status code.", resp.StatusCode)
 	}
 }
 
@@ -408,10 +433,10 @@ func TestDo_redirectLoop(t *testing.T) {
 	defer teardown()
 
 	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
-		http.Redirect(w, r, "/", http.StatusFound)
+		http.Redirect(w, r, baseURLPath, http.StatusFound)
 	})
 
-	req, _ := client.NewRequest("GET", "/", nil)
+	req, _ := client.NewRequest("GET", ".", nil)
 	_, err := client.Do(context.Background(), req, nil)
 
 	if err == nil {
@@ -431,7 +456,7 @@ func TestDo_sanitizeURL(t *testing.T) {
 	}
 	unauthedClient := NewClient(tp.Client())
 	unauthedClient.BaseURL = &url.URL{Scheme: "http", Host: "127.0.0.1:0", Path: "/"} // Use port 0 on purpose to trigger a dial TCP error, expect to get "dial tcp 127.0.0.1:0: connect: can't assign requested address".
-	req, err := unauthedClient.NewRequest("GET", "/", nil)
+	req, err := unauthedClient.NewRequest("GET", ".", nil)
 	if err != nil {
 		t.Fatalf("NewRequest returned unexpected error: %v", err)
 	}
@@ -454,7 +479,7 @@ func TestDo_rateLimit(t *testing.T) {
 		w.Header().Set(headerRateReset, "1372700873")
 	})
 
-	req, _ := client.NewRequest("GET", "/", nil)
+	req, _ := client.NewRequest("GET", ".", nil)
 	resp, err := client.Do(context.Background(), req, nil)
 	if err != nil {
 		t.Errorf("Do returned unexpected error: %v", err)
@@ -483,7 +508,7 @@ func TestDo_rateLimit_errorResponse(t *testing.T) {
 		http.Error(w, "Bad Request", 400)
 	})
 
-	req, _ := client.NewRequest("GET", "/", nil)
+	req, _ := client.NewRequest("GET", ".", nil)
 	resp, err := client.Do(context.Background(), req, nil)
 	if err == nil {
 		t.Error("Expected error to be returned.")
@@ -520,7 +545,7 @@ func TestDo_rateLimit_rateLimitError(t *testing.T) {
 }`)
 	})
 
-	req, _ := client.NewRequest("GET", "/", nil)
+	req, _ := client.NewRequest("GET", ".", nil)
 	_, err := client.Do(context.Background(), req, nil)
 
 	if err == nil {
@@ -567,11 +592,11 @@ func TestDo_rateLimit_noNetworkCall(t *testing.T) {
 	})
 
 	// First request is made, and it makes the client aware of rate reset time being in the future.
-	req, _ := client.NewRequest("GET", "/first", nil)
+	req, _ := client.NewRequest("GET", "first", nil)
 	client.Do(context.Background(), req, nil)
 
 	// Second request should not cause a network call to be made, since client can predict a rate limit error.
-	req, _ = client.NewRequest("GET", "/second", nil)
+	req, _ = client.NewRequest("GET", "second", nil)
 	_, err := client.Do(context.Background(), req, nil)
 
 	if madeNetworkCall {
@@ -613,7 +638,7 @@ func TestDo_rateLimit_abuseRateLimitError(t *testing.T) {
 }`)
 	})
 
-	req, _ := client.NewRequest("GET", "/", nil)
+	req, _ := client.NewRequest("GET", ".", nil)
 	_, err := client.Do(context.Background(), req, nil)
 
 	if err == nil {
@@ -643,7 +668,7 @@ func TestDo_rateLimit_abuseRateLimitError_retryAfter(t *testing.T) {
 }`)
 	})
 
-	req, _ := client.NewRequest("GET", "/", nil)
+	req, _ := client.NewRequest("GET", ".", nil)
 	_, err := client.Do(context.Background(), req, nil)
 
 	if err == nil {
@@ -671,7 +696,7 @@ func TestDo_noContent(t *testing.T) {
 
 	var body json.RawMessage
 
-	req, _ := client.NewRequest("GET", "/", nil)
+	req, _ := client.NewRequest("GET", ".", nil)
 	_, err := client.Do(context.Background(), req, &body)
 	if err != nil {
 		t.Fatalf("Do returned unexpected error: %v", err)
@@ -864,7 +889,7 @@ func TestUnauthenticatedRateLimitedTransport(t *testing.T) {
 	}
 	unauthedClient := NewClient(tp.Client())
 	unauthedClient.BaseURL = client.BaseURL
-	req, _ := unauthedClient.NewRequest("GET", "/", nil)
+	req, _ := unauthedClient.NewRequest("GET", ".", nil)
 	unauthedClient.Do(context.Background(), req, nil)
 }
 
@@ -938,7 +963,7 @@ func TestBasicAuthTransport(t *testing.T) {
 	}
 	basicAuthClient := NewClient(tp.Client())
 	basicAuthClient.BaseURL = client.BaseURL
-	req, _ := basicAuthClient.NewRequest("GET", "/", nil)
+	req, _ := basicAuthClient.NewRequest("GET", ".", nil)
 	basicAuthClient.Do(context.Background(), req, nil)
 }
 
diff --git a/github/projects_test.go b/github/projects_test.go
index 1045ca9b..bf287e02 100644
--- a/github/projects_test.go
+++ b/github/projects_test.go
@@ -18,7 +18,7 @@ func TestProjectsService_UpdateProject(t *testing.T) {
 	setup()
 	defer teardown()
 
-	input := &ProjectOptions{Name: "Project Name", Body: "Project body."}
+	input := &ProjectOptions{Name: "Project Name", Body: "Project body.", State: "open"}
 
 	mux.HandleFunc("/projects/1", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "PATCH")
diff --git a/github/repos_contents_test.go b/github/repos_contents_test.go
index 498dfc81..770098e8 100644
--- a/github/repos_contents_test.go
+++ b/github/repos_contents_test.go
@@ -98,7 +98,7 @@ func TestRepositoriesService_DownloadContents_Success(t *testing.T) {
 		fmt.Fprint(w, `[{
 		  "type": "file",
 		  "name": "f",
-		  "download_url": "`+server.URL+`/download/f"
+		  "download_url": "`+server.URL+baseURLPath+`/download/f"
 		}]`)
 	})
 	mux.HandleFunc("/download/f", func(w http.ResponseWriter, r *http.Request) {
diff --git a/github/repos_test.go b/github/repos_test.go
index 1f7b424f..2ccea39f 100644
--- a/github/repos_test.go
+++ b/github/repos_test.go
@@ -145,14 +145,12 @@ func TestRepositoriesService_ListAll(t *testing.T) {
 	mux.HandleFunc("/repositories", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
 		testFormValues(t, r, values{
-			"since":    "1",
-			"page":     "2",
-			"per_page": "3",
+			"since": "1",
 		})
 		fmt.Fprint(w, `[{"id":1}]`)
 	})
 
-	opt := &RepositoryListAllOptions{1, ListOptions{2, 3}}
+	opt := &RepositoryListAllOptions{1}
 	repos, _, err := client.Repositories.ListAll(context.Background(), opt)
 	if err != nil {
 		t.Errorf("Repositories.ListAll returned error: %v", err)