Add ReviewSet.ConsistentSet() check

This commit is contained in:
Adam Majer 2025-01-02 13:46:59 +01:00
parent 888582a74a
commit cfe15a0551
4 changed files with 214 additions and 111 deletions

12
dev_test_helper.sh Executable file
View File

@ -0,0 +1,12 @@
#!/bin/sh
if [ "x$1" = 'x' ]; then
echo 'param to `go test --run ...` missing'
exit 1
fi
while true; do
go test --run "$1"
fsnotifywait -qqr -e close_write .. && clear;
done

View File

@ -79,6 +79,7 @@ func main() {
req := new(RequestProcessor) req := new(RequestProcessor)
req.configuredRepos = make(map[string][]*common.AutogitConfig) req.configuredRepos = make(map[string][]*common.AutogitConfig)
req.git = &common.GitHandlerGeneratorImpl{}
orgs := make([]string, 0, 1) orgs := make([]string, 0, 1)
for _, c := range configs { for _, c := range configs {
if slices.Contains(c.Workflows, "pr") { if slices.Contains(c.Workflows, "pr") {

View File

@ -32,11 +32,6 @@ func readPRData(gitea common.GiteaPRFetcher, org, repo string, num int64, curren
return nil, err return nil, err
} }
_, refPRs := common.ExtractDescriptionAndPRs(bufio.NewScanner(strings.NewReader(pr.Body))) _, refPRs := common.ExtractDescriptionAndPRs(bufio.NewScanner(strings.NewReader(pr.Body)))
/*
if len(refPRs) < 1 {
return nil, errors.New("missing links")
}
*/
retSet := []PRInfo{PRInfo{pr: pr}} retSet := []PRInfo{PRInfo{pr: pr}}
@ -61,12 +56,43 @@ func FetchReviewSet(gitea common.GiteaPRFetcher, org, repo string, num int64, co
} }
func (rs *ReviewSet) GetPrjGitPR() (*models.PullRequest, error) { func (rs *ReviewSet) GetPrjGitPR() (*models.PullRequest, error) {
var ret *models.PullRequest
for _, prinfo := range rs.prs { for _, prinfo := range rs.prs {
if prinfo.pr.Base.Repo.Name == rs.config.GitProjectName && prinfo.pr.Base.Repo.Owner.UserName == rs.config.Organization { if prinfo.pr.Base.Repo.Name == rs.config.GitProjectName && prinfo.pr.Base.Repo.Owner.UserName == rs.config.Organization {
return prinfo.pr, nil if ret == nil {
ret = prinfo.pr
} else {
return nil, errors.New("Multiple PrjGit PRs in one review set")
}
} }
} }
if ret != nil {
return ret, nil
}
return nil, errors.New("No PrjGit PR found") return nil, errors.New("No PrjGit PR found")
} }
func (rs *ReviewSet) IsConsistent() bool {
prjpr, err := rs.GetPrjGitPR()
if err != nil {
return false
}
_, prjpr_set := common.ExtractDescriptionAndPRs(bufio.NewScanner(strings.NewReader(prjpr.Body)))
if len(prjpr_set) != len(rs.prs)-1 { // 1 to many mapping
return false
}
for _, prinfo := range rs.prs {
if prjpr == prinfo.pr {
continue
}
_, prs := common.ExtractDescriptionAndPRs(bufio.NewScanner(strings.NewReader(prinfo.pr.Body)))
if len(prs) != 1 || prs[0].Repo != prjpr.Base.Repo.Name || prs[0].Org != prjpr.Base.Repo.Owner.UserName || prs[0].Num != prjpr.Index {
return false
}
}
return true
}

View File

@ -33,129 +33,193 @@ func TestPR(t *testing.T) {
err error err error
} }
tests := []struct { t.Run("TestPR", func(t *testing.T) {
name string tests := []struct {
data []prdata name string
api_error string data []prdata
api_error string
resLen int resLen int
reviewed bool reviewed bool
prjGitPRIndex int consistentSet bool
}{ prjGitPRIndex int
{
name: "Error fetching PullRequest", customMockSetup func(*mock_common.MockGiteaPRFetcher) error
data: []prdata{ reviewSetFetcher func(*mock_common.MockGiteaPRFetcher) (*ReviewSet, error)
{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 PullRequest",
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, fixable", name: "Error fetching PullRequest in PrjGit",
data: []prdata{ data: []prdata{
{pr: &models.PullRequest{Body: "", Index: 42, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "test"}}}}}, {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"}}}}, err: errors.New("Missing PR")},
},
}, },
resLen: 1,
prjGitPRIndex: -1,
},
{ {
name: "Review set is not consistent, but fixable", name: "Error fetching is missing links, fixable",
data: []prdata{ 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: 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"}}}}}, },
resLen: 1,
prjGitPRIndex: -1,
}, },
resLen: 2, {
prjGitPRIndex: 1, name: "Review set is not consistent, but fixable",
}, 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"}}}}},
name: "Review set is consistent: 1pkg", },
data: []prdata{ resLen: 2,
{pr: &models.PullRequest{Body: "PR: foo/barPrj#22", Index: 42, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "test"}}}}}, prjGitPRIndex: 1,
{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,
prjGitPRIndex: 1, {
}, name: "Review set is consistent: 1pkg",
{ data: []prdata{
name: "Review set is consistent: 2pkg", {pr: &models.PullRequest{Body: "PR: foo/barPrj#22", Index: 42, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "test"}}}}},
data: []prdata{ {pr: &models.PullRequest{Body: "PR: test/repo#42", Index: 22, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "barPrj", Owner: &models.User{UserName: "foo"}}}}},
{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"}}}}}, resLen: 2,
{pr: &models.PullRequest{Body: "PR: foo/barPrj#22", Index: 41, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "repo2", Owner: &models.User{UserName: "test"}}}}}, prjGitPRIndex: 1,
consistentSet: true,
}, },
resLen: 3, {
prjGitPRIndex: 1, 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,
prjGitPRIndex: 1,
consistentSet: true,
},
{
name: "Review set of prjgit PR is consistent",
data: []prdata{
{pr: &models.PullRequest{Body: "", Index: 42, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "barPrj", Owner: &models.User{UserName: "foo"}}}}},
},
resLen: 1,
prjGitPRIndex: 0,
consistentSet: true,
reviewSetFetcher: func(mock *mock_common.MockGiteaPRFetcher) (*ReviewSet, error) {
return FetchReviewSet(mock, "foo", "barPrj", 42, &baseConfig)
},
},
{
name: "Review set is inconsistent: 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/repo2#41", Index: 20, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "barPrj", Owner: &models.User{UserName: "foo"}}}}},
{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#20", Index: 41, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "repo2", Owner: &models.User{UserName: "test"}}}}},
},
resLen: 4,
prjGitPRIndex: -1,
},
{
name: "Review set is inconsistent: not pointing at PrjGit",
data: []prdata{
{pr: &models.PullRequest{Body: "PR: test/repo2#41", 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: test/repo#42", Index: 41, Base: &models.PRBranchInfo{Repo: &models.Repository{Name: "repo2", Owner: &models.User{UserName: "test"}}}}},
},
resLen: 2,
prjGitPRIndex: -1,
},
}
for _, test := range tests { for _, test := range tests {
t.Run(test.name, func(t *testing.T) { t.Run(test.name, func(t *testing.T) {
ctl := gomock.NewController(t) ctl := gomock.NewController(t)
mock := mock_common.NewMockGiteaPRFetcher(ctl) mock := mock_common.NewMockGiteaPRFetcher(ctl)
var test_err error var test_err error
for _, data := range test.data { if test.customMockSetup != nil {
mock.EXPECT().GetPullRequest(data.pr.Base.Repo.Owner.UserName, data.pr.Base.Repo.Name, data.pr.Index).Return(data.pr, data.err) if err := test.customMockSetup(mock); err != nil {
if data.err != nil { test_err = err
test_err = data.err }
} } else {
} 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).AnyTimes()
res, err := FetchReviewSet(mock, "test", "repo", 42, &baseConfig) if data.err != nil {
if err == nil { test_err = data.err
if test_err != nil { }
t.Fatal("Expected", test_err, "but got", err)
}
} else {
if res != nil {
t.Fatal("error but got ReviewSet?")
}
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) var res *ReviewSet
var err error
if test.resLen != len(res.prs) { if test.reviewSetFetcher != nil {
t.Error("expected result len", test.resLen, "but got", len(res.prs)) res, err = test.reviewSetFetcher(mock)
} } else {
res, err = FetchReviewSet(mock, "test", "repo", 42, &baseConfig)
pr, err := res.GetPrjGitPR() }
if test.prjGitPRIndex < 0 {
if err == nil { if err == nil {
t.Error("expected error, but nothing") if test_err != nil {
t.Fatal("Expected", test_err, "but got", err)
}
} else {
if res != nil {
t.Fatal("error but got ReviewSet?")
}
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
} }
}
pr_found := false t.Log(res)
if test.prjGitPRIndex >= 0 {
for i := range test.data { if test.resLen != len(res.prs) {
if pr == test.data[i].pr && i == test.prjGitPRIndex { t.Error("expected result len", test.resLen, "but got", len(res.prs))
t.Log("found at index", i) }
pr_found = true
PrjGitPR, err := res.GetPrjGitPR()
if test.prjGitPRIndex < 0 {
if err == nil {
t.Error("expected error, but nothing")
} }
} }
if !pr_found { pr_found := false
t.Error("Cannot find expected PrjGit location in PrjGit set", pr) if test.prjGitPRIndex >= 0 {
for i := range test.data {
if PrjGitPR == test.data[i].pr && i == test.prjGitPRIndex {
t.Log("found at index", i)
pr_found = true
}
}
if !pr_found {
t.Error("Cannot find expected PrjGit location in PR set", PrjGitPR)
}
} else {
if PrjGitPR != nil {
t.Log("Expected prjgit not found, but found?", PrjGitPR)
}
} }
} else {
if pr != nil {
t.Log("Expected prjgit not found, but found?", pr)
}
}
/* if res.IsConsistent() != test.consistentSet {
if res.IsReviewed() != test.reviewed { t.Error("IsConsistent() returned unexpected:", test.consistentSet)
t.Error("expected reviewed to be NOT", res.IsReviewed())
} }
*/
}) /*
} if res.IsReviewed() != test.reviewed {
t.Error("expected reviewed to be NOT", res.IsReviewed())
}
*/
})
}
})
} }