Skip to content

Commit

Permalink
Enforce only one CODEOWNERS file (#100)
Browse files Browse the repository at this point in the history
* Enforce only one CODEOWNERS file

* Adjust code before merging to support more verbose err messages

Co-authored-by: Mateusz Szostok <szostok.mateusz@gmail.com>
  • Loading branch information
athtran and mszostok committed Feb 9, 2022
1 parent ac35737 commit add91fe
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 2 deletions.
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ require (
gotest.tools v2.2.0+incompatible
)

require github.com/dustin/go-humanize v1.0.0

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/emirpasic/gods v1.12.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ3
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo=
github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
github.com/emirpasic/gods v1.12.0 h1:QAUIPSaCu4G+POclxeqb3F+WPpdKqFGlw36+yOzGlrg=
github.com/emirpasic/gods v1.12.0/go.mod h1:YfzfFFoVP/catgzJb4IKIqXjX78Ha8FMSDh3ymbK86o=
github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
Expand Down
22 changes: 20 additions & 2 deletions pkg/codeowners/owners.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"path"
"strings"

"github.com/dustin/go-humanize/english"
"github.com/spf13/afero"
)

Expand Down Expand Up @@ -38,6 +39,7 @@ func NewFromPath(path string) ([]Entry, error) {
// openCodeownersFile finds a CODEOWNERS file and returns content.
// see: https://help.github.com/articles/about-code-owners/#codeowners-file-location
func openCodeownersFile(dir string) (io.Reader, error) {
var detectedFiles []string
for _, p := range []string{".", "docs", ".github"} {
pth := path.Join(dir, p)
exists, err := afero.DirExists(fs, pth)
Expand All @@ -59,10 +61,26 @@ func openCodeownersFile(dir string) (io.Reader, error) {
return nil, err
}

return fs.Open(f)
detectedFiles = append(detectedFiles, f)
}

return nil, fmt.Errorf("No CODEOWNERS found in the root, docs/, or .github/ directory of the repository %s", dir)
switch l := len(detectedFiles); l {
case 0:
return nil, fmt.Errorf("No CODEOWNERS found in the root, docs/, or .github/ directory of the repository %s", dir)
case 1:
return fs.Open(detectedFiles[0])
default:
return nil, fmt.Errorf("Multiple CODEOWNERS files found in the %s locations of the repository %s",
english.OxfordWordSeries(replacePrefix(detectedFiles, dir, "./"), "and"),
dir)
}
}

func replacePrefix(in []string, prefix string, s string) []string {
for idx := range in {
in[idx] = fmt.Sprintf("%s%s", s, strings.TrimPrefix(in[idx], prefix))
}
return in
}

func ParseCodeowners(r io.Reader) []Entry {
Expand Down
49 changes: 49 additions & 0 deletions pkg/codeowners/owners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,52 @@ func TestFindCodeownersFileFailure(t *testing.T) {
assert.EqualError(t, err, expErrMsg)
assert.Nil(t, entries)
}

func TestMultipleCodeownersFileFailure(t *testing.T) {
const givenRepoPath = "/workspace/go/repo-without-codeowners/"
tests := map[string]struct {
expErrMsg string
givenCodeownersLocations []string
}{
"Should report that no CODEOWNERS file was found": {
expErrMsg: fmt.Sprintf("No CODEOWNERS found in the root, docs/, or .github/ directory of the repository %s", givenRepoPath),
givenCodeownersLocations: nil,
},
"Should report that CODEOWNERS file was found on root and docs/": {
expErrMsg: fmt.Sprintf("Multiple CODEOWNERS files found in the ./CODEOWNERS and ./docs/CODEOWNERS locations of the repository %s", givenRepoPath),
givenCodeownersLocations: []string{"CODEOWNERS", path.Join("docs", "CODEOWNERS")},
},
"Should report that CODEOWNERS file was found on root and .github/": {
expErrMsg: fmt.Sprintf("Multiple CODEOWNERS files found in the ./CODEOWNERS and ./.github/CODEOWNERS locations of the repository %s", givenRepoPath),
givenCodeownersLocations: []string{"CODEOWNERS", path.Join(".github/", "CODEOWNERS")},
},
"Should report that CODEOWNERS file was found in docs/ and .github/": {
expErrMsg: fmt.Sprintf("Multiple CODEOWNERS files found in the ./docs/CODEOWNERS and ./.github/CODEOWNERS locations of the repository %s", givenRepoPath),
givenCodeownersLocations: []string{path.Join(".github", "CODEOWNERS"), path.Join("docs", "CODEOWNERS")},
},
"Should report that CODEOWNERS file was found on root, docs/ and .github/": {
expErrMsg: fmt.Sprintf("Multiple CODEOWNERS files found in the ./CODEOWNERS, ./docs/CODEOWNERS, and ./.github/CODEOWNERS locations of the repository %s", givenRepoPath),
givenCodeownersLocations: []string{"CODEOWNERS", path.Join(".github", "CODEOWNERS"), path.Join("docs", "CODEOWNERS")},
},
}
for tn, tc := range tests {
t.Run(tn, func(t *testing.T) {
// given
tFS := afero.NewMemMapFs()
revert := codeowners.SetFS(tFS)
defer revert()

for _, location := range tc.givenCodeownersLocations {
_, err := tFS.Create(path.Join(givenRepoPath, location))
require.NoError(t, err)
}

// when
entries, err := codeowners.NewFromPath(givenRepoPath)

// then
assert.EqualError(t, err, tc.expErrMsg)
assert.Nil(t, entries)
})
}
}

0 comments on commit add91fe

Please sign in to comment.