Skip to content

Commit 17559cf

Browse files
committed
Chore: address feedback from CR
- renamed AllowedBranches to AllowedRefs - use t.Run() in tests
1 parent 1d2f53d commit 17559cf

File tree

2 files changed

+35
-38
lines changed

2 files changed

+35
-38
lines changed

receiver.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ import (
1515
const ZeroSHA = "0000000000000000000000000000000000000000"
1616

1717
type Receiver struct {
18-
Debug bool
19-
MasterOnly bool
20-
AllowedBranches []string
21-
TmpDir string
22-
HandlerFunc func(*HookInfo, string) error
18+
Debug bool
19+
MasterOnly bool
20+
AllowedRefs []string
21+
TmpDir string
22+
HandlerFunc func(*HookInfo, string) error
2323
}
2424

2525
func ReadCommitMessage(sha string) (string, error) {
@@ -49,15 +49,15 @@ func IsForcePush(hook *HookInfo) (bool, error) {
4949

5050
func (r *Receiver) CheckAllowedBranch(hook *HookInfo) error {
5151
if r.MasterOnly { // for BC
52-
r.AllowedBranches = append(r.AllowedBranches, "refs/heads/master")
52+
r.AllowedRefs = append(r.AllowedRefs, "refs/heads/master")
5353
}
5454

55-
if len(r.AllowedBranches) == 0 {
55+
if len(r.AllowedRefs) == 0 {
5656
return nil
5757
}
5858

59-
if !slices.Contains(r.AllowedBranches, hook.Ref) {
60-
return fmt.Errorf("cannot push branch, allowed branches: %s", strings.Join(r.AllowedBranches, ", "))
59+
if !slices.Contains(r.AllowedRefs, hook.Ref) {
60+
return fmt.Errorf("cannot push branch, allowed branches: %s", strings.Join(r.AllowedRefs, ", "))
6161
}
6262

6363
return nil

receiver_test.go

Lines changed: 26 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package gitkit_test
22

33
import (
4+
"fmt"
45
"testing"
56

67
"github.com/sosedoff/gitkit"
@@ -12,7 +13,7 @@ type gitReceiveMock struct {
1213
masterOnly bool
1314
allowedBranches []string
1415
ref string
15-
isErr bool
16+
err error
1617
}
1718

1819
func TestMasterOnly(t *testing.T) {
@@ -21,30 +22,28 @@ func TestMasterOnly(t *testing.T) {
2122
name: "push to master, no error",
2223
masterOnly: true,
2324
ref: "refs/heads/master",
24-
isErr: false,
25+
err: nil,
2526
},
2627
{
2728
name: "push to a branch, should trigger error",
2829
masterOnly: true,
2930
ref: "refs/heads/branch",
30-
isErr: true,
31+
err: fmt.Errorf("cannot push branch, allowed branches: refs/heads/master"),
3132
},
3233
}
3334

3435
for _, tc := range testCases {
35-
r := &gitkit.Receiver{
36-
MasterOnly: tc.masterOnly,
37-
}
36+
t.Run(tc.name, func(t *testing.T) {
37+
r := &gitkit.Receiver{
38+
MasterOnly: tc.masterOnly,
39+
}
3840

39-
err := r.CheckAllowedBranch(&gitkit.HookInfo{
40-
Ref: tc.ref,
41-
})
41+
err := r.CheckAllowedBranch(&gitkit.HookInfo{
42+
Ref: tc.ref,
43+
})
4244

43-
if !tc.isErr {
44-
assert.NoError(t, err, "expected no error: %s", tc.name)
45-
} else {
46-
assert.Error(t, err, "expected an error: %s", tc.name)
47-
}
45+
assert.Equal(t, tc.err, err)
46+
})
4847
}
4948
}
5049

@@ -54,41 +53,39 @@ func TestAllowedBranches(t *testing.T) {
5453
name: "push to master, no error",
5554
allowedBranches: []string{"refs/heads/master"},
5655
ref: "refs/heads/master",
57-
isErr: false,
56+
err: nil,
5857
},
5958
{
6059
name: "push to a branch, should trigger error",
6160
allowedBranches: []string{"refs/heads/master"},
6261
ref: "refs/heads/some-branch",
63-
isErr: true,
62+
err: fmt.Errorf("cannot push branch, allowed branches: refs/heads/master"),
6463
},
6564
{
6665
name: "push to another-branch",
6766
allowedBranches: []string{"refs/heads/another-branch"},
6867
ref: "refs/heads/another-branch",
69-
isErr: false,
68+
err: nil,
7069
},
7170
{
7271
name: "push to main and only allow main",
7372
allowedBranches: []string{"refs/heads/main"},
7473
ref: "refs/heads/main",
75-
isErr: false,
74+
err: nil,
7675
},
7776
}
7877

7978
for _, tc := range testCases {
80-
r := &gitkit.Receiver{
81-
AllowedBranches: tc.allowedBranches,
82-
}
79+
t.Run(tc.name, func(t *testing.T) {
80+
r := &gitkit.Receiver{
81+
AllowedRefs: tc.allowedBranches,
82+
}
8383

84-
err := r.CheckAllowedBranch(&gitkit.HookInfo{
85-
Ref: tc.ref,
86-
})
84+
err := r.CheckAllowedBranch(&gitkit.HookInfo{
85+
Ref: tc.ref,
86+
})
8787

88-
if !tc.isErr {
89-
assert.NoError(t, err, "expected no error: %s", tc.name)
90-
} else {
91-
assert.Error(t, err, "expected an error: %s", tc.name)
92-
}
88+
assert.Equal(t, tc.err, err)
89+
})
9390
}
9491
}

0 commit comments

Comments
 (0)