Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix hidden commit status on multiple checks #22889

Merged
merged 14 commits into from
Feb 20, 2023
37 changes: 25 additions & 12 deletions templates/repo/commit_statuses.tmpl
Original file line number Diff line number Diff line change
@@ -1,14 +1,27 @@
{{if eq (len .Statuses) 1}}{{$status := index .Statuses 0}}{{if $status.TargetURL}}<a class="ui link commit-statuses-trigger vm" href="{{$status.TargetURL}}">{{template "repo/commit_status" .Status}}</a>{{end}}{{end}}
<div class="ui commit-statuses-popup commit-statuses tippy-target">
<div class="ui relaxed list divided">
{{range .Statuses}}
<div class="ui item singular-status df">
{{template "repo/commit_status" .}}
<span class="ui ml-3 f1">{{.Context}} <span class="text grey">{{.Description}}</span></span>
{{if .TargetURL}}
<a class="ml-3" href="{{.TargetURL}}" target="_blank" rel="noopener noreferrer">{{$.root.locale.Tr "repo.pulls.status_checks_details"}}</a>
{{end}}
</div>
{{if .Statuses}}
<span class="commit-statuses-trigger vm">
oliverpool marked this conversation as resolved.
Show resolved Hide resolved
{{if eq (len .Statuses) 1}}
{{$status := index .Statuses 0}}
{{if $status.TargetURL}}
<a class="ui link" href="{{$status.TargetURL}}">{{template "repo/commit_status" .Status}}</a>
{{else}}
{{template "repo/commit_status" .Status}}
{{end}}
{{else}}
{{template "repo/commit_status" .Status}}
{{end}}
</span>
<div class="ui commit-statuses-popup commit-statuses tippy-target">
<div class="ui relaxed list divided">
{{range .Statuses}}
<div class="ui item singular-status df">
{{template "repo/commit_status" .}}
<span class="ui ml-3 f1">{{.Context}} <span class="text grey">{{.Description}}</span></span>
{{if .TargetURL}}
<a class="ml-3" href="{{.TargetURL}}" target="_blank" rel="noopener noreferrer">{{$.root.locale.Tr "repo.pulls.status_checks_details"}}</a>
{{end}}
</div>
{{end}}
</div>
</div>
</div>
{{end}}
15 changes: 12 additions & 3 deletions tests/integration/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -630,8 +630,17 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) {

commitID := path.Base(commitURL)

addCommitStatus := func(status api.CommitStatusState) func(*testing.T) {
return doAPICreateCommitStatus(ctx, commitID, api.CreateStatusOption{
State: status,
TargetURL: "http://test.ci/",
Description: "",
Context: "testci",
})
}

// Call API to add Pending status for commit
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, commitID, api.CommitStatusPending))
t.Run("CreateStatus", addCommitStatus(api.CommitStatusPending))

// Cancel not existing auto merge
ctx.ExpectedCode = http.StatusNotFound
Expand Down Expand Up @@ -660,15 +669,15 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) {
assert.False(t, pr.HasMerged)

// Call API to add Failure status for commit
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, commitID, api.CommitStatusFailure))
t.Run("CreateStatus", addCommitStatus(api.CommitStatusFailure))

// Check pr status
pr, err = doAPIGetPullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t)
assert.NoError(t, err)
assert.False(t, pr.HasMerged)

// Call API to add Success status for commit
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, commitID, api.CommitStatusSuccess))
t.Run("CreateStatus", addCommitStatus(api.CommitStatusSuccess))

// wait to let gitea merge stuff
time.Sleep(time.Second)
Expand Down
21 changes: 12 additions & 9 deletions tests/integration/pull_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,12 @@ func TestPullCreate_CommitStatus(t *testing.T) {
for _, status := range statusList {

// Call API to add status for commit
t.Run("CreateStatus", doAPICreateCommitStatus(testCtx, commitID, status))
t.Run("CreateStatus", doAPICreateCommitStatus(testCtx, commitID, api.CreateStatusOption{
State: status,
TargetURL: "http://test.ci/",
Description: "",
Context: "testci",
}))

req = NewRequestf(t, "GET", "/user1/repo1/pulls/1/commits")
resp = session.MakeRequest(t, req, http.StatusOK)
Expand All @@ -88,15 +93,13 @@ func TestPullCreate_CommitStatus(t *testing.T) {
})
}

func doAPICreateCommitStatus(ctx APITestContext, commitID string, status api.CommitStatusState) func(*testing.T) {
func doAPICreateCommitStatus(ctx APITestContext, commitID string, data api.CreateStatusOption) func(*testing.T) {
return func(t *testing.T) {
req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/statuses/%s?token=%s", ctx.Username, ctx.Reponame, commitID, ctx.Token),
api.CreateStatusOption{
State: status,
TargetURL: "http://test.ci/",
Description: "",
Context: "testci",
},
req := NewRequestWithJSON(
t,
http.MethodPost,
fmt.Sprintf("/api/v1/repos/%s/%s/statuses/%s?token=%s", ctx.Username, ctx.Reponame, commitID, ctx.Token),
data,
)
if ctx.ExpectedCode != 0 {
ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
Expand Down
56 changes: 53 additions & 3 deletions tests/integration/repo_commits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,19 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) {

// Call API to add status for commit
ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeRepo)
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CommitStatusState(state)))
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{
State: api.CommitStatusState(state),
TargetURL: "http://test.ci/",
Description: "",
Context: "testci",
}))

req = NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
resp = session.MakeRequest(t, req, http.StatusOK)

doc = NewHTMLParser(t, resp.Body)
// Check if commit status is displayed in message column
sel := doc.doc.Find("#commits-table tbody tr td.message a.commit-statuses-trigger .commit-status")
sel := doc.doc.Find("#commits-table tbody tr td.message .commit-statuses-trigger .commit-status")
assert.Equal(t, 1, sel.Length())
for _, class := range classes {
assert.True(t, sel.HasClass(class))
Expand Down Expand Up @@ -145,11 +150,56 @@ func TestRepoCommitsStatusParallel(t *testing.T) {
go func(parentT *testing.T, i int) {
parentT.Run(fmt.Sprintf("ParallelCreateStatus_%d", i), func(t *testing.T) {
ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeRepoStatus)
runBody := doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CommitStatusState("pending"))
runBody := doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{
State: api.CommitStatusPending,
TargetURL: "http://test.ci/",
Description: "",
Context: "testci",
})
runBody(t)
wg.Done()
})
}(t, i)
}
wg.Wait()
}

func TestRepoCommitsStatusMultiple(t *testing.T) {
defer tests.PrepareTestEnv(t)()

session := loginUser(t, "user2")

// Request repository commits page
req := NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
resp := session.MakeRequest(t, req, http.StatusOK)

doc := NewHTMLParser(t, resp.Body)
// Get first commit URL
commitURL, exists := doc.doc.Find("#commits-table tbody tr td.sha a").Attr("href")
assert.True(t, exists)
assert.NotEmpty(t, commitURL)

// Call API to add status for commit
ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeRepo)
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{
State: api.CommitStatusSuccess,
TargetURL: "http://test.ci/",
Description: "",
Context: "testci",
}))

t.Run("CreateStatus", doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{
State: api.CommitStatusSuccess,
TargetURL: "http://test.ci/",
Description: "",
Context: "other_context",
}))

req = NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
resp = session.MakeRequest(t, req, http.StatusOK)

doc = NewHTMLParser(t, resp.Body)
// Check that the commit-statuses-trigger (for tooltip) and commit-status (svg) are present
sel := doc.doc.Find("#commits-table tbody tr td.message .commit-statuses-trigger .commit-status")
assert.Equal(t, 1, sel.Length())
}