Skip to content

Commit a4545a8

Browse files
authored
Make the org comparison case-insensitive too (#122)
* Make the org comparision case-insensitivie too * Add integration test coverage
1 parent 6bf04a9 commit a4545a8

12 files changed

Lines changed: 177 additions & 73 deletions

File tree

.goreleaser.yml

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,15 @@ builds:
2222
post: upx -9 "{{ .Path }}"
2323

2424
archives:
25-
- format: binary
26-
name_template: "{{ .Binary }}_{{ .Os }}_{{ .Arch }}{{ if .Arm }}v{{ .Arm }}{{ end }}{{ if .Mips }}_{{ .Mips }}{{ end }}"
25+
- replacements:
26+
darwin: Darwin
27+
linux: Linux
28+
windows: Windows
29+
386: i386
30+
amd64: x86_64
31+
format_overrides:
32+
- goos: windows
33+
format: zip
2734

2835
dockers:
2936
- dockerfile: Dockerfile

internal/check/valid_owner.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,21 +186,21 @@ func (v *ValidOwner) validateTeam(ctx context.Context, name string) *validateErr
186186
}
187187
}
188188

189-
// GitHub normalizes name before comparison
190-
name = strings.ToLower(name)
191189
// called after validation it's safe to work on `parts` slice
192190
parts := strings.SplitN(name, "/", 2)
193191
org := parts[0]
194192
org = strings.TrimPrefix(org, "@")
195193
team := parts[1]
196194

