diff --git a/bots-common/gitea_utils.go b/bots-common/gitea_utils.go index e681d51..3aa0674 100644 --- a/bots-common/gitea_utils.go +++ b/bots-common/gitea_utils.go @@ -23,6 +23,7 @@ import ( "io" "os" "path/filepath" + "slices" "time" transport "github.com/go-openapi/runtime/client" @@ -59,8 +60,12 @@ type GiteaMaintainershipInterface interface { FetchMaintainershipFile(org, prjGit, branch string) ([]byte, error) } -type GiteaPRReviewFetcher interface { - GetPullRequestAndReviews(org, project string, num int64) (*models.PullRequest, []*models.PullReview, error) +type GiteaPRFetcher interface { + GetPullRequest(org, project string, num int64) (*models.PullRequest, error) +} + +type GiteaReviewFetcher interface { + GetPullRequestReviews(org, project string, num int64) ([]*models.PullReview, error) } type GiteaReviewRequester interface { @@ -74,7 +79,7 @@ type GiteaReviewer interface { type Gitea interface { GiteaReviewRequester GiteaReviewer - GiteaPRReviewFetcher + GiteaPRFetcher GetPullNotifications(since *time.Time) ([]*models.NotificationThread, error) SetNotificationRead(notificationId int64) error @@ -110,7 +115,7 @@ func (gitea *GiteaTransport) FetchMaintainershipFile(org, repo, branch string) ( return gitea.GetRepositoryFileContent(org, repo, branch, MaintainershipFile) } -func (gitea *GiteaTransport) GetPullRequestAndReviews(org, project string, num int64) (*models.PullRequest, []*models.PullReview, error) { +func (gitea *GiteaTransport) GetPullRequest(org, project string, num int64) (*models.PullRequest, error) { pr, err := gitea.client.Repository.RepoGetPullRequest( repository.NewRepoGetPullRequestParams(). WithDefaults(). @@ -120,26 +125,39 @@ func (gitea *GiteaTransport) GetPullRequestAndReviews(org, project string, num i gitea.transport.DefaultAuthentication, ) - if err != nil { - return nil, nil, err + return pr.Payload, err +} + +func (gitea *GiteaTransport) GetPullReviews(org, project string, num int64) ([]*models.PullReview, error) { + limit := int64(20) + var page int64 + + var allReviews []*models.PullReview + + for { + reviews, err := gitea.client.Repository.RepoListPullReviews( + repository.NewRepoListPullReviewsParams(). + WithDefaults(). + WithOwner(org). + WithRepo(project). + WithIndex(num). + WithPage(&page). + WithLimit(&limit), + gitea.transport.DefaultAuthentication, + ) + + if err != nil { + return nil, err + } + + allReviews = slices.Concat(allReviews, reviews.Payload) + if len(reviews.Payload) < int(limit) { + break + } + page++ } - limit := int64(1000) - reviews, err := gitea.client.Repository.RepoListPullReviews( - repository.NewRepoListPullReviewsParams(). - WithDefaults(). - WithOwner(org). - WithRepo(project). - WithIndex(num). - WithLimit(&limit), - gitea.transport.DefaultAuthentication, - ) - - if err != nil { - return nil, nil, err - } - - return pr.Payload, reviews.Payload, nil + return allReviews, nil } func (gitea *GiteaTransport) GetPullNotifications(since *time.Time) ([]*models.NotificationThread, error) { diff --git a/workflow-pr/pr.go b/workflow-pr/pr.go index 7d01996..401c229 100644 --- a/workflow-pr/pr.go +++ b/workflow-pr/pr.go @@ -1,15 +1,60 @@ package main import ( + "bufio" + "errors" + "slices" + "strings" + + "src.opensuse.org/autogits/common" "src.opensuse.org/autogits/common/gitea-generated/models" ) -//go:generate mockgen -source=pr.go -destination=mock/pr.go -typed -type GiteaPRInterface interface { - GetPullRequestAndReviews(org, pkg string, num int64) (*models.PullRequest, []*models.PullReview, error) - - GetAssociatedPrjGitPR(org, repo string, id int64) (*models.PullRequest, error) - GetAssociatedPRs(org, repo string, id int64) ([]*models.PullRequest, error) +type PRInfo struct { + pr *models.PullRequest + reviews []*models.PullReview } +type ReviewSet struct { + maintainers MaintainershipData + prs []PRInfo +} + +func readPRData(gitea common.GiteaPRFetcher, 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.GetPullRequest(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.GiteaPRFetcher, 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 &ReviewSet{prs: prs}, nil +} diff --git a/workflow-pr/pr_processor_sync.go b/workflow-pr/pr_processor_sync.go index 255cf96..2d051e3 100644 --- a/workflow-pr/pr_processor_sync.go +++ b/workflow-pr/pr_processor_sync.go @@ -39,7 +39,7 @@ func (o *PullRequestSynced) Process(req *common.PullRequestWebhookEvent, git com // need to verify that submodule in the PR for prjgit // is still pointing to the HEAD of the PR - pr, _, err := o.gitea.GetPullRequestAndReviews(req.Repository.Owner.Username, req.Repository.Name, req.Number) + pr, err := o.gitea.GetPullRequest(req.Repository.Owner.Username, req.Repository.Name, req.Number) if err != nil { return fmt.Errorf("Cannot fetch PR data from gitea: %w", err) } @@ -49,7 +49,7 @@ func (o *PullRequestSynced) Process(req *common.PullRequestWebhookEvent, git com return fmt.Errorf("Package update associated with invalid number of projects. Expected 1. Got %d", len(prs)) } - prjPr, _, err := o.gitea.GetPullRequestAndReviews(prs[0].Org, prs[0].Repo, prs[0].Num) + prjPr, err := o.gitea.GetPullRequest(prs[0].Org, prs[0].Repo, prs[0].Num) if err != nil { return fmt.Errorf("Cannot get PrjGit PR in processPullRequestSync. Err: %w", err) diff --git a/workflow-pr/pr_processor_sync_test.go b/workflow-pr/pr_processor_sync_test.go index b373210..9877304 100644 --- a/workflow-pr/pr_processor_sync_test.go +++ b/workflow-pr/pr_processor_sync_test.go @@ -132,8 +132,8 @@ func TestSyncPR(t *testing.T) { defer func() { PrjGitPR.Head.Sha = oldSha }() PrjGitPR.Head.Sha = "ab8adab91edb476b9762097d10c6379aa71efd6b60933a1c0e355ddacf419a95" - mock.EXPECT().GetPullRequestAndReviews(config.Organization, "testRepo", event.Pull_Request.Number).Return(modelPR, nil, nil) - mock.EXPECT().GetPullRequestAndReviews(config.Organization, "prj", int64(24)).Return(PrjGitPR, nil, nil) + mock.EXPECT().GetPullRequest(config.Organization, "testRepo", event.Pull_Request.Number).Return(modelPR, nil) + mock.EXPECT().GetPullRequest(config.Organization, "prj", int64(24)).Return(PrjGitPR, nil) err := pr.Process(event, git, config) @@ -155,7 +155,7 @@ func TestSyncPR(t *testing.T) { setupGitForTests(t, git) expectedErr := errors.New("Missing PR should throw error") - mock.EXPECT().GetPullRequestAndReviews(config.Organization, "tester", event.Pull_Request.Number).Return(modelPR, nil, expectedErr) + mock.EXPECT().GetPullRequest(config.Organization, "tester", event.Pull_Request.Number).Return(modelPR, expectedErr) err := pr.Process(event, git, config) @@ -184,8 +184,8 @@ func TestSyncPR(t *testing.T) { git.DebugLogger = true DebugMode = true // mock.EXPECT().GetAssociatedPrjGitPR(event).Return(PrjGitPR, nil) - mock.EXPECT().GetPullRequestAndReviews(config.Organization, "testRepo", event.Pull_Request.Number).Return(modelPR, nil, nil) - mock.EXPECT().GetPullRequestAndReviews(config.Organization, "prj", int64(24)).Return(PrjGitPR, nil, nil) + mock.EXPECT().GetPullRequest(config.Organization, "testRepo", event.Pull_Request.Number).Return(modelPR, nil) + mock.EXPECT().GetPullRequest(config.Organization, "prj", int64(24)).Return(PrjGitPR, nil) err := pr.Process(event, git, config) @@ -206,8 +206,8 @@ func TestSyncPR(t *testing.T) { */ os.RemoveAll(path.Join(git.GitPath, common.DefaultGitPrj)) - mock.EXPECT().GetPullRequestAndReviews(config.Organization, "testRepo", event.Pull_Request.Number).Return(modelPR, nil, nil) - mock.EXPECT().GetPullRequestAndReviews(config.Organization, "prj", int64(24)).Return(PrjGitPR, nil, nil) + mock.EXPECT().GetPullRequest(config.Organization, "testRepo", event.Pull_Request.Number).Return(modelPR, nil) + mock.EXPECT().GetPullRequest(config.Organization, "prj", int64(24)).Return(PrjGitPR, nil) err = pr.Process(event, git, config) if err != nil { diff --git a/workflow-pr/pr_test.go b/workflow-pr/pr_test.go index d5b7764..d1d3978 100644 --- a/workflow-pr/pr_test.go +++ b/workflow-pr/pr_test.go @@ -1,20 +1,127 @@ package main import ( + "errors" + "os" + "os/exec" + "path" "testing" -/* + "go.uber.org/mock/gomock" + "src.opensuse.org/autogits/common" "src.opensuse.org/autogits/common/gitea-generated/models" - mock_main "src.opensuse.org/workflow-pr/mock" - */ + mock_common "src.opensuse.org/autogits/common/mock" ) func TestPR(t *testing.T) { - t.Run("Test simple PR to PrjGit checkout", func(t *testing.T) { - // ctl := gomock.NewController(t) - // pr := mock_main.NewMockGiteaPRInterface(ctl) - - // pr.EXPECT().GetPR("foo", "bar", 22).Return(&models.PullRequest{}, nil) - }) -} + cwd, _ := os.Getwd() + cmd := exec.Command("/usr/bin/bash", path.Join(cwd, "test_repo_setup.sh")) + cmd.Dir = t.TempDir() + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + baseConfig := common.AutogitConfig{ + Reviewers: []string{"super1", "super2"}, + Branch: "branch", + Organization: "test", + GitProjectName: "barPrj", + } + + type prdata struct { + pr *models.PullRequest + err error + } + + tests := []struct { + name string + data []prdata + api_error string + + resLen int + reviewed bool + }{ + { + 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 { + t.Run(test.name, func(t *testing.T) { + ctl := gomock.NewController(t) + mock := mock_common.NewMockGiteaPRFetcher(ctl) + + var test_err error + for _, data := range test.data { + mock.EXPECT().GetPullRequest(data.pr.Base.Repo.Owner.UserName, data.pr.Base.Repo.Name, data.pr.Index).Return(data.pr, data.err) + if data.err != nil { + test_err = data.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)) + } +/* + if res.IsReviewed() != test.reviewed { + t.Error("expected reviewed to be NOT", res.IsReviewed()) + } + */ + }) + } +} diff --git a/workflow-pr/review.go b/workflow-pr/review.go deleted file mode 100644 index 368fe98..0000000 --- a/workflow-pr/review.go +++ /dev/null @@ -1,63 +0,0 @@ -package main - -import ( - "bufio" - "errors" - "slices" - "strings" - - "src.opensuse.org/autogits/common" - "src.opensuse.org/autogits/common/gitea-generated/models" -) - -type Review interface { - IsApproved() (bool, error) -} - -type PRInfo struct { - pr *models.PullRequest - reviews []*models.PullReview -} - -type ReviewSet struct { - maintainers MaintainershipData - prs []PRInfo -} - -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 &ReviewSet{prs: prs}, nil -} diff --git a/workflow-pr/review_old.go b/workflow-pr/review_old.go index 74c67dc..034e2d2 100644 --- a/workflow-pr/review_old.go +++ b/workflow-pr/review_old.go @@ -1,5 +1,6 @@ package main +/* import ( "bufio" "slices" @@ -8,7 +9,6 @@ import ( "src.opensuse.org/autogits/common" "src.opensuse.org/autogits/common/gitea-generated/models" ) - func fetchPRInfo(gitea GiteaPRInterface, pr common.BasicPR) PRInfo { data, reviews, _ := gitea.GetPullRequestAndReviews(pr.Org, pr.Repo, pr.Num) return PRInfo{ @@ -73,4 +73,4 @@ func (rs *ReviewSet) IsMaintainerApproved() (bool, error) { } } return true, nil -} +}*/ diff --git a/workflow-pr/review_old_test.go b/workflow-pr/review_old_test.go index 0ac89c1..ad939a4 100644 --- a/workflow-pr/review_old_test.go +++ b/workflow-pr/review_old_test.go @@ -1,5 +1,5 @@ package main - +/* import ( "bufio" "strings" @@ -282,3 +282,4 @@ func TestReviewApproval(t *testing.T) { }) } } +*/ diff --git a/workflow-pr/review_test.go b/workflow-pr/review_test.go deleted file mode 100644 index 682c49e..0000000 --- a/workflow-pr/review_test.go +++ /dev/null @@ -1,121 +0,0 @@ -package main - -import ( - "errors" - "os" - "os/exec" - "path" - "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" -) - -func TestReview(t *testing.T) { - cwd, _ := os.Getwd() - cmd := exec.Command("/usr/bin/bash", path.Join(cwd, "test_repo_setup.sh")) - cmd.Dir = t.TempDir() - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - - baseConfig := common.AutogitConfig{ - Reviewers: []string{"super1", "super2"}, - Branch: "branch", - Organization: "test", - GitProjectName: "barPrj", - } - - 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", - 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 { - t.Run(test.name, func(t *testing.T) { - ctl := gomock.NewController(t) - mock := mock_common.NewMockGiteaPRReviewFetcher(ctl) - - 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 - } - } - - 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)) - } - }) - } -}