Adam Majer
139f40fce3
All checks were successful
go-generate-check / go-generate-check (push) Successful in 8s
With group expansion, we were not operating on a copy of the slice but the original slice. This results in the original data being modified instead of the copy as intended.
421 lines
12 KiB
Go
421 lines
12 KiB
Go
package common_test
|
|
|
|
import (
|
|
"bytes"
|
|
"errors"
|
|
"slices"
|
|
"testing"
|
|
|
|
"go.uber.org/mock/gomock"
|
|
"src.opensuse.org/autogits/common"
|
|
"src.opensuse.org/autogits/common/gitea-generated/client/repository"
|
|
mock_common "src.opensuse.org/autogits/common/mock"
|
|
)
|
|
|
|
func TestMaintainership(t *testing.T) {
|
|
config := &common.AutogitConfig{
|
|
Branch: "bar",
|
|
Organization: "foo",
|
|
GitProjectName: common.DefaultGitPrj + "#bar",
|
|
}
|
|
|
|
packageTests := []struct {
|
|
name string
|
|
maintainers []string
|
|
otherError bool
|
|
packageName string
|
|
|
|
maintainersFile []byte
|
|
maintainersFileErr error
|
|
|
|
groups []*common.ReviewGroup
|
|
|
|
maintainersDir map[string][]byte
|
|
}{
|
|
/* PACKAGE MAINTAINERS */
|
|
// package tests have packageName, projects do not
|
|
{
|
|
name: "No maintainer in empty package",
|
|
packageName: "foo",
|
|
},
|
|
{
|
|
name: "Error in MaintainerListForPackage when remote has an error",
|
|
maintainersFileErr: errors.New("Some error"), // repository.NewRepoGetRawFileNotFound(),
|
|
packageName: "foo",
|
|
},
|
|
{
|
|
name: "Multiple package maintainers",
|
|
maintainersFile: []byte(`{"pkg": ["user1", "user2"], "": ["user1", "user3"]}`),
|
|
maintainersDir: map[string][]byte{
|
|
"_project": []byte(`{"": ["user1", "user3"]}`),
|
|
"pkg": []byte(`{"pkg": ["user1", "user2"]}`),
|
|
},
|
|
maintainers: []string{"user1", "user2", "user3"},
|
|
packageName: "pkg",
|
|
},
|
|
{
|
|
name: "Multiple package maintainers and groups",
|
|
maintainersFile: []byte(`{"pkg": ["user1", "user2", "g2"], "": ["g2", "user1", "user3"]}`),
|
|
maintainersDir: map[string][]byte{
|
|
"_project": []byte(`{"": ["user1", "user3", "g2"]}`),
|
|
"pkg": []byte(`{"pkg": ["user1", "g2", "user2"]}`),
|
|
},
|
|
maintainers: []string{"user1", "user2", "user3", "user5"},
|
|
packageName: "pkg",
|
|
groups: []*common.ReviewGroup{
|
|
{
|
|
Name: "g2",
|
|
Reviewers: []string{"user1", "user5"},
|
|
},
|
|
},
|
|
},
|
|
{
|
|
name: "No package maintainers and only project maintainer",
|
|
maintainersFile: []byte(`{"pkg2": ["user1", "user2"], "": ["user1", "user3"]}`),
|
|
maintainersDir: map[string][]byte{
|
|
"_project": []byte(`{"": ["user1", "user3"]}`),
|
|
},
|
|
maintainers: []string{"user1", "user3"},
|
|
packageName: "pkg",
|
|
},
|
|
{
|
|
name: "Invalid list of package maintainers",
|
|
maintainersFile: []byte(`{"pkg": 3,"": ["user", 4]}`),
|
|
maintainersDir: map[string][]byte{
|
|
"_project": []byte(`{"": ["user1", 4]}`),
|
|
"pkg": []byte(`"pkg": 3`),
|
|
},
|
|
otherError: true,
|
|
packageName: "pkg",
|
|
},
|
|
|
|
/* PROJECT MAINTAINERS */
|
|
{
|
|
name: "No maintainer for empty project",
|
|
},
|
|
{
|
|
name: "No maintainer for empty project maintainer file",
|
|
maintainersFile: []byte("{}"),
|
|
maintainersDir: map[string][]byte{
|
|
"_project": []byte(`{}`),
|
|
},
|
|
},
|
|
{
|
|
name: "Error in MaintainerListForProject when remote has an error",
|
|
maintainersFileErr: errors.New("some error"), //repository.NewRepoGetRawFileNotFound(),
|
|
},
|
|
{
|
|
name: "Multiple project maintainers",
|
|
maintainersFile: []byte(`{"": ["user1", "user2"]}`),
|
|
maintainersDir: map[string][]byte{
|
|
"_project": []byte(`{"": ["user1", "user2"]}`),
|
|
},
|
|
maintainers: []string{"user1", "user2"},
|
|
},
|
|
{
|
|
name: "Single project maintainer",
|
|
maintainersFile: []byte(`{"": ["user"]}`),
|
|
maintainersDir: map[string][]byte{
|
|
"_project": []byte(`{"": ["user"]}`),
|
|
},
|
|
maintainers: []string{"user"},
|
|
},
|
|
{
|
|
name: "Invalid list of project maintainers",
|
|
maintainersFile: []byte(`{"": ["user", 4]}`),
|
|
maintainersDir: map[string][]byte{
|
|
"_project": []byte(`{"": ["user", 4]}`),
|
|
},
|
|
otherError: true,
|
|
},
|
|
{
|
|
name: "Invalid list of project maintainers",
|
|
maintainersFile: []byte(`{"": 4}`),
|
|
maintainersDir: map[string][]byte{
|
|
"_project": []byte(`{"": 4}`),
|
|
},
|
|
otherError: true,
|
|
},
|
|
}
|
|
|
|
notFoundError := repository.NewRepoGetContentsNotFound()
|
|
for _, test := range packageTests {
|
|
runTests := func(t *testing.T, mi common.GiteaMaintainershipReader) {
|
|
maintainers, err := common.FetchProjectMaintainershipData(mi, config)
|
|
if err != nil && !test.otherError {
|
|
if test.maintainersFileErr == nil {
|
|
t.Fatal("Unexpected error recieved", err)
|
|
} else if err != test.maintainersFileErr {
|
|
t.Error("Wrong error recieved", err)
|
|
}
|
|
} else if test.maintainersFileErr != nil {
|
|
t.Fatal("Expected an error...")
|
|
} else if test.otherError && err == nil {
|
|
t.Fatal("Expected an error...")
|
|
}
|
|
|
|
var m []string
|
|
if len(test.packageName) > 0 {
|
|
m = maintainers.ListPackageMaintainers(test.packageName, test.groups)
|
|
} else {
|
|
m = maintainers.ListProjectMaintainers(test.groups)
|
|
}
|
|
|
|
if len(m) != len(test.maintainers) {
|
|
t.Error("Invalid number of maintainers for package", test.packageName, len(m), "vs", len(test.maintainers))
|
|
}
|
|
for i := range m {
|
|
if !slices.Contains(test.maintainers, m[i]) {
|
|
t.Fatal("Can't find expected users. Found:", m)
|
|
}
|
|
}
|
|
}
|
|
|
|
t.Run(test.name+"_File", func(t *testing.T) {
|
|
ctl := gomock.NewController(t)
|
|
mi := mock_common.NewMockGiteaMaintainershipReader(ctl)
|
|
|
|
// tests with maintainership file
|
|
mi.EXPECT().FetchMaintainershipFile("foo", common.DefaultGitPrj, "bar").
|
|
Return(test.maintainersFile, "", test.maintainersFileErr)
|
|
mi.EXPECT().FetchMaintainershipDirFile("foo", common.DefaultGitPrj, "bar", common.ProjectFileKey).
|
|
Return(nil, "", notFoundError)
|
|
|
|
runTests(t, mi)
|
|
})
|
|
|
|
t.Run(test.name+"_Dir", func(t *testing.T) {
|
|
ctl := gomock.NewController(t)
|
|
mi := mock_common.NewMockGiteaMaintainershipReader(ctl)
|
|
|
|
// run same tests with directory maintainership data
|
|
for filename, data := range test.maintainersDir {
|
|
mi.EXPECT().FetchMaintainershipDirFile("foo", common.DefaultGitPrj, "bar", filename).Return(data, "", test.maintainersFileErr).AnyTimes()
|
|
}
|
|
if _, found := test.maintainersDir[common.ProjectFileKey]; !found {
|
|
mi.EXPECT().FetchMaintainershipDirFile("foo", common.DefaultGitPrj, "bar", common.ProjectFileKey).Return(nil, "", test.maintainersFileErr).AnyTimes()
|
|
mi.EXPECT().FetchMaintainershipFile("foo", common.DefaultGitPrj, "bar").Return(nil, "", test.maintainersFileErr).AnyTimes()
|
|
}
|
|
mi.EXPECT().FetchMaintainershipDirFile("foo", common.DefaultGitPrj, "bar", gomock.Any()).Return(nil, "", notFoundError).AnyTimes()
|
|
|
|
runTests(t, mi)
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestMaintainershipFileWrite(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
is_dir bool
|
|
maintainers map[string][]string
|
|
raw []byte
|
|
expected_output string
|
|
expected_error error
|
|
}{
|
|
{
|
|
name: "empty dataset",
|
|
expected_output: "{\n}\n",
|
|
},
|
|
{
|
|
name: "2 project maintainers only",
|
|
maintainers: map[string][]string{
|
|
"": {"two", "one"},
|
|
},
|
|
expected_output: "{\n \"\": [\"one\",\"two\"]\n}\n",
|
|
},
|
|
{
|
|
name: "2 project maintainers and 2 single package maintainers",
|
|
maintainers: map[string][]string{
|
|
"": {"two", "one"},
|
|
"pkg1": {},
|
|
"foo": {"four", "byte"},
|
|
},
|
|
expected_output: "{\n \"\": [\"one\",\"two\"],\n \"foo\": [\"byte\",\"four\"],\n \"pkg1\": []\n}\n",
|
|
},
|
|
{
|
|
name: "surgical modification",
|
|
maintainers: map[string][]string{
|
|
"": {"one", "two"},
|
|
"foo": {"byte", "four", "newone"},
|
|
"pkg1": {},
|
|
},
|
|
raw: []byte("{\n \"\": [\"one\",\"two\"],\n \"foo\": [\"byte\",\"four\"],\n \"pkg1\": []\n}\n"),
|
|
expected_output: "{\n \"\": [\"one\",\"two\"],\n \"foo\": [\"byte\",\"four\",\"newone\"],\n \"pkg1\": []\n}\n",
|
|
},
|
|
{
|
|
name: "no change",
|
|
maintainers: map[string][]string{
|
|
"": {"one", "two"},
|
|
"foo": {"byte", "four"},
|
|
"pkg1": {},
|
|
},
|
|
raw: []byte("{\n \"\": [\"one\",\"two\"],\n \"foo\": [\"byte\",\"four\"],\n \"pkg1\": []\n}\n"),
|
|
expected_output: "{\n \"\": [\"one\",\"two\"],\n \"foo\": [\"byte\",\"four\"],\n \"pkg1\": []\n}\n",
|
|
},
|
|
{
|
|
name: "surgical addition",
|
|
maintainers: map[string][]string{
|
|
"": {"one"},
|
|
"new": {"user"},
|
|
},
|
|
raw: []byte("{\n \"\": [ \"one\" ]\n}\n"),
|
|
expected_output: "{\n \"\": [ \"one\" ],\n \"new\": [\"user\"]\n}\n",
|
|
},
|
|
{
|
|
name: "surgical deletion",
|
|
maintainers: map[string][]string{
|
|
"": {"one"},
|
|
},
|
|
raw: []byte("{\n \"\": [\"one\"],\n \"old\": [\"user\"]\n}\n"),
|
|
expected_output: "{\n \"\": [\"one\"]\n}\n",
|
|
},
|
|
}
|
|
|
|
for _, test := range tests {
|
|
t.Run(test.name, func(t *testing.T) {
|
|
b := bytes.Buffer{}
|
|
data := common.MaintainershipMap{
|
|
Data: test.maintainers,
|
|
IsDir: test.is_dir,
|
|
Raw: test.raw,
|
|
}
|
|
|
|
if err := data.WriteMaintainershipFile(&b); err != test.expected_error {
|
|
t.Fatal("unexpected error:", err, "Expecting:", test.expected_error)
|
|
}
|
|
|
|
output := b.String()
|
|
|
|
if test.expected_output != output {
|
|
t.Fatalf("unexpected output:\n%q\nExpecting:\n%q", output, test.expected_output)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestReviewRequired(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
maintainers []string
|
|
config *common.AutogitConfig
|
|
is_approved bool
|
|
}{
|
|
{
|
|
name: "ReviewRequired=false",
|
|
maintainers: []string{"maintainer1", "maintainer2"},
|
|
config: &common.AutogitConfig{ReviewRequired: false},
|
|
is_approved: true,
|
|
},
|
|
{
|
|
name: "ReviewRequired=true",
|
|
maintainers: []string{"maintainer1", "maintainer2"},
|
|
config: &common.AutogitConfig{ReviewRequired: true},
|
|
is_approved: false,
|
|
},
|
|
{
|
|
name: "ReviewRequired=true",
|
|
maintainers: []string{"maintainer1"},
|
|
config: &common.AutogitConfig{ReviewRequired: true},
|
|
is_approved: true,
|
|
},
|
|
}
|
|
|
|
for _, test := range tests {
|
|
t.Run(test.name, func(t *testing.T) {
|
|
m := &common.MaintainershipMap{
|
|
Data: map[string][]string{"": test.maintainers},
|
|
}
|
|
m.Config = test.config
|
|
if approved := m.IsApproved("", nil, "maintainer1", nil); approved != test.is_approved {
|
|
t.Error("Expected m.IsApproved()->", test.is_approved, "but didn't get it")
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestMaintainershipDataCorruption_PackageAppend(t *testing.T) {
|
|
// Test corruption when append happens (merging project maintainers)
|
|
// If backing array has capacity, append writes to it.
|
|
|
|
// We construct a slice with capacity > len to simulate this common scenario
|
|
backingArray := make([]string, 1, 10)
|
|
backingArray[0] = "@g1"
|
|
|
|
initialData := map[string][]string{
|
|
"pkg": backingArray, // len 1, cap 10
|
|
"": {"prjUser"},
|
|
}
|
|
|
|
m := &common.MaintainershipMap{
|
|
Data: initialData,
|
|
}
|
|
|
|
groups := []*common.ReviewGroup{
|
|
{
|
|
Name: "@g1",
|
|
Reviewers: []string{"u1"},
|
|
},
|
|
}
|
|
|
|
// ListPackageMaintainers("pkg", groups)
|
|
// 1. gets ["@g1"] (cap 10)
|
|
// 2. Appends "prjUser" -> ["@g1", "prjUser"] (in backing array)
|
|
// 3. Expands "@g1" -> "u1".
|
|
// Replace: ["u1", "prjUser"]
|
|
// Sort: ["prjUser", "u1"]
|
|
//
|
|
// The backing array is now ["prjUser", "u1", ...]
|
|
// The map entry "pkg" is still len 1.
|
|
// So it sees ["prjUser"].
|
|
|
|
list1 := m.ListPackageMaintainers("pkg", groups)
|
|
t.Logf("List1: %v", list1)
|
|
|
|
// ListPackageMaintainers("pkg", nil)
|
|
// Should be ["@g1", "prjUser"] (because prjUser is appended from project maintainers)
|
|
// But since backing array is corrupted:
|
|
// It sees ["prjUser"] (from map) + appends "prjUser" -> ["prjUser", "prjUser"].
|
|
|
|
list2 := m.ListPackageMaintainers("pkg", nil)
|
|
t.Logf("List2: %v", list2)
|
|
|
|
if !slices.Contains(list2, "@g1") {
|
|
t.Errorf("Corruption: '@g1' is missing from second call. Got %v", list2)
|
|
}
|
|
}
|
|
|
|
func TestMaintainershipDataCorruption_ProjectInPlace(t *testing.T) {
|
|
// Test corruption in ListProjectMaintainers when replacement fits in place
|
|
// e.g. replacing 1 group with 1 user.
|
|
|
|
initialData := map[string][]string{
|
|
"": {"@g1"},
|
|
}
|
|
|
|
m := &common.MaintainershipMap{
|
|
Data: initialData,
|
|
}
|
|
|
|
groups := []*common.ReviewGroup{
|
|
{
|
|
Name: "@g1",
|
|
Reviewers: []string{"u1"},
|
|
},
|
|
}
|
|
|
|
// First call with expansion
|
|
// Replaces "@g1" with "u1". Length stays 1. Modifies backing array in place.
|
|
list1 := m.ListProjectMaintainers(groups)
|
|
t.Logf("List1: %v", list1)
|
|
|
|
// Second call without expansion
|
|
// Should return ["@g1"]
|
|
list2 := m.ListProjectMaintainers(nil)
|
|
t.Logf("List2: %v", list2)
|
|
|
|
if !slices.Contains(list2, "@g1") {
|
|
t.Errorf("Corruption: '@g1' is missing from second call (Project). Got %v", list2)
|
|
}
|
|
}
|
|
|