197-
if org != v.orgName {
195+
// GitHub normalizes name before comparison
196+
if !strings.EqualFold(org, v.orgName) {
198197
return newValidateError("Team %q does not belong to %q organization.", name, v.orgName)
199198
}
200199

201200
teamExists := func() bool {
202201
for _, v := range v.orgTeams {
203-
if v.GetSlug() == team {
202+
// GitHub normalizes name before comparison
203+
if strings.EqualFold(v.GetSlug(), team) {
204204
return true
205205
}
206206
}

tests/integration/helpers_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
//go:build integration
12
// +build integration
23

34
package integration
@@ -105,3 +106,10 @@ func (s *Executor) Binary(binaryPath string) *Executor {
105106
s.binaryPath = binaryPath
106107
return s
107108
}
109+
110+
func stringDefault(in, def string) string {
111+
if in == "" {
112+
return def
113+
}
114+
return in
115+
}

tests/integration/integration_test.go

Lines changed: 140 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package integration
55

66
import (
7+
"fmt"
78
"os"
89
"runtime"
910
"testing"
@@ -15,10 +16,25 @@ import (
1516
)
1617

1718
const (
18-
binaryPathEnvName = "BINARY_PATH"
19-
codeownersSamplesRepo = "https://github.com/gh-codeowners/codeowners-samples.git"
19+
binaryPathEnvName = "BINARY_PATH"
20+
codeownersSamplesRepo = "https://github.com/gh-codeowners/codeowners-samples.git"
21+
caseInsensitiveOrgCodeownersSamplesRepo = "https://github.com/GitHubCODEOWNERS/codeowners-samples.git"
2022
)
2123

24+
var repositories = []struct {
25+
name string
26+
repo string
27+
}{
28+
{
29+
name: "gh-codeowners",
30+
repo: codeownersSamplesRepo,
31+
},
32+
{
33+
name: "GitHubCODEOWNERS",
34+
repo: caseInsensitiveOrgCodeownersSamplesRepo,
35+
},
36+
}
37+
2238
// TestCheckHappyPath tests that codeowners-validator reports no issues for valid CODEOWNERS file.
2339
//
2440
// This test is based on golden file.
@@ -28,74 +44,131 @@ const (
2844
// To update golden file, run:
2945
// UPDATE_GOLDEN=true make test-integration
3046
func TestCheckSuccess(t *testing.T) {
31-
type Envs map[string]string
32-
tests := []struct {
33-
name string
34-
envs Envs
35-
skipOS string
36-
}{
37-
{
38-
name: "files",
39-
envs: Envs{
40-
"CHECKS": "files",
41-
},
42-
},
43-
{
44-
name: "owners",
45-
envs: Envs{
46-
"CHECKS": "owners",
47-
"OWNER_CHECKER_REPOSITORY": "gh-codeowners/codeowners-samples",
48-
"GITHUB_ACCESS_TOKEN": os.Getenv("GITHUB_TOKEN"),
49-
},
50-
},
51-
{
52-
name: "duppatterns",
53-
envs: Envs{
54-
"CHECKS": "duppatterns",
55-
},
56-
},
57-
{
58-
name: "notowned",
59-
envs: Envs{
60-
"PATH": os.Getenv("PATH"), // need to be set to find the `git` binary
61-
"CHECKS": "disable-all",
62-
"EXPERIMENTAL_CHECKS": "notowned",
63-
},
64-
skipOS: "windows",
65-
},
66-
}
67-
for _, tc := range tests {
68-
t.Run(tc.name, func(t *testing.T) {
69-
if runtime.GOOS == tc.skipOS {
70-
t.Skip("this test is marked as skipped for this OS")
71-
}
72-
47+
type (
48+
Envs map[string]string
49+
testCase []struct {
50+
name string
51+
repo string
52+
envs Envs
53+
skipOS string
54+
}
55+
)
56+
57+
t.Run("offline checks", func(t *testing.T) {
58+
for _, repoTC := range repositories {
7359
// given
74-
repoDir, cleanup := CloneRepo(t, codeownersSamplesRepo, "happy-path")
75-
defer cleanup()
76-
77-
binaryPath := os.Getenv(binaryPathEnvName)
78-
codeownersCmd := Exec().
79-
Binary(binaryPath).
80-
// codeowners-validator basic config
81-
WithEnv("REPOSITORY_PATH", repoDir)
82-
83-
for k, v := range tc.envs {
84-
codeownersCmd.WithEnv(k, v)
60+
repoDir, cleanup := CloneRepo(t, repoTC.repo, "happy-path")
61+
62+
tests := testCase{
63+
{
64+
name: "files",
65+
envs: Envs{
66+
"CHECKS": "files",
67+
},
68+
},
69+
{
70+
name: "duppatterns",
71+
envs: Envs{
72+
"CHECKS": "duppatterns",
73+
},
74+
},
75+
{
76+
name: "notowned",
77+
envs: Envs{
78+
"PATH": os.Getenv("PATH"), // need to be set to find the `git` binary
79+
"CHECKS": "disable-all",
80+
"EXPERIMENTAL_CHECKS": "notowned",
81+
},
82+
skipOS: "windows",
83+
},
84+
}
85+
for _, tc := range tests {
86+
t.Run(fmt.Sprintf("%s/%s", repoTC.name, tc.name), func(t *testing.T) {
87+
if runtime.GOOS == tc.skipOS {
88+
t.Skip("this test is marked as skipped for this OS")
89+
}
90+
91+
binaryPath := os.Getenv(binaryPathEnvName)
92+
codeownersCmd := Exec().
93+
Binary(binaryPath).
94+
// codeowners-validator basic config
95+
WithEnv("REPOSITORY_PATH", repoDir)
96+
97+
for k, v := range tc.envs {
98+
codeownersCmd.WithEnv(k, v)
99+
}
100+
101+
// when
102+
result, err := codeownersCmd.AwaitResultAtMost(3 * time.Minute)
103+
104+
// then
105+
require.NoError(t, err)
106+
assert.Equal(t, 0, result.ExitCode)
107+
normalizedOutput := normalizeTimeDurations(result.Stdout)
108+
109+
g := goldie.New(t, goldie.WithNameSuffix(".golden.txt"))
110+
g.Assert(t, t.Name(), []byte(normalizedOutput))
111+
})
85112
}
86113

87-
// when
88-
result, err := codeownersCmd.AwaitResultAtMost(3 * time.Minute)
89-
90-
// then
91-
require.NoError(t, err)
92-
assert.Equal(t, 0, result.ExitCode)
93-
normalizedOutput := normalizeTimeDurations(result.Stdout)
94-
95-
g := goldie.New(t, goldie.WithNameSuffix(".golden.txt"))
96-
g.Assert(t, t.Name(), []byte(normalizedOutput))
97-
})
98-
}
114+
cleanup()
115+
}
116+
})
117+
118+
t.Run("online checks", func(t *testing.T) {
119+
tests := testCase{
120+
{
121+
name: "gh-codeowners/owners",
122+
envs: Envs{
123+
"CHECKS": "owners",
124+
"OWNER_CHECKER_REPOSITORY": "gh-codeowners/codeowners-samples",
125+
"GITHUB_ACCESS_TOKEN": os.Getenv("GITHUB_TOKEN"),
126+
},
127+
repo: codeownersSamplesRepo,
128+
},
129+
{
130+
name: "GitHubCODEOWNERS/owners",
131+
envs: Envs{
132+
"CHECKS": "owners",
133+
"OWNER_CHECKER_REPOSITORY": "GitHubCODEOWNERS/codeowners-samples",
134+
"GITHUB_ACCESS_TOKEN": os.Getenv("GITHUB_TOKEN"),
135+
},
136+
repo: caseInsensitiveOrgCodeownersSamplesRepo,
137+
},
138+
}
139+
for _, tc := range tests {
140+
t.Run(tc.name, func(t *testing.T) {
141+
// given
142+
repoDir, cleanup := CloneRepo(t, tc.repo, "happy-path")
143+
defer cleanup()
144+
145+
if runtime.GOOS == tc.skipOS {
146+
t.Skip("this test is marked as skipped for this OS")
147+
}
148+
149+
binaryPath := os.Getenv(binaryPathEnvName)
150+
codeownersCmd := Exec().
151+
Binary(binaryPath).
152+
// codeowners-validator basic config
153+
WithEnv("REPOSITORY_PATH", repoDir)
154+
155+
for k, v := range tc.envs {
156+
codeownersCmd.WithEnv(k, v)
157+
}
158+
159+
// when
160+
result, err := codeownersCmd.AwaitResultAtMost(3 * time.Minute)
161+
162+
// then
163+
require.NoError(t, err)
164+
assert.Equal(t, 0, result.ExitCode)
165+
normalizedOutput := normalizeTimeDurations(result.Stdout)
166+
167+
g := goldie.New(t, goldie.WithNameSuffix(".golden.txt"))
168+
g.Assert(t, t.Name(), []byte(normalizedOutput))
169+
})
170+
}
171+
})
99172
}
100173

101174
// TestCheckFailures tests that codeowners-validator reports issues for not valid CODEOWNERS file.

tests/integration/testdata/TestCheckSuccess/duppatterns.golden.txt renamed to tests/integration/testdata/TestCheckSuccess/offline_checks/GitHubCODEOWNERS/duppatterns.golden.txt

File renamed without changes.

tests/integration/testdata/TestCheckSuccess/files.golden.txt renamed to tests/integration/testdata/TestCheckSuccess/offline_checks/GitHubCODEOWNERS/files.golden.txt

File renamed without changes.

tests/integration/testdata/TestCheckSuccess/notowned.golden.txt renamed to tests/integration/testdata/TestCheckSuccess/offline_checks/GitHubCODEOWNERS/notowned.golden.txt

File renamed without changes.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
==> Executing Duplicated Pattern Checker (<duration>)
2+
Check OK
3+
4+
1 check(s) executed, no failure(s)
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
==> Executing File Exist Checker (<duration>)
2+
Check OK
3+
4+
1 check(s) executed, no failure(s)
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
==> Executing [Experimental] Not Owned File Checker (<duration>)
2+
Check OK
3+
4+
1 check(s) executed, no failure(s)

0 commit comments

Comments
 (0)