dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

perf(github): speed up CI check feedback (#1882)

Browse Source
This commit is contained in:
Carlos Florêncio
2026-07-23 15:08:05 +01:00
committed by GitHub
parent f6007de1cb
commit bfc372b4ad
2 changed files with 261 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/internal/github/client_helpers.go
+38 -10
View File
@@ -35,17 +35,45 @@ func getPRFeedback(ctx context.Context, c Client, owner, repo string, number int
if err != nil {
return nil, err
}
reviews, err := c.ListPRReviews(ctx, owner, repo, number)
if err != nil {
return nil, err
fetchCtx, cancel := context.WithCancel(ctx)
defer cancel()
type feedbackResult struct {
reviews []PRReview
comments []PRComment
checks []CheckRun
err error
}
comments, err := c.ListPRComments(ctx, owner, repo, number, nil)
if err != nil {
return nil, err
}
checks, err := c.ListCheckRuns(ctx, owner, repo, pr.HeadSHA)
if err != nil {
return nil, err
results := make(chan feedbackResult, 3)
go func() {
value, callErr := c.ListPRReviews(fetchCtx, owner, repo, number)
results <- feedbackResult{reviews: value, err: callErr}
}()
go func() {
value, callErr := c.ListPRComments(fetchCtx, owner, repo, number, nil)
results <- feedbackResult{comments: value, err: callErr}
}()
go func() {
value, callErr := c.ListCheckRuns(fetchCtx, owner, repo, pr.HeadSHA)
results <- feedbackResult{checks: value, err: callErr}
}()
var reviews []PRReview
var comments []PRComment
var checks []CheckRun
for range 3 {
result := <-results
if result.err != nil {
return nil, result.err
}
if result.reviews != nil {
reviews = result.reviews
}
if result.comments != nil {
comments = result.comments
}
if result.checks != nil {
checks = result.checks
}
}
// Ensure non-nil slices so JSON serialization produces [] instead of null.
if reviews == nil {
apps/backend/internal/github/client_helpers_test.go
+223
View File
@@ -1,10 +1,233 @@
package github
import (
"context"
"errors"
"testing"
"time"
)
func TestGetPRFeedbackStartsIndependentRequestsConcurrently(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()
started := make(chan string, 3)
release := make(chan struct{})
pr := &PR{Number: 1, HeadSHA: "head-sha"}
reviews := []PRReview{{ID: 1, State: reviewStateChangesRequested}}
comments := []PRComment{{ID: 2, Body: "please fix"}}
checks := []CheckRun{{Name: "lint", Status: checkStatusCompleted, Conclusion: checkConclusionFail}}
client := feedbackConcurrencyClient{
started: started,
release: release,
pr: pr,
reviews: reviews,
comments: comments,
checks: checks,
}
result := make(chan struct {
feedback *PRFeedback
err error
}, 1)
go func() {
feedback, err := getPRFeedback(ctx, &client, "owner", "repo", 1)
result <- struct {
feedback *PRFeedback
err error
}{feedback: feedback, err: err}
}()
assertStartedRequests(t, ctx, started)
close(release)
got := <-result
if got.err != nil {
t.Fatalf("getPRFeedback() error = %v", got.err)
}
if got.feedback.PR != pr {
t.Error("getPRFeedback() did not preserve the PR")
}
if len(got.feedback.Reviews) != 1 || got.feedback.Reviews[0] != reviews[0] {
t.Errorf("getPRFeedback() reviews = %#v, want %#v", got.feedback.Reviews, reviews)
}
if len(got.feedback.Comments) != 1 || got.feedback.Comments[0] != comments[0] {
t.Errorf("getPRFeedback() comments = %#v, want %#v", got.feedback.Comments, comments)
}
if len(got.feedback.Checks) != 1 || got.feedback.Checks[0] != checks[0] {
t.Errorf("getPRFeedback() checks = %#v, want %#v", got.feedback.Checks, checks)
}
if !got.feedback.HasIssues {
t.Error("getPRFeedback() HasIssues = false, want true")
}
}
func TestGetPRFeedbackNormalizesNilResponses(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()
started := make(chan string, 3)
release := make(chan struct{})
client := feedbackConcurrencyClient{
started: started,
release: release,
pr: &PR{HeadSHA: "head-sha"},
}
result := make(chan struct {
feedback *PRFeedback
err error
}, 1)
go func() {
feedback, err := getPRFeedback(ctx, &client, "owner", "repo", 1)
result <- struct {
feedback *PRFeedback
err error
}{feedback: feedback, err: err}
}()
assertStartedRequests(t, ctx, started)
close(release)
got := <-result
if got.err != nil {
t.Fatalf("getPRFeedback() error = %v", got.err)
}
if got.feedback.Reviews == nil || got.feedback.Comments == nil || got.feedback.Checks == nil {
t.Errorf("getPRFeedback() slices = %#v, want non-nil empty slices", got.feedback)
}
if got.feedback.HasIssues {
t.Error("getPRFeedback() HasIssues = true, want false")
}
}
func TestGetPRFeedbackReturnsWorkerErrorWithoutWaitingForBlockedSibling(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()
failure := errors.New("checks failed")
started := make(chan string, 3)
release := make(chan struct{})
cancelled := make(chan string, 2)
client := feedbackConcurrencyClient{
started: started,
release: release,
cancelled: cancelled,
pr: &PR{HeadSHA: "head-sha"},
fail: "checks",
failure: failure,
}
result := make(chan error, 1)
go func() {
_, err := getPRFeedback(ctx, &client, "owner", "repo", 1)
result <- err
}()
assertStartedRequests(t, ctx, started)
close(release)
select {
case err := <-result:
if !errors.Is(err, failure) {
t.Fatalf("getPRFeedback() error = %v, want %v", err, failure)
}
case <-ctx.Done():
t.Fatal("getPRFeedback() waited for blocked sibling")
}
assertCancelledRequests(t, ctx, cancelled)
}
func assertStartedRequests(t *testing.T, ctx context.Context, started <-chan string) {
t.Helper()
want := map[string]bool{"reviews": true, "comments": true, "checks": true}
for range 3 {
select {
case request := <-started:
if !want[request] {
t.Fatalf("unexpected or duplicate request start %q", request)
}
delete(want, request)
case <-ctx.Done():
t.Fatalf("requests that did not start: %v", want)
}
}
if len(want) != 0 {
t.Fatalf("requests that did not start: %v", want)
}
}
func assertCancelledRequests(t *testing.T, ctx context.Context, cancelled <-chan string) {
t.Helper()
want := map[string]bool{"reviews": true, "comments": true}
for range 2 {
select {
case request := <-cancelled:
if !want[request] {
t.Fatalf("unexpected or duplicate request cancellation %q", request)
}
delete(want, request)
case <-ctx.Done():
t.Fatalf("requests that were not cancelled: %v", want)
}
}
if len(want) != 0 {
t.Fatalf("requests that were not cancelled: %v", want)
}
}
type feedbackConcurrencyClient struct {
Client
started chan<- string
release <-chan struct{}
cancelled chan<- string
pr *PR
reviews []PRReview
comments []PRComment
checks []CheckRun
fail string
failure error
}
func (c *feedbackConcurrencyClient) GetPR(context.Context, string, string, int) (*PR, error) {
return c.pr, nil
}
func (c *feedbackConcurrencyClient) ListPRReviews(ctx context.Context, _ string, _ string, _ int) ([]PRReview, error) {
if err := c.wait(ctx, "reviews"); err != nil {
return nil, err
}
return c.reviews, nil
}
func (c *feedbackConcurrencyClient) ListPRComments(ctx context.Context, _ string, _ string, _ int, _ *time.Time) ([]PRComment, error) {
if err := c.wait(ctx, "comments"); err != nil {
return nil, err
}
return c.comments, nil
}
func (c *feedbackConcurrencyClient) ListCheckRuns(ctx context.Context, _ string, _ string, _ string) ([]CheckRun, error) {
if err := c.wait(ctx, "checks"); err != nil {
return nil, err
}
return c.checks, nil
}
func (c *feedbackConcurrencyClient) wait(ctx context.Context, request string) error {
c.started <- request
if c.fail != "" && c.fail != request {
<-ctx.Done()
c.cancelled <- request
return ctx.Err()
}
select {
case <-c.release:
if c.fail == request {
return c.failure
}
return nil
case <-ctx.Done():
return ctx.Err()
}
}
func TestBuildReviewSearchQuery(t *testing.T) {
tests := []struct {
name string