mirror of
https://gitea.com/gitea/tea.git
synced 2026-10-26 11:01:25 +01:00
perf(pr): read the pull request template with one request
Signed-off-by: bircni <bircni@icloud.com>
This commit is contained in:
3 files changed
+79
-21
No files matched your search
@@ -10,6 +10,7 @@ import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"gitea.dev/tea/internal/clierr"
|
||||
@@ -263,12 +264,15 @@ func TestPullCreateDerivesTitleAndBodyFromSingleCommit(t *testing.T) {
|
||||
|
||||
func TestPullCreateEditorSeedsPullRequestTemplate(t *testing.T) {
|
||||
var payload map[string]any
|
||||
var mu sync.Mutex
|
||||
var templateRequests []string
|
||||
server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch {
|
||||
case strings.HasPrefix(r.URL.Path, "/api/v1/repos/alice/project/raw/"):
|
||||
require.Equal(t, "main", r.URL.Query().Get("ref"))
|
||||
mu.Lock()
|
||||
templateRequests = append(templateRequests, strings.TrimPrefix(r.URL.Path, "/api/v1/repos/alice/project/raw/"))
|
||||
mu.Unlock()
|
||||
if r.URL.Path != "/api/v1/repos/alice/project/raw/.gitea/PULL_REQUEST_TEMPLATE.md" {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
return
|
||||
@@ -289,7 +293,11 @@ func TestPullCreateEditorSeedsPullRequestTemplate(t *testing.T) {
|
||||
deps.Getenv = serverEnv(server.URL, map[string]string{"EDITOR": "editor"})
|
||||
require.NoError(t, Execute(context.Background(), deps, []string{"pr", "create", "-R", "alice/project", "--title", "PR", "--head", "feature", "--base", "main", "--editor", "--json=number", "--no-input"}))
|
||||
require.Equal(t, "## Checklist\n", editor.initial)
|
||||
require.Equal(t, []string{"PULL_REQUEST_TEMPLATE.md", "pull_request_template.md", ".gitea/PULL_REQUEST_TEMPLATE.md"}, templateRequests)
|
||||
require.ElementsMatch(t, []string{
|
||||
"PULL_REQUEST_TEMPLATE.md", "pull_request_template.md",
|
||||
".gitea/PULL_REQUEST_TEMPLATE.md", ".gitea/pull_request_template.md",
|
||||
".github/PULL_REQUEST_TEMPLATE.md", ".github/pull_request_template.md",
|
||||
}, templateRequests)
|
||||
require.Equal(t, "filled", payload["body"])
|
||||
}
|
||||
|
||||
|
||||
+34
-11
@@ -15,6 +15,7 @@ import (
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
gitea "gitea.dev/sdk"
|
||||
@@ -474,20 +475,42 @@ var pullTemplatePaths = []string{
|
||||
const maxPullTemplateBytes = 1 << 20
|
||||
|
||||
// Template returns the first pull request template found at ref, or "" when the repository has none.
|
||||
// All paths are probed concurrently; the result is decided in priority order, so a failure is
|
||||
// reported only when every higher-priority path is missing.
|
||||
func (s Pulls) Template(ctx context.Context, ref string) (string, error) {
|
||||
for _, path := range pullTemplatePaths {
|
||||
file, err := repoCall(ctx, s.Client, s.Repository, func(ctx context.Context, c *gitea.Client, owner, name string) (io.ReadCloser, *gitea.Response, error) {
|
||||
return c.Repositories.GetFileReader(ctx, owner, name, ref, path)
|
||||
type probe struct {
|
||||
content string
|
||||
missing bool
|
||||
err error
|
||||
}
|
||||
probes := make([]probe, len(pullTemplatePaths))
|
||||
var wg sync.WaitGroup
|
||||
for i, path := range pullTemplatePaths {
|
||||
wg.Go(func() {
|
||||
probes[i].content, probes[i].missing, probes[i].err = s.templateAt(ctx, ref, path)
|
||||
})
|
||||
if err != nil {
|
||||
if IsNotFound(err) {
|
||||
continue
|
||||
}
|
||||
return "", err
|
||||
}
|
||||
wg.Wait()
|
||||
for _, p := range probes {
|
||||
if !p.missing {
|
||||
return p.content, p.err
|
||||
}
|
||||
data, err := io.ReadAll(io.LimitReader(file, maxPullTemplateBytes))
|
||||
_ = file.Close()
|
||||
return string(data), err
|
||||
}
|
||||
return "", nil
|
||||
}
|
||||
|
||||
// templateAt reads one template path; missing reports a 404.
|
||||
func (s Pulls) templateAt(ctx context.Context, ref, path string) (content string, missing bool, err error) {
|
||||
file, err := repoCall(ctx, s.Client, s.Repository, func(ctx context.Context, c *gitea.Client, owner, name string) (io.ReadCloser, *gitea.Response, error) {
|
||||
return c.Repositories.GetFileReader(ctx, owner, name, ref, path)
|
||||
})
|
||||
if err != nil {
|
||||
if IsNotFound(err) {
|
||||
return "", true, nil
|
||||
}
|
||||
return "", false, err
|
||||
}
|
||||
data, err := io.ReadAll(io.LimitReader(file, maxPullTemplateBytes))
|
||||
_ = file.Close()
|
||||
return string(data), false, err
|
||||
}
|
||||
@@ -11,7 +11,10 @@ import (
|
||||
"net/http/httptest"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
gitea "gitea.dev/sdk"
|
||||
"gitea.dev/tea/internal/transport"
|
||||
@@ -114,25 +117,28 @@ func TestPullTemplateLookup(t *testing.T) {
|
||||
status map[string]int
|
||||
requests int
|
||||
}{
|
||||
{"root first", "template", "", map[string]int{"PULL_REQUEST_TEMPLATE.md": http.StatusOK, ".gitea/pull_request_template.md": http.StatusOK}, 1},
|
||||
{"uppercase before lowercase", "template", "", map[string]int{".gitea/PULL_REQUEST_TEMPLATE.md": http.StatusOK}, 3},
|
||||
{"later match", "template", "", map[string]int{".github/pull_request_template.md": http.StatusOK}, 6},
|
||||
{"root first", "PULL_REQUEST_TEMPLATE.md", "", map[string]int{"PULL_REQUEST_TEMPLATE.md": http.StatusOK, ".gitea/pull_request_template.md": http.StatusOK}, 6},
|
||||
{"uppercase before lowercase", ".gitea/PULL_REQUEST_TEMPLATE.md", "", map[string]int{".gitea/PULL_REQUEST_TEMPLATE.md": http.StatusOK, ".gitea/pull_request_template.md": http.StatusOK}, 6},
|
||||
{"later match", ".github/pull_request_template.md", "", map[string]int{".github/pull_request_template.md": http.StatusOK}, 6},
|
||||
{"docs is not searched", "", "", map[string]int{"docs/pull_request_template.md": http.StatusOK}, 6},
|
||||
{"none", "", "", nil, 6},
|
||||
{"server error", "", "500", map[string]int{".gitea/pull_request_template.md": http.StatusInternalServerError}, 4},
|
||||
{"server error", "", "500", map[string]int{".gitea/pull_request_template.md": http.StatusInternalServerError}, 6},
|
||||
{"match before error", "pull_request_template.md", "", map[string]int{"pull_request_template.md": http.StatusOK, ".github/PULL_REQUEST_TEMPLATE.md": http.StatusInternalServerError}, 6},
|
||||
{"error before match", "", "500", map[string]int{"pull_request_template.md": http.StatusInternalServerError, ".github/PULL_REQUEST_TEMPLATE.md": http.StatusOK}, 6},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
requests := 0
|
||||
var requests atomic.Int32
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
requests++
|
||||
requests.Add(1)
|
||||
assert.Equal(t, "release/1.0", r.URL.Query().Get("ref"))
|
||||
status, ok := tc.status[strings.TrimPrefix(r.URL.Path, "/api/v1/repos/owner/repo/raw/")]
|
||||
path := strings.TrimPrefix(r.URL.Path, "/api/v1/repos/owner/repo/raw/")
|
||||
status, ok := tc.status[path]
|
||||
if !ok {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
w.WriteHeader(status)
|
||||
_, _ = w.Write([]byte("template"))
|
||||
_, _ = w.Write([]byte(path))
|
||||
}))
|
||||
defer server.Close()
|
||||
got, err := (Pulls{Client: transport.New(server.URL, "", false), Repository: "owner/repo"}).Template(context.Background(), "release/1.0")
|
||||
@@ -142,11 +148,32 @@ func TestPullTemplateLookup(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
}
|
||||
require.Equal(t, tc.want, got)
|
||||
require.Equal(t, tc.requests, requests)
|
||||
require.Equal(t, tc.requests, int(requests.Load()))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestPullTemplateProbesConcurrently(t *testing.T) {
|
||||
// Every probe only completes once all of them are in flight.
|
||||
var arrived sync.WaitGroup
|
||||
arrived.Add(len(pullTemplatePaths))
|
||||
allArrived := make(chan struct{})
|
||||
go func() { arrived.Wait(); close(allArrived) }()
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
arrived.Done()
|
||||
select {
|
||||
case <-allArrived:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Errorf("%s did not run concurrently with the other template probes", r.URL.Path)
|
||||
}
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}))
|
||||
defer server.Close()
|
||||
got, err := (Pulls{Client: transport.New(server.URL, "", false), Repository: "owner/repo"}).Template(context.Background(), "main")
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, got)
|
||||
}
|
||||
|
||||
func TestPullMergeExplainsRefusal(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name, pull, want string
|
||||
|
||||
Reference in new issue
Block a user