Skip to content

Commit 92fe3b3

Browse files
GrantBirkiGrantBirki
committed
fixes and tests
1 parent e9e44f0 commit 92fe3b3

File tree

4 files changed

+555
-5
lines changed

4 files changed

+555
-5
lines changed

‎internal/cmd/inputs.go

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package cmd
22

3-
import "errors"
3+
import (
4+
"errors"
5+
"fmt"
6+
)
47

58
// validateInputs checks if the provided inputs are valid
69
func ValidateInputs(args []string) error {
@@ -9,14 +12,43 @@ func ValidateInputs(args []string) error {
912
return errors.New("--ignore-label(s) and --label(s) cannot have the same value")
1013
}
1114

15+
// Check for conflicts between ignoreLabels and selectLabel
16+
if selectLabel != "" && len(ignoreLabels) > 0 {
17+
for _, ignoreL := range ignoreLabels {
18+
if ignoreL == selectLabel {
19+
return fmt.Errorf("--ignore-labels contains %q which conflicts with --label %q", ignoreL, selectLabel)
20+
}
21+
}
22+
}
23+
24+
// Check for conflicts between ignoreLabel and selectLabels
25+
if ignoreLabel != "" && len(selectLabels) > 0 {
26+
for _, selectL := range selectLabels {
27+
if selectL == ignoreLabel {
28+
return fmt.Errorf("--label(s) contains %q which conflicts with --ignore-label %q", selectL, ignoreLabel)
29+
}
30+
}
31+
}
32+
33+
// Check for conflicts between ignoreLabels and selectLabels
34+
if len(ignoreLabels) > 0 && len(selectLabels) > 0 {
35+
for _, ignoreL := range ignoreLabels {
36+
for _, selectL := range selectLabels {
37+
if ignoreL == selectL {
38+
return fmt.Errorf("--ignore-labels contains %q which conflicts with --labels containing the same value", ignoreL)
39+
}
40+
}
41+
}
42+
}
43+
1244
// If no args and no file, we can't proceed
1345
if len(args) == 0 && reposFile == "" {
14-
return errors.New("must specify repositories or provide a file with --file")
46+
return errors.New("must specify repositories or provide a file containing a list of repositories with --file")
1547
}
1648

1749
// Warn if no filtering options are provided at all
1850
if branchPrefix == "" && branchSuffix == "" && branchRegex == "" &&
19-
ignoreLabel == "" && selectLabel == "" && len(selectLabels) == 0 &&
51+
ignoreLabel == "" && len(ignoreLabels) == 0 && selectLabel == "" && len(selectLabels) == 0 &&
2052
!requireCI && !mustBeApproved {
2153
Logger.Warn("No filtering options specified. This will attempt to combine ALL open pull requests.")
2254
}

‎internal/cmd/inputs_test.go

Lines changed: 295 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,295 @@
1+
package cmd
2+
3+
import (
4+
"bytes"
5+
"log/slog"
6+
"os"
7+
"strings"
8+
"testing"
9+
)
10+
11+
// mockLogger creates a test logger that writes to a bytes.Buffer
12+
func setupMockLogger() (*bytes.Buffer, func()) {
13+
var buf bytes.Buffer
14+
origLogger := Logger
15+
Logger = slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug}))
16+
17+
return &buf, func() {
18+
Logger = origLogger
19+
}
20+
}
21+
22+
func TestValidateInputs_NoReposSpecified(t *testing.T) {
23+
// Save original values
24+
origReposFile := reposFile
25+
defer func() { reposFile = origReposFile }()
26+
27+
// Set up test values
28+
reposFile = ""
29+
30+
// Run test
31+
err := ValidateInputs([]string{})
32+
33+
// Verify results
34+
if err == nil {
35+
t.Errorf("Expected error when no repos specified, but got nil")
36+
}
37+
38+
expectedErrMsg := "must specify repositories or provide a file"
39+
if err != nil && !strings.Contains(err.Error(), expectedErrMsg) {
40+
t.Errorf("Expected error message containing %q, got: %q", expectedErrMsg, err.Error())
41+
}
42+
}
43+
44+
func TestValidateInputs_SameIgnoreAndSelectLabel(t *testing.T) {
45+
// Save original values
46+
origIgnoreLabel := ignoreLabel
47+
origSelectLabel := selectLabel
48+
origReposFile := reposFile
49+
defer func() {
50+
ignoreLabel = origIgnoreLabel
51+
selectLabel = origSelectLabel
52+
reposFile = origReposFile
53+
}()
54+
55+
// Set up test values
56+
ignoreLabel = "test-label"
57+
selectLabel = "test-label"
58+
reposFile = "dummy.txt" // To prevent the no-repos error
59+
60+
// Run test
61+
err := ValidateInputs([]string{})
62+
63+
// Verify results
64+
if err == nil {
65+
t.Errorf("Expected error when ignoreLabel and selectLabel have same value, but got nil")
66+
}
67+
68+
expectedErrMsg := "--ignore-label(s) and --label(s) cannot have the same value"
69+
if err != nil && !strings.Contains(err.Error(), expectedErrMsg) {
70+
t.Errorf("Expected error message containing %q, got: %q", expectedErrMsg, err.Error())
71+
}
72+
}
73+
74+
func TestValidateInputs_NoFilteringOptions(t *testing.T) {
75+
// Save original values
76+
origBranchPrefix := branchPrefix
77+
origBranchSuffix := branchSuffix
78+
origBranchRegex := branchRegex
79+
origIgnoreLabel := ignoreLabel
80+
origSelectLabel := selectLabel
81+
origSelectLabels := selectLabels
82+
origRequireCI := requireCI
83+
origMustBeApproved := mustBeApproved
84+
origReposFile := reposFile
85+
defer func() {
86+
branchPrefix = origBranchPrefix
87+
branchSuffix = origBranchSuffix
88+
branchRegex = origBranchRegex
89+
ignoreLabel = origIgnoreLabel
90+
selectLabel = origSelectLabel
91+
selectLabels = origSelectLabels
92+
requireCI = origRequireCI
93+
mustBeApproved = origMustBeApproved
94+
reposFile = origReposFile
95+
}()
96+
97+
// Set up test values - no filtering options
98+
branchPrefix = ""
99+
branchSuffix = ""
100+
branchRegex = ""
101+
ignoreLabel = ""
102+
selectLabel = ""
103+
selectLabels = nil
104+
requireCI = false
105+
mustBeApproved = false
106+
reposFile = "dummy.txt" // Prevent no-repos error
107+
108+
// Set up mock logger
109+
logBuf, cleanup := setupMockLogger()
110+
defer cleanup()
111+
112+
// Run test
113+
err := ValidateInputs([]string{"repo1"})
114+
115+
// Verify results
116+
if err != nil {
117+
t.Errorf("Unexpected error: %v", err)
118+
}
119+
120+
logOutput := logBuf.String()
121+
if !strings.Contains(logOutput, "No filtering options specified") {
122+
t.Errorf("Expected warning about no filtering options, but it wasn't logged. Got: %s", logOutput)
123+
}
124+
}
125+
126+
func TestValidateInputs_WithFilteringOptions(t *testing.T) {
127+
// Save original values and restore after test
128+
origBranchPrefix := branchPrefix
129+
origSelectLabels := selectLabels
130+
origReposFile := reposFile
131+
defer func() {
132+
branchPrefix = origBranchPrefix
133+
selectLabels = origSelectLabels
134+
reposFile = origReposFile
135+
}()
136+
137+
// Set up test values - with filtering options
138+
branchPrefix = "feature/"
139+
selectLabels = []string{"enhancement"}
140+
reposFile = "dummy.txt"
141+
142+
// Set up mock logger
143+
logBuf, cleanup := setupMockLogger()
144+
defer cleanup()
145+
146+
// Run test
147+
err := ValidateInputs([]string{"owner/repo1"})
148+
149+
// Verify results
150+
if err != nil {
151+
t.Errorf("Unexpected error: %v", err)
152+
}
153+
154+
logOutput := logBuf.String()
155+
if strings.Contains(logOutput, "No filtering options specified") {
156+
t.Errorf("Warning about no filtering options was logged, but it shouldn't have been. Got: %s", logOutput)
157+
}
158+
}
159+
160+
func TestValidateInputs_WithIgnoreLabels(t *testing.T) {
161+
// Save original values
162+
origIgnoreLabels := ignoreLabels
163+
origSelectLabel := selectLabel
164+
origReposFile := reposFile
165+
defer func() {
166+
ignoreLabels = origIgnoreLabels
167+
selectLabel = origSelectLabel
168+
reposFile = origReposFile
169+
}()
170+
171+
// Test case where ignoreLabels contains the same value as selectLabel
172+
ignoreLabels = []string{"bug", "enhancement"}
173+
selectLabel = "enhancement"
174+
reposFile = "dummy.txt"
175+
176+
err := ValidateInputs([]string{"repo1"})
177+
178+
if err == nil {
179+
t.Errorf("Expected error when ignoreLabels contains a value from selectLabel, but got nil")
180+
}
181+
}
182+
183+
func TestValidateInputs_WithSelectLabelsAndIgnoreLabels(t *testing.T) {
184+
// Save original values
185+
origIgnoreLabels := ignoreLabels
186+
origSelectLabels := selectLabels
187+
origReposFile := reposFile
188+
defer func() {
189+
ignoreLabels = origIgnoreLabels
190+
selectLabels = origSelectLabels
191+
reposFile = origReposFile
192+
}()
193+
194+
// Test case where ignoreLabels and selectLabels have common values
195+
ignoreLabels = []string{"bug", "enhancement"}
196+
selectLabels = []string{"documentation", "enhancement"}
197+
reposFile = "dummy.txt"
198+
199+
err := ValidateInputs([]string{"repo1"})
200+
201+
if err == nil {
202+
t.Errorf("Expected error when ignoreLabels and selectLabels have common values, but got nil")
203+
}
204+
}
205+
206+
func TestValidateInputs_ValidInputs(t *testing.T) {
207+
// Test with valid inputs
208+
tests := []struct {
209+
name string
210+
args []string
211+
reposFile string
212+
branchPrefix string
213+
ignoreLabel string
214+
selectLabel string
215+
ignoreLabels []string
216+
selectLabels []string
217+
requireCI bool
218+
mustBeApproved bool
219+
}{
220+
{
221+
name: "With args",
222+
args: []string{"owner/repo1", "owner/repo2"},
223+
branchPrefix: "feature/",
224+
},
225+
{
226+
name: "With file",
227+
args: []string{},
228+
reposFile: "repos.txt",
229+
selectLabel: "enhancement",
230+
},
231+
{
232+
name: "With multiple filtering options",
233+
args: []string{"owner/repo"},
234+
branchPrefix: "feature/",
235+
requireCI: true,
236+
mustBeApproved: true,
237+
},
238+
}
239+
240+
for _, tt := range tests {
241+
t.Run(tt.name, func(t *testing.T) {
242+
// Save original values
243+
origReposFile := reposFile
244+
origBranchPrefix := branchPrefix
245+
origIgnoreLabel := ignoreLabel
246+
origSelectLabel := selectLabel
247+
origIgnoreLabels := ignoreLabels
248+
origSelectLabels := selectLabels
249+
origRequireCI := requireCI
250+
origMustBeApproved := mustBeApproved
251+
defer func() {
252+
reposFile = origReposFile
253+
branchPrefix = origBranchPrefix
254+
ignoreLabel = origIgnoreLabel
255+
selectLabel = origSelectLabel
256+
ignoreLabels = origIgnoreLabels
257+
selectLabels = origSelectLabels
258+
requireCI = origRequireCI
259+
mustBeApproved = origMustBeApproved
260+
}()
261+
262+
// Set test values
263+
reposFile = tt.reposFile
264+
branchPrefix = tt.branchPrefix
265+
ignoreLabel = tt.ignoreLabel
266+
selectLabel = tt.selectLabel
267+
ignoreLabels = tt.ignoreLabels
268+
selectLabels = tt.selectLabels
269+
requireCI = tt.requireCI
270+
mustBeApproved = tt.mustBeApproved
271+
272+
// Run test
273+
err := ValidateInputs(tt.args)
274+
275+
// Verify results
276+
if err != nil {
277+
t.Errorf("Expected no error with valid inputs, got: %v", err)
278+
}
279+
})
280+
}
281+
}
282+
283+
// TestMain sets up and tears down the testing environment
284+
func TestMain(m *testing.M) {
285+
// Save original logger
286+
origLogger := Logger
287+
288+
// Run tests
289+
code := m.Run()
290+
291+
// Restore original logger
292+
Logger = origLogger
293+
294+
os.Exit(code)
295+
}

‎internal/cmd/repos.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,13 @@ import (
88

99
// ParseRepositories parses repository names from arguments or file with support for default owner
1010
func ParseRepositories(args []string, reposFile string, defaultOwner string) ([]string, error) {
11-
var repos []string
11+
// Explicitly initialize repos as an empty slice
12+
repos := []string{}
13+
14+
// If both args and reposFile are empty, return an empty slice
15+
if len(args) == 0 && reposFile == "" {
16+
return repos, nil
17+
}
1218

1319
// Parse from command line arguments
1420
if len(args) > 0 {
@@ -36,7 +42,18 @@ func ParseRepositories(args []string, reposFile string, defaultOwner string) ([]
3642

3743
lines := strings.Split(string(fileContent), "\n")
3844
for _, line := range lines {
39-
if trimmedLine := strings.TrimSpace(line); trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") {
45+
// Trim whitespace and ignore comments
46+
trimmedLine := strings.TrimSpace(line)
47+
if trimmedLine == "" || strings.HasPrefix(trimmedLine, "#") {
48+
continue
49+
}
50+
51+
// Remove inline comments
52+
if idx := strings.Index(trimmedLine, "#"); idx != -1 {
53+
trimmedLine = strings.TrimSpace(trimmedLine[:idx])
54+
}
55+
56+
if trimmedLine != "" {
4057
repos = append(repos, applyDefaultOwner(trimmedLine, defaultOwner))
4158
}
4259
}

0 commit comments

Comments
 (0)