From e59adcde655aac0e8afd3249407c9a0a2b1b1d6b Mon Sep 17 00:00:00 2001 From: Ethan Koenig Date: Sun, 3 Dec 2017 14:46:01 -0800 Subject: [PATCH] Use httptest in integration tests (#3080) --- integrations/api_branch_test.go | 4 +- integrations/api_gpg_keys_test.go | 69 ++++++++++++++++--------------- integrations/branches_test.go | 8 ++-- integrations/editor_test.go | 11 ++--- integrations/html_helper.go | 4 +- integrations/integration_test.go | 57 ++++++------------------- integrations/internal_test.go | 8 ++-- integrations/pull_compare_test.go | 2 +- integrations/pull_create_test.go | 3 +- integrations/pull_merge_test.go | 5 ++- integrations/repo_fork_test.go | 3 +- integrations/repo_migrate_test.go | 3 +- 12 files changed, 77 insertions(+), 100 deletions(-) diff --git a/integrations/api_branch_test.go b/integrations/api_branch_test.go index 8701229b09..5a28c1f494 100644 --- a/integrations/api_branch_test.go +++ b/integrations/api_branch_test.go @@ -20,10 +20,10 @@ func testAPIGetBranch(t *testing.T, branchName string, exists bool) { req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo1/branches/%s", branchName) resp := session.MakeRequest(t, req, NoExpectedStatus) if !exists { - assert.EqualValues(t, http.StatusNotFound, resp.HeaderCode) + assert.EqualValues(t, http.StatusNotFound, resp.Code) return } - assert.EqualValues(t, http.StatusOK, resp.HeaderCode) + assert.EqualValues(t, http.StatusOK, resp.Code) var branch api.Branch DecodeJSON(t, resp, &branch) assert.EqualValues(t, branchName, branch.Name) diff --git a/integrations/api_gpg_keys_test.go b/integrations/api_gpg_keys_test.go index 51dc7843c0..4d3745d942 100644 --- a/integrations/api_gpg_keys_test.go +++ b/integrations/api_gpg_keys_test.go @@ -6,27 +6,30 @@ package integrations import ( "net/http" + "net/http/httptest" "strconv" "testing" - "github.com/stretchr/testify/assert" - api "code.gitea.io/sdk/gitea" + + "github.com/stretchr/testify/assert" ) +type makeRequestFunc func(testing.TB, *http.Request, int) *httptest.ResponseRecorder + func TestGPGKeys(t *testing.T) { prepareTestEnv(t) session := loginUser(t, "user2") tt := []struct { - name string - reqBuilder func(testing.TB, *http.Request, int) *TestResponse - results []int + name string + makeRequest makeRequestFunc + results []int }{ - {name: "NoLogin", reqBuilder: MakeRequest, + {name: "NoLogin", makeRequest: MakeRequest, results: []int{http.StatusUnauthorized, http.StatusUnauthorized, http.StatusUnauthorized, http.StatusUnauthorized, http.StatusUnauthorized, http.StatusUnauthorized, http.StatusUnauthorized, http.StatusUnauthorized}, }, - {name: "LoggedAsUser2", reqBuilder: session.MakeRequest, + {name: "LoggedAsUser2", makeRequest: session.MakeRequest, results: []int{http.StatusOK, http.StatusOK, http.StatusNotFound, http.StatusNoContent, http.StatusInternalServerError, http.StatusInternalServerError, http.StatusCreated, http.StatusCreated}}, } @@ -35,29 +38,29 @@ func TestGPGKeys(t *testing.T) { //Basic test on result code t.Run(tc.name, func(t *testing.T) { t.Run("ViewOwnGPGKeys", func(t *testing.T) { - testViewOwnGPGKeys(t, tc.reqBuilder, tc.results[0]) + testViewOwnGPGKeys(t, tc.makeRequest, tc.results[0]) }) t.Run("ViewGPGKeys", func(t *testing.T) { - testViewGPGKeys(t, tc.reqBuilder, tc.results[1]) + testViewGPGKeys(t, tc.makeRequest, tc.results[1]) }) t.Run("GetGPGKey", func(t *testing.T) { - testGetGPGKey(t, tc.reqBuilder, tc.results[2]) + testGetGPGKey(t, tc.makeRequest, tc.results[2]) }) t.Run("DeleteGPGKey", func(t *testing.T) { - testDeleteGPGKey(t, tc.reqBuilder, tc.results[3]) + testDeleteGPGKey(t, tc.makeRequest, tc.results[3]) }) t.Run("CreateInvalidGPGKey", func(t *testing.T) { - testCreateInvalidGPGKey(t, tc.reqBuilder, tc.results[4]) + testCreateInvalidGPGKey(t, tc.makeRequest, tc.results[4]) }) t.Run("CreateNoneRegistredEmailGPGKey", func(t *testing.T) { - testCreateNoneRegistredEmailGPGKey(t, tc.reqBuilder, tc.results[5]) + testCreateNoneRegistredEmailGPGKey(t, tc.makeRequest, tc.results[5]) }) t.Run("CreateValidGPGKey", func(t *testing.T) { - testCreateValidGPGKey(t, tc.reqBuilder, tc.results[6]) + testCreateValidGPGKey(t, tc.makeRequest, tc.results[6]) }) t.Run("CreateValidSecondaryEmailGPGKey", func(t *testing.T) { - testCreateValidSecondaryEmailGPGKey(t, tc.reqBuilder, tc.results[7]) + testCreateValidSecondaryEmailGPGKey(t, tc.makeRequest, tc.results[7]) }) }) } @@ -140,39 +143,39 @@ func TestGPGKeys(t *testing.T) { }) } -func testViewOwnGPGKeys(t *testing.T, reqBuilder func(testing.TB, *http.Request, int) *TestResponse, expected int) { +func testViewOwnGPGKeys(t *testing.T, makeRequest makeRequestFunc, expected int) { req := NewRequest(t, "GET", "/api/v1/user/gpg_keys") - reqBuilder(t, req, expected) + makeRequest(t, req, expected) } -func testViewGPGKeys(t *testing.T, reqBuilder func(testing.TB, *http.Request, int) *TestResponse, expected int) { +func testViewGPGKeys(t *testing.T, makeRequest makeRequestFunc, expected int) { req := NewRequest(t, "GET", "/api/v1/users/user2/gpg_keys") - reqBuilder(t, req, expected) + makeRequest(t, req, expected) } -func testGetGPGKey(t *testing.T, reqBuilder func(testing.TB, *http.Request, int) *TestResponse, expected int) { +func testGetGPGKey(t *testing.T, makeRequest makeRequestFunc, expected int) { req := NewRequest(t, "GET", "/api/v1/user/gpg_keys/1") - reqBuilder(t, req, expected) + makeRequest(t, req, expected) } -func testDeleteGPGKey(t *testing.T, reqBuilder func(testing.TB, *http.Request, int) *TestResponse, expected int) { +func testDeleteGPGKey(t *testing.T, makeRequest makeRequestFunc, expected int) { req := NewRequest(t, "DELETE", "/api/v1/user/gpg_keys/1") - reqBuilder(t, req, expected) + makeRequest(t, req, expected) } -func testCreateGPGKey(t *testing.T, reqBuilder func(testing.TB, *http.Request, int) *TestResponse, expected int, publicKey string) { +func testCreateGPGKey(t *testing.T, makeRequest makeRequestFunc, expected int, publicKey string) { req := NewRequestWithJSON(t, "POST", "/api/v1/user/gpg_keys", api.CreateGPGKeyOption{ ArmoredKey: publicKey, }) - reqBuilder(t, req, expected) + makeRequest(t, req, expected) } -func testCreateInvalidGPGKey(t *testing.T, reqBuilder func(testing.TB, *http.Request, int) *TestResponse, expected int) { - testCreateGPGKey(t, reqBuilder, expected, "invalid_key") +func testCreateInvalidGPGKey(t *testing.T, makeRequest makeRequestFunc, expected int) { + testCreateGPGKey(t, makeRequest, expected, "invalid_key") } -func testCreateNoneRegistredEmailGPGKey(t *testing.T, reqBuilder func(testing.TB, *http.Request, int) *TestResponse, expected int) { - testCreateGPGKey(t, reqBuilder, expected, `-----BEGIN PGP PUBLIC KEY BLOCK----- +func testCreateNoneRegistredEmailGPGKey(t *testing.T, makeRequest makeRequestFunc, expected int) { + testCreateGPGKey(t, makeRequest, expected, `-----BEGIN PGP PUBLIC KEY BLOCK----- mQENBFmGUygBCACjCNbKvMGgp0fd5vyFW9olE1CLCSyyF9gQN2hSuzmZLuAZF2Kh dCMCG2T1UwzUB/yWUFWJ2BtCwSjuaRv+cGohqEy6bhEBV90peGA33lHfjx7wP25O @@ -191,9 +194,9 @@ INx/MmBfmtCq05FqNclvU+sj2R3N1JJOtBOjZrJHQbJhzoILou8AkxeX1A+q9OAz -----END PGP PUBLIC KEY BLOCK-----`) } -func testCreateValidGPGKey(t *testing.T, reqBuilder func(testing.TB, *http.Request, int) *TestResponse, expected int) { +func testCreateValidGPGKey(t *testing.T, makeRequest makeRequestFunc, expected int) { //User2 //primary & activated - testCreateGPGKey(t, reqBuilder, expected, `-----BEGIN PGP PUBLIC KEY BLOCK----- + testCreateGPGKey(t, makeRequest, expected, `-----BEGIN PGP PUBLIC KEY BLOCK----- mQENBFmGVsMBCACuxgZ7W7rI9xN08Y4M7B8yx/6/I4Slm94+wXf8YNRvAyqj30dW VJhyBcnfNRDLKSQp5o/hhfDkCgdqBjLa1PnHlGS3PXJc0hP/FyYPD2BFvNMPpCYS @@ -225,9 +228,9 @@ uy6MA3VSB99SK9ducGmE1Jv8mcziREroz2TEGr0zPs6h -----END PGP PUBLIC KEY BLOCK-----`) } -func testCreateValidSecondaryEmailGPGKey(t *testing.T, reqBuilder func(testing.TB, *http.Request, int) *TestResponse, expected int) { +func testCreateValidSecondaryEmailGPGKey(t *testing.T, makeRequest makeRequestFunc, expected int) { //User2 //secondary and not activated - testCreateGPGKey(t, reqBuilder, expected, `-----BEGIN PGP PUBLIC KEY BLOCK----- + testCreateGPGKey(t, makeRequest, expected, `-----BEGIN PGP PUBLIC KEY BLOCK----- mQENBFmGWN4BCAC18V4tVGO65VLCV7p14FuXJlUtZ5CuYMvgEkcOqrvRaBSW9ao4 PGESOhJpfWpnW3QgJniYndLzPpsmdHEclEER6aZjiNgReWPOjHD5tykWocZAJqXD diff --git a/integrations/branches_test.go b/integrations/branches_test.go index 7fb57242d9..01c6dd2a4b 100644 --- a/integrations/branches_test.go +++ b/integrations/branches_test.go @@ -59,9 +59,8 @@ func branchAction(t *testing.T, button string) (*HTMLDoc, string) { link, exists := htmlDoc.doc.Find(button).Attr("data-url") assert.True(t, exists, "The template has changed") - htmlDoc = NewHTMLParser(t, resp.Body) req = NewRequestWithValues(t, "POST", link, map[string]string{ - "_csrf": getCsrf(htmlDoc.doc), + "_csrf": getCsrf(t, htmlDoc.doc), }) resp = session.MakeRequest(t, req, http.StatusOK) @@ -73,7 +72,8 @@ func branchAction(t *testing.T, button string) (*HTMLDoc, string) { return NewHTMLParser(t, resp.Body), url.Query()["name"][0] } -func getCsrf(doc *goquery.Document) string { - csrf, _ := doc.Find("meta[name=\"_csrf\"]").Attr("content") +func getCsrf(t *testing.T, doc *goquery.Document) string { + csrf, exists := doc.Find("meta[name=\"_csrf\"]").Attr("content") + assert.True(t, exists) return csrf } diff --git a/integrations/editor_test.go b/integrations/editor_test.go index 712cc91fbc..0722a917f0 100644 --- a/integrations/editor_test.go +++ b/integrations/editor_test.go @@ -6,6 +6,7 @@ package integrations import ( "net/http" + "net/http/httptest" "path" "testing" @@ -72,7 +73,7 @@ func TestCreateFileOnProtectedBranch(t *testing.T) { resp = session.MakeRequest(t, req, http.StatusOK) // Check body for error message - assert.Contains(t, string(resp.Body), "Can not commit to protected branch 'master'.") + assert.Contains(t, resp.Body.String(), "Can not commit to protected branch 'master'.") // remove the protected branch csrf = GetCSRF(t, session, "/user2/repo1/settings/branches") @@ -89,7 +90,7 @@ func TestCreateFileOnProtectedBranch(t *testing.T) { } -func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePath, newContent string) *TestResponse { +func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePath, newContent string) *httptest.ResponseRecorder { // Get to the 'edit this file' page req := NewRequest(t, "GET", path.Join(user, repo, "_edit", branch, filePath)) resp := session.MakeRequest(t, req, http.StatusOK) @@ -113,12 +114,12 @@ func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePa // Verify the change req = NewRequest(t, "GET", path.Join(user, repo, "raw/branch", branch, filePath)) resp = session.MakeRequest(t, req, http.StatusOK) - assert.EqualValues(t, newContent, string(resp.Body)) + assert.EqualValues(t, newContent, resp.Body.String()) return resp } -func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, branch, targetBranch, filePath, newContent string) *TestResponse { +func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, branch, targetBranch, filePath, newContent string) *httptest.ResponseRecorder { // Get to the 'edit this file' page req := NewRequest(t, "GET", path.Join(user, repo, "_edit", branch, filePath)) @@ -144,7 +145,7 @@ func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, bra // Verify the change req = NewRequest(t, "GET", path.Join(user, repo, "raw/branch", targetBranch, filePath)) resp = session.MakeRequest(t, req, http.StatusOK) - assert.EqualValues(t, newContent, string(resp.Body)) + assert.EqualValues(t, newContent, resp.Body.String()) return resp } diff --git a/integrations/html_helper.go b/integrations/html_helper.go index c181a25b57..139261d694 100644 --- a/integrations/html_helper.go +++ b/integrations/html_helper.go @@ -18,8 +18,8 @@ type HTMLDoc struct { } // NewHTMLParser parse html file -func NewHTMLParser(t testing.TB, content []byte) *HTMLDoc { - doc, err := goquery.NewDocumentFromReader(bytes.NewReader(content)) +func NewHTMLParser(t testing.TB, body *bytes.Buffer) *HTMLDoc { + doc, err := goquery.NewDocumentFromReader(body) assert.NoError(t, err) return &HTMLDoc{doc: doc} } diff --git a/integrations/integration_test.go b/integrations/integration_test.go index eae0638587..ee0b1a8868 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -13,6 +13,7 @@ import ( "log" "net/http" "net/http/cookiejar" + "net/http/httptest" "net/url" "os" "path" @@ -158,7 +159,7 @@ func (s *TestSession) GetCookie(name string) *http.Cookie { return nil } -func (s *TestSession) MakeRequest(t testing.TB, req *http.Request, expectedStatus int) *TestResponse { +func (s *TestSession) MakeRequest(t testing.TB, req *http.Request, expectedStatus int) *httptest.ResponseRecorder { baseURL, err := url.Parse(setting.AppURL) assert.NoError(t, err) for _, c := range s.jar.Cookies(baseURL) { @@ -167,7 +168,7 @@ func (s *TestSession) MakeRequest(t testing.TB, req *http.Request, expectedStatu resp := MakeRequest(t, req, expectedStatus) ch := http.Header{} - ch.Add("Cookie", strings.Join(resp.Headers["Set-Cookie"], ";")) + ch.Add("Cookie", strings.Join(resp.HeaderMap["Set-Cookie"], ";")) cr := http.Request{Header: ch} s.jar.SetCookies(baseURL, cr.Cookies()) @@ -207,7 +208,7 @@ func loginUserWithPassword(t testing.TB, userName, password string) *TestSession resp = MakeRequest(t, req, http.StatusFound) ch := http.Header{} - ch.Add("Cookie", strings.Join(resp.Headers["Set-Cookie"], ";")) + ch.Add("Cookie", strings.Join(resp.HeaderMap["Set-Cookie"], ";")) cr := http.Request{Header: ch} session := emptyTestSession(t) @@ -219,30 +220,6 @@ func loginUserWithPassword(t testing.TB, userName, password string) *TestSession return session } -type TestResponseWriter struct { - HeaderCode int - Writer io.Writer - Headers http.Header -} - -func (w *TestResponseWriter) Header() http.Header { - return w.Headers -} - -func (w *TestResponseWriter) Write(b []byte) (int, error) { - return w.Writer.Write(b) -} - -func (w *TestResponseWriter) WriteHeader(n int) { - w.HeaderCode = n -} - -type TestResponse struct { - HeaderCode int - Body []byte - Headers http.Header -} - func NewRequest(t testing.TB, method, urlStr string) *http.Request { return NewRequestWithBody(t, method, urlStr, nil) } @@ -278,26 +255,18 @@ func NewRequestWithBody(t testing.TB, method, urlStr string, body io.Reader) *ht const NoExpectedStatus = -1 -func MakeRequest(t testing.TB, req *http.Request, expectedStatus int) *TestResponse { - buffer := bytes.NewBuffer(nil) - respWriter := &TestResponseWriter{ - Writer: buffer, - Headers: make(map[string][]string), - } - mac.ServeHTTP(respWriter, req) +func MakeRequest(t testing.TB, req *http.Request, expectedStatus int) *httptest.ResponseRecorder { + recorder := httptest.NewRecorder() + mac.ServeHTTP(recorder, req) if expectedStatus != NoExpectedStatus { - assert.EqualValues(t, expectedStatus, respWriter.HeaderCode, + assert.EqualValues(t, expectedStatus, recorder.Code, "Request: %s %s", req.Method, req.URL.String()) } - return &TestResponse{ - HeaderCode: respWriter.HeaderCode, - Body: buffer.Bytes(), - Headers: respWriter.Headers, - } + return recorder } -func DecodeJSON(t testing.TB, resp *TestResponse, v interface{}) { - decoder := json.NewDecoder(bytes.NewBuffer(resp.Body)) +func DecodeJSON(t testing.TB, resp *httptest.ResponseRecorder, v interface{}) { + decoder := json.NewDecoder(resp.Body) assert.NoError(t, decoder.Decode(v)) } @@ -308,8 +277,8 @@ func GetCSRF(t testing.TB, session *TestSession, urlStr string) string { return doc.GetCSRF() } -func RedirectURL(t testing.TB, resp *TestResponse) string { - urlSlice := resp.Headers["Location"] +func RedirectURL(t testing.TB, resp *httptest.ResponseRecorder) string { + urlSlice := resp.HeaderMap["Location"] assert.NotEmpty(t, urlSlice, "No redirect URL founds") return urlSlice[0] } diff --git a/integrations/internal_test.go b/integrations/internal_test.go index d58b8b0b4e..3c8657e642 100644 --- a/integrations/internal_test.go +++ b/integrations/internal_test.go @@ -25,12 +25,12 @@ func assertProtectedBranch(t *testing.T, repoID int64, branchName string, isErr, resp := MakeRequest(t, req, NoExpectedStatus) if isErr { - assert.EqualValues(t, http.StatusInternalServerError, resp.HeaderCode) + assert.EqualValues(t, http.StatusInternalServerError, resp.Code) } else { - assert.EqualValues(t, http.StatusOK, resp.HeaderCode) + assert.EqualValues(t, http.StatusOK, resp.Code) var branch models.ProtectedBranch - t.Log(string(resp.Body)) - assert.NoError(t, json.Unmarshal(resp.Body, &branch)) + t.Log(resp.Body.String()) + assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), &branch)) assert.Equal(t, canPush, !branch.IsProtected()) } } diff --git a/integrations/pull_compare_test.go b/integrations/pull_compare_test.go index 593b2f4d01..dddaa391ed 100644 --- a/integrations/pull_compare_test.go +++ b/integrations/pull_compare_test.go @@ -23,5 +23,5 @@ func TestPullCompare(t *testing.T) { req = NewRequest(t, "GET", link) resp = session.MakeRequest(t, req, http.StatusOK) - assert.EqualValues(t, http.StatusOK, resp.HeaderCode) + assert.EqualValues(t, http.StatusOK, resp.Code) } diff --git a/integrations/pull_create_test.go b/integrations/pull_create_test.go index 8f1658b118..c392ecc18d 100644 --- a/integrations/pull_create_test.go +++ b/integrations/pull_create_test.go @@ -6,6 +6,7 @@ package integrations import ( "net/http" + "net/http/httptest" "path" "strings" "testing" @@ -13,7 +14,7 @@ import ( "github.com/stretchr/testify/assert" ) -func testPullCreate(t *testing.T, session *TestSession, user, repo, branch string) *TestResponse { +func testPullCreate(t *testing.T, session *TestSession, user, repo, branch string) *httptest.ResponseRecorder { req := NewRequest(t, "GET", path.Join(user, repo)) resp := session.MakeRequest(t, req, http.StatusOK) diff --git a/integrations/pull_merge_test.go b/integrations/pull_merge_test.go index f3be6f2885..d6413be549 100644 --- a/integrations/pull_merge_test.go +++ b/integrations/pull_merge_test.go @@ -6,6 +6,7 @@ package integrations import ( "net/http" + "net/http/httptest" "path" "strings" "testing" @@ -13,7 +14,7 @@ import ( "github.com/stretchr/testify/assert" ) -func testPullMerge(t *testing.T, session *TestSession, user, repo, pullnum string) *TestResponse { +func testPullMerge(t *testing.T, session *TestSession, user, repo, pullnum string) *httptest.ResponseRecorder { req := NewRequest(t, "GET", path.Join(user, repo, "pulls", pullnum)) resp := session.MakeRequest(t, req, http.StatusOK) @@ -29,7 +30,7 @@ func testPullMerge(t *testing.T, session *TestSession, user, repo, pullnum strin return resp } -func testPullCleanUp(t *testing.T, session *TestSession, user, repo, pullnum string) *TestResponse { +func testPullCleanUp(t *testing.T, session *TestSession, user, repo, pullnum string) *httptest.ResponseRecorder { req := NewRequest(t, "GET", path.Join(user, repo, "pulls", pullnum)) resp := session.MakeRequest(t, req, http.StatusOK) diff --git a/integrations/repo_fork_test.go b/integrations/repo_fork_test.go index f8cc2e3f3b..64bc46f777 100644 --- a/integrations/repo_fork_test.go +++ b/integrations/repo_fork_test.go @@ -7,6 +7,7 @@ package integrations import ( "fmt" "net/http" + "net/http/httptest" "testing" "code.gitea.io/gitea/models" @@ -14,7 +15,7 @@ import ( "github.com/stretchr/testify/assert" ) -func testRepoFork(t *testing.T, session *TestSession, ownerName, repoName, forkOwnerName, forkRepoName string) *TestResponse { +func testRepoFork(t *testing.T, session *TestSession, ownerName, repoName, forkOwnerName, forkRepoName string) *httptest.ResponseRecorder { forkOwner := models.AssertExistsAndLoadBean(t, &models.User{Name: forkOwnerName}).(*models.User) // Step0: check the existence of the to-fork repo diff --git a/integrations/repo_migrate_test.go b/integrations/repo_migrate_test.go index 4307ddf901..9f41cca827 100644 --- a/integrations/repo_migrate_test.go +++ b/integrations/repo_migrate_test.go @@ -6,12 +6,13 @@ package integrations import ( "net/http" + "net/http/httptest" "testing" "github.com/stretchr/testify/assert" ) -func testRepoMigrate(t testing.TB, session *TestSession, cloneAddr, repoName string) *TestResponse { +func testRepoMigrate(t testing.TB, session *TestSession, cloneAddr, repoName string) *httptest.ResponseRecorder { req := NewRequest(t, "GET", "/repo/migrate") resp := session.MakeRequest(t, req, http.StatusOK) htmlDoc := NewHTMLParser(t, resp.Body)