From fe2a577b3b6defa4c92f8f8f695e66b67faef54c2d28343bc4308a7f060f5217 Mon Sep 17 00:00:00 2001 From: Adam Majer Date: Tue, 17 Dec 2024 18:19:04 +0100 Subject: [PATCH] wip --- workflow-pr/review.go | 38 +++++++++++++-- workflow-pr/review_test.go | 98 +++++++++++++++++++++++++++++++++----- 2 files changed, 120 insertions(+), 16 deletions(-) diff --git a/workflow-pr/review.go b/workflow-pr/review.go index 3763e75..368fe98 100644 --- a/workflow-pr/review.go +++ b/workflow-pr/review.go @@ -1,7 +1,10 @@ package main import ( + "bufio" "errors" + "slices" + "strings" "src.opensuse.org/autogits/common" "src.opensuse.org/autogits/common/gitea-generated/models" @@ -21,11 +24,40 @@ type ReviewSet struct { prs []PRInfo } -func FetchReviewSet(gitea common.GiteaPRReviewFetcher, org, repo string, num int64) (*ReviewSet, error) { - _,_, err := gitea.GetPullRequestAndReviews(org, repo, num) +func readPRData(gitea common.GiteaPRReviewFetcher, org, repo string, num int64, currentSet []PRInfo) ([]PRInfo, error) { + for _, p := range currentSet { + if num == p.pr.Index && repo == p.pr.Base.Repo.Name && org == p.pr.Base.Repo.Owner.UserName { + return nil, nil + } + } + + pr, _, err := gitea.GetPullRequestAndReviews(org, repo, num) + if err != nil { + return nil, err + } + _, refPRs := common.ExtractDescriptionAndPRs(bufio.NewScanner(strings.NewReader(pr.Body))) + if len(refPRs) < 1 { + return nil, errors.New("missing links") + } + + retSet := []PRInfo{PRInfo{pr: pr}} + + for _, prdata := range refPRs { + data, err := readPRData(gitea, prdata.Org, prdata.Repo, prdata.Num, slices.Concat(currentSet, retSet)) + if err != nil { + return nil, err + } + retSet = slices.Concat(retSet, data) + } + + return retSet, nil +} + +func FetchReviewSet(gitea common.GiteaPRReviewFetcher, org, repo string, num int64, config *common.AutogitConfig) (*ReviewSet, error) { + prs, err := readPRData(gitea, org, repo, num, nil) if err != nil { return nil, err } - return nil, errors.New("Error") + return &ReviewSet{prs: prs}, nil } diff --git a/workflow-pr/review_test.go b/workflow-pr/review_test.go index 57f76f2..682c49e 100644 --- a/workflow-pr/review_test.go +++ b/workflow-pr/review_test.go @@ -8,6 +8,7 @@ import ( "testing" "go.uber.org/mock/gomock" + "src.opensuse.org/autogits/common" "src.opensuse.org/autogits/common/gitea-generated/models" mock_common "src.opensuse.org/autogits/common/mock" ) @@ -20,17 +21,65 @@ func TestReview(t *testing.T) { t.Fatal(string(out)) } - tests := []struct { - name string + baseConfig := common.AutogitConfig{ + Reviewers: []string{"super1", "super2"}, + Branch: "branch", + Organization: "test", + GitProjectName: "barPrj", + } - org string - repo string - pr_no int64 - pr *models.PullRequest - err error + type prdata struct { + pr *models.PullRequest + err error + } + + tests := []struct { + name string + data []prdata + api_error string + + resLen int }{ - {name: "Error fetching ReviewSet if there are missing links", org: "test", repo: "repo", pr_no: 42, pr: &models.PullRequest{Body: ""}}, - {name: "Error fetching ReviewSet if there are missing links", org: "test", repo: "repo", pr_no: 42, pr: &models.PullRequest{Body: ""}, err: errors.New("Missing PR")}, + { + name: "Error fetching ReviewSet if there are missing links", + data: []prdata{ + {pr: &models.PullRequest{Body: "", Index: 42, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "test"}}}}, err: errors.New("Missing PR")}, + }, + }, + + { + name: "Error fetching is missing links", + data: []prdata{ + {pr: &models.PullRequest{Body: "", Index: 42, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "test"}}}}}, + }, api_error: "missing links", + }, + + { + name: "Review set is not consistent", + data: []prdata{ + {pr: &models.PullRequest{Body: "PR: foo/barPrj#22", Index: 42, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "test"}}}}}, + {pr: &models.PullRequest{Body: "", Index: 22, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "barPrj", Owner: &models.User{UserName: "foo"}}}}}, + }, + api_error: "missing links", + }, + + { + name: "Review set is consistent: 1pkg", + data: []prdata{ + {pr: &models.PullRequest{Body: "PR: foo/barPrj#22", Index: 42, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "test"}}}}}, + {pr: &models.PullRequest{Body: "PR: test/repo#42", Index: 22, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "barPrj", Owner: &models.User{UserName: "foo"}}}}}, + }, + resLen: 2, + }, + { + name: "Review set is consistent: 2pkg", + data: []prdata{ + {pr: &models.PullRequest{Body: "PR: foo/barPrj#22", Index: 42, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "test"}}}}}, + {pr: &models.PullRequest{Body: "PR: test/repo#42\nPR: test/repo2#41", Index: 22, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "barPrj", Owner: &models.User{UserName: "foo"}}}}}, + {pr: &models.PullRequest{Body: "PR: foo/barPrj#22", Index: 41, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "repo2", Owner: &models.User{UserName: "test"}}}}}, + }, + resLen: 3, + }, } for _, test := range tests { @@ -38,11 +87,34 @@ func TestReview(t *testing.T) { ctl := gomock.NewController(t) mock := mock_common.NewMockGiteaPRReviewFetcher(ctl) - mock.EXPECT().GetPullRequestAndReviews(test.org, test.repo, test.pr_no).Return(test.pr, nil, test.err) + var test_err error + for _, data := range test.data { + mock.EXPECT().GetPullRequestAndReviews(data.pr.Base.Repo.Owner.UserName, data.pr.Base.Repo.Name, data.pr.Index).Return(data.pr, nil, data.err) + if data.err != nil { + test_err = data.err + } + } - _, err := FetchReviewSet(mock, "test", "repo", 42) - if test.err != nil && err == nil { - t.Fatal("Expected", test.err, "but got", err) + res, err := FetchReviewSet(mock, "test", "repo", 42, &baseConfig) + if err == nil { + if test_err != nil { + t.Fatal("Expected", test_err, "but got", err) + } + } else { + if test.api_error != "" { + if err.Error() != test.api_error { + t.Fatal("expected", test.api_error, "but got", err) + } + } else if test_err != err { + t.Fatal("expected", test_err, "but got", err) + } + return + } + + t.Log(res) + + if test.resLen != len(res.prs) { + t.Error("expected result len", test.resLen, "but got", len(res.prs)) } }) }