Make the PushCreate test declarative (#11229)

Reduce the code duplication in the PushCreate test and switch
to a declarative format.

* Instead of explicitly creating the repository re-use functions from the other declarative tests and add comments
* Ensure that the test repository is deleted at the end of test
* Slightly reorder the sub-tests

Also reduce the code duplication in MergeFork and add some comments there too and make doGitCloneFail be self-contained.

Signed-off-by: Andrew Thornton art27@cantab.net
This commit is contained in:
zeripath 2020-04-28 09:32:23 +01:00 committed by GitHub
parent b0849abf3d
commit 1f0b797ddc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 72 additions and 102 deletions

View File

@ -115,10 +115,13 @@ func doGitClone(dstLocalPath string, u *url.URL) func(*testing.T) {
} }
} }
func doGitCloneFail(dstLocalPath string, u *url.URL) func(*testing.T) { func doGitCloneFail(u *url.URL) func(*testing.T) {
return func(t *testing.T) { return func(t *testing.T) {
assert.Error(t, git.Clone(u.String(), dstLocalPath, git.CloneRepoOptions{})) tmpDir, err := ioutil.TempDir("", "doGitCloneFail")
assert.False(t, com.IsExist(filepath.Join(dstLocalPath, "README.md"))) assert.NoError(t, err)
defer os.RemoveAll(tmpDir)
assert.Error(t, git.Clone(u.String(), tmpDir, git.CloneRepoOptions{}))
assert.False(t, com.IsExist(filepath.Join(tmpDir, "README.md")))
} }
} }

View File

@ -433,133 +433,104 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun
defer PrintCurrentTest(t)() defer PrintCurrentTest(t)()
var pr api.PullRequest var pr api.PullRequest
var err error var err error
// Create a test pullrequest
t.Run("CreatePullRequest", func(t *testing.T) { t.Run("CreatePullRequest", func(t *testing.T) {
pr, err = doAPICreatePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, baseBranch, headBranch)(t) pr, err = doAPICreatePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, baseBranch, headBranch)(t)
assert.NoError(t, err) assert.NoError(t, err)
}) })
t.Run("EnsureCanSeePull", func(t *testing.T) {
req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index)) // Ensure the PR page works
ctx.Session.MakeRequest(t, req, http.StatusOK) t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/files", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index))
ctx.Session.MakeRequest(t, req, http.StatusOK) // Then get the diff string
req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/commits", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index))
ctx.Session.MakeRequest(t, req, http.StatusOK)
})
var diffStr string var diffStr string
t.Run("GetDiff", func(t *testing.T) { t.Run("GetDiff", func(t *testing.T) {
req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d.diff", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index)) req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d.diff", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index))
resp := ctx.Session.MakeRequest(t, req, http.StatusOK) resp := ctx.Session.MakeRequest(t, req, http.StatusOK)
diffStr = resp.Body.String() diffStr = resp.Body.String()
}) })
// Now: Merge the PR & make sure that doesn't break the PR page or change its diff
t.Run("MergePR", doAPIMergePullRequest(baseCtx, baseCtx.Username, baseCtx.Reponame, pr.Index)) t.Run("MergePR", doAPIMergePullRequest(baseCtx, baseCtx.Username, baseCtx.Reponame, pr.Index))
t.Run("EnsureCanSeePull", func(t *testing.T) { t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index)) t.Run("EnsureDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffStr))
ctx.Session.MakeRequest(t, req, http.StatusOK)
req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/files", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index)) // Then: Delete the head branch & make sure that doesn't break the PR page or change its diff
ctx.Session.MakeRequest(t, req, http.StatusOK)
req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/commits", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index))
ctx.Session.MakeRequest(t, req, http.StatusOK)
})
t.Run("EnsureDiffNoChange", func(t *testing.T) {
req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d.diff", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index))
resp := ctx.Session.MakeRequest(t, req, http.StatusOK)
assert.Equal(t, diffStr, resp.Body.String())
})
t.Run("DeleteHeadBranch", doBranchDelete(baseCtx, baseCtx.Username, baseCtx.Reponame, headBranch)) t.Run("DeleteHeadBranch", doBranchDelete(baseCtx, baseCtx.Username, baseCtx.Reponame, headBranch))
t.Run("EnsureCanSeePull", func(t *testing.T) { t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index)) t.Run("EnsureDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffStr))
ctx.Session.MakeRequest(t, req, http.StatusOK)
req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/files", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index)) // Delete the head repository & make sure that doesn't break the PR page or change its diff
ctx.Session.MakeRequest(t, req, http.StatusOK) t.Run("DeleteHeadRepository", doAPIDeleteRepository(ctx))
req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/commits", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index)) t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
ctx.Session.MakeRequest(t, req, http.StatusOK) t.Run("EnsureDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffStr))
}) }
t.Run("EnsureDiffNoChange", func(t *testing.T) { }
req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d.diff", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index))
resp := ctx.Session.MakeRequest(t, req, http.StatusOK) func doEnsureCanSeePull(ctx APITestContext, pr api.PullRequest) func(t *testing.T) {
assert.Equal(t, diffStr, resp.Body.String()) return func(t *testing.T) {
}) req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr.Index))
t.Run("DeleteRepository", doAPIDeleteRepository(ctx)) ctx.Session.MakeRequest(t, req, http.StatusOK)
t.Run("EnsureCanSeePull", func(t *testing.T) { req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/files", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr.Index))
req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index)) ctx.Session.MakeRequest(t, req, http.StatusOK)
ctx.Session.MakeRequest(t, req, http.StatusOK) req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/commits", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr.Index))
req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/files", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index)) ctx.Session.MakeRequest(t, req, http.StatusOK)
ctx.Session.MakeRequest(t, req, http.StatusOK) }
req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/commits", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index)) }
ctx.Session.MakeRequest(t, req, http.StatusOK)
}) func doEnsureDiffNoChange(ctx APITestContext, pr api.PullRequest, diffStr string) func(t *testing.T) {
t.Run("EnsureDiffNoChange", func(t *testing.T) { return func(t *testing.T) {
req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d.diff", url.PathEscape(baseCtx.Username), url.PathEscape(baseCtx.Reponame), pr.Index)) req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d.diff", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr.Index))
resp := ctx.Session.MakeRequest(t, req, http.StatusOK) resp := ctx.Session.MakeRequest(t, req, http.StatusOK)
assert.Equal(t, diffStr, resp.Body.String()) assert.Equal(t, diffStr, resp.Body.String())
})
} }
} }
func doPushCreate(ctx APITestContext, u *url.URL) func(t *testing.T) { func doPushCreate(ctx APITestContext, u *url.URL) func(t *testing.T) {
return func(t *testing.T) { return func(t *testing.T) {
defer PrintCurrentTest(t)() defer PrintCurrentTest(t)()
// create a context for a currently non-existent repository
ctx.Reponame = fmt.Sprintf("repo-tmp-push-create-%s", u.Scheme) ctx.Reponame = fmt.Sprintf("repo-tmp-push-create-%s", u.Scheme)
u.Path = ctx.GitPath() u.Path = ctx.GitPath()
// Create a temporary directory
tmpDir, err := ioutil.TempDir("", ctx.Reponame) tmpDir, err := ioutil.TempDir("", ctx.Reponame)
assert.NoError(t, err) assert.NoError(t, err)
defer os.RemoveAll(tmpDir)
err = git.InitRepository(tmpDir, false) // Now create local repository to push as our test and set its origin
assert.NoError(t, err) t.Run("InitTestRepository", doGitInitTestRepository(tmpDir))
t.Run("AddRemote", doGitAddRemote(tmpDir, "origin", u))
_, err = os.Create(filepath.Join(tmpDir, "test.txt")) // Disable "Push To Create" and attempt to push
assert.NoError(t, err)
err = git.AddChanges(tmpDir, true)
assert.NoError(t, err)
err = git.CommitChanges(tmpDir, git.CommitChangesOptions{
Committer: &git.Signature{
Email: "user2@example.com",
Name: "User Two",
When: time.Now(),
},
Author: &git.Signature{
Email: "user2@example.com",
Name: "User Two",
When: time.Now(),
},
Message: fmt.Sprintf("Testing push create @ %v", time.Now()),
})
assert.NoError(t, err)
_, err = git.NewCommand("remote", "add", "origin", u.String()).RunInDir(tmpDir)
assert.NoError(t, err)
invalidCtx := ctx
invalidCtx.Reponame = fmt.Sprintf("invalid/repo-tmp-push-create-%s", u.Scheme)
u.Path = invalidCtx.GitPath()
_, err = git.NewCommand("remote", "add", "invalid", u.String()).RunInDir(tmpDir)
assert.NoError(t, err)
// Push to create disabled
setting.Repository.EnablePushCreateUser = false setting.Repository.EnablePushCreateUser = false
_, err = git.NewCommand("push", "origin", "master").RunInDir(tmpDir) t.Run("FailToPushAndCreateTestRepository", doGitPushTestRepositoryFail(tmpDir, "origin", "master"))
assert.Error(t, err)
// Push to create enabled // Enable "Push To Create"
setting.Repository.EnablePushCreateUser = true setting.Repository.EnablePushCreateUser = true
// Invalid repo // Assert that cloning from a non-existent repository does not create it and that it definitely wasn't create above
_, err = git.NewCommand("push", "invalid", "master").RunInDir(tmpDir) t.Run("FailToCloneFromNonExistentRepository", doGitCloneFail(u))
assert.Error(t, err)
// Valid repo // Then "Push To Create"x
_, err = git.NewCommand("push", "origin", "master").RunInDir(tmpDir) t.Run("SuccessfullyPushAndCreateTestRepository", doGitPushTestRepository(tmpDir, "origin", "master"))
assert.NoError(t, err)
// Fetch repo from database // Finally, fetch repo from database and ensure the correct repository has been created
repo, err := models.GetRepositoryByOwnerAndName(ctx.Username, ctx.Reponame) repo, err := models.GetRepositoryByOwnerAndName(ctx.Username, ctx.Reponame)
assert.NoError(t, err) assert.NoError(t, err)
assert.False(t, repo.IsEmpty) assert.False(t, repo.IsEmpty)
assert.True(t, repo.IsPrivate) assert.True(t, repo.IsPrivate)
// Now add a remote that is invalid to "Push To Create"
invalidCtx := ctx
invalidCtx.Reponame = fmt.Sprintf("invalid/repo-tmp-push-create-%s", u.Scheme)
u.Path = invalidCtx.GitPath()
t.Run("AddInvalidRemote", doGitAddRemote(tmpDir, "invalid", u))
// Fail to "Push To Create" the invalid
t.Run("FailToPushAndCreateInvalidTestRepository", doGitPushTestRepositoryFail(tmpDir, "invalid", "master"))
} }
} }

View File

@ -113,7 +113,7 @@ func testKeyOnlyOneType(t *testing.T, u *url.URL) {
sshURL := createSSHUrl(ctx.GitPath(), u) sshURL := createSSHUrl(ctx.GitPath(), u)
t.Run("FailToClone", doGitCloneFail(dstPath, sshURL)) t.Run("FailToClone", doGitCloneFail(sshURL))
t.Run("CreateUserKey", doAPICreateUserKey(ctx, keyname, keyFile, func(t *testing.T, publicKey api.PublicKey) { t.Run("CreateUserKey", doAPICreateUserKey(ctx, keyname, keyFile, func(t *testing.T, publicKey api.PublicKey) {
userKeyPublicKeyID = publicKey.ID userKeyPublicKeyID = publicKey.ID
@ -139,7 +139,7 @@ func testKeyOnlyOneType(t *testing.T, u *url.URL) {
sshURL := createSSHUrl(ctx.GitPath(), u) sshURL := createSSHUrl(ctx.GitPath(), u)
t.Run("FailToClone", doGitCloneFail(dstPath, sshURL)) t.Run("FailToClone", doGitCloneFail(sshURL))
// Should now be able to add... // Should now be able to add...
t.Run("AddReadOnlyDeployKey", doAPICreateDeployKey(ctx, keyname, keyFile, true)) t.Run("AddReadOnlyDeployKey", doAPICreateDeployKey(ctx, keyname, keyFile, true))
@ -204,15 +204,11 @@ func testKeyOnlyOneType(t *testing.T, u *url.URL) {
}) })
t.Run("DeleteUserKeyShouldRemoveAbilityToClone", func(t *testing.T) { t.Run("DeleteUserKeyShouldRemoveAbilityToClone", func(t *testing.T) {
dstPath, err := ioutil.TempDir("", ctx.Reponame)
assert.NoError(t, err)
defer os.RemoveAll(dstPath)
sshURL := createSSHUrl(ctx.GitPath(), u) sshURL := createSSHUrl(ctx.GitPath(), u)
t.Run("DeleteUserKey", doAPIDeleteUserKey(ctx, userKeyPublicKeyID)) t.Run("DeleteUserKey", doAPIDeleteUserKey(ctx, userKeyPublicKeyID))
t.Run("FailToClone", doGitCloneFail(dstPath, sshURL)) t.Run("FailToClone", doGitCloneFail(sshURL))
}) })
}) })
} }