Skip to content

Commit ba09dcc

Browse files
behinddwallsbehinddwalls
committed
feat(mergechecker): add GitHub MergeChecker implementation with multi-host routing
1 parent 8e4e657 commit ba09dcc

22 files changed

Lines changed: 1467 additions & 21 deletions

File tree

‎CLAUDE.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ make clean # Clean Bazel cache
186186

187187
### Testing
188188

189+
- **Table-driven tests** — prefer table-driven tests with `t.Run` subtests over individual test functions.
189190
- **Avoid asserting on error messages** — assert on error type or generic error.
190191
- **No change detector tests** — don't assert on default values, internal structure, or implementation details that can change without affecting behavior. Test what the code *does*, not how it's constructed.
191192
- **No `time.Sleep` for synchronization** — use channels, callbacks, condition variables.

‎entity/github/BUILD.bazel‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
load("@rules_go//go:def.bzl", "go_library", "go_test")
2+
3+
go_library(
4+
name = "github",
5+
srcs = ["change_id.go"],
6+
importpath = "github.com/uber/submitqueue/entity/github",
7+
visibility = ["//visibility:public"],
8+
)
9+
10+
go_test(
11+
name = "github_test",
12+
srcs = ["change_id_test.go"],
13+
embed = [":github"],
14+
deps = [
15+
"@com_github_stretchr_testify//assert",
16+
"@com_github_stretchr_testify//require",
17+
],
18+
)

‎entity/github/change_id.go‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package github
2+
3+
import (
4+
"fmt"
5+
"strconv"
6+
"strings"
7+
)
8+
9+
// changeIDFormat is the expected format for change IDs, included in error messages.
10+
const changeIDFormat = "{scheme}://{owner}/{repo}/{pr_number}/{head_commit_sha}"
11+
12+
// ChangeID represents a parsed GitHub-family change identifier.
13+
// Covers GitHub.com, GitHub Enterprise (GHE), and GitHub Enterprise Server (GHES)
14+
// since they share the same pull request model.
15+
// Format: {scheme}://{owner}/{repo}/{pr_number}/{head_commit_sha}
16+
type ChangeID struct {
17+
// Scheme captures the source variant (e.g., "github", "ghe", "ghes").
18+
Scheme string
19+
// Org is the organization or owner of the repository.
20+
Org string
21+
// Repo is the repository name.
22+
Repo string
23+
// PRNumber is the pull request number.
24+
PRNumber int
25+
// HeadCommitSHA is the head commit SHA at the time of request creation.
26+
HeadCommitSHA string
27+
}
28+
29+
// ParseChangeID parses a raw change ID string into a ChangeID.
30+
// Expected format: {scheme}://{owner}/{repo}/{pr_number}/{head_commit_sha}
31+
// The parser works from the end: SHA (last), PR number (second-to-last),
32+
// and everything before is the repo path (split into owner and repo).
33+
func ParseChangeID(raw string) (ChangeID, error) {
34+
// Split on "://" to get scheme and path
35+
schemeSplit := strings.SplitN(raw, "://", 2)
36+
if len(schemeSplit) != 2 {
37+
return ChangeID{}, fmt.Errorf("invalid change ID %q: missing '://' separator (expected format: %s)", raw, changeIDFormat)
38+
}
39+
40+
scheme := schemeSplit[0]
41+
if scheme == "" {
42+
return ChangeID{}, fmt.Errorf("invalid change ID %q: empty scheme (expected format: %s)", raw, changeIDFormat)
43+
}
44+
45+
path := schemeSplit[1]
46+
47+
// Split the path into segments and parse from the end.
48+
segments := strings.Split(path, "/")
49+
// Need at least 4 segments: {owner}/{repo}/{pr_number}/{sha}
50+
if len(segments) < 4 {
51+
return ChangeID{}, fmt.Errorf("invalid change ID %q: need at least owner/repo/pr/sha, got %d segments (expected format: %s)", raw, len(segments), changeIDFormat)
52+
}
53+
54+
sha := segments[len(segments)-1]
55+
prStr := segments[len(segments)-2]
56+
repoSegments := segments[:len(segments)-2]
57+
58+
if sha == "" {
59+
return ChangeID{}, fmt.Errorf("invalid change ID %q: empty head commit SHA (expected format: %s)", raw, changeIDFormat)
60+
}
61+
62+
prNumber, err := strconv.Atoi(prStr)
63+
if err != nil {
64+
return ChangeID{}, fmt.Errorf("invalid change ID %q: PR number %q is not a valid integer (expected format: %s)", raw, prStr, changeIDFormat)
65+
}
66+
67+
// Split repo path: last segment is repo name, everything before is the owner.
68+
if len(repoSegments) < 2 {
69+
return ChangeID{}, fmt.Errorf("invalid change ID %q: repo path must have at least owner/repo (expected format: %s)", raw, changeIDFormat)
70+
}
71+
72+
repo := repoSegments[len(repoSegments)-1]
73+
org := strings.Join(repoSegments[:len(repoSegments)-1], "/")
74+
75+
if org == "" {
76+
return ChangeID{}, fmt.Errorf("invalid change ID %q: empty owner (expected format: %s)", raw, changeIDFormat)
77+
}
78+
if repo == "" {
79+
return ChangeID{}, fmt.Errorf("invalid change ID %q: empty repo (expected format: %s)", raw, changeIDFormat)
80+
}
81+
82+
return ChangeID{
83+
Scheme: scheme,
84+
Org: org,
85+
Repo: repo,
86+
PRNumber: prNumber,
87+
HeadCommitSHA: sha,
88+
}, nil
89+
}
90+
91+
// String returns the string representation of the change ID.
92+
func (c ChangeID) String() string {
93+
return fmt.Sprintf("%s://%s/%s/%d/%s", c.Scheme, c.Org, c.Repo, c.PRNumber, c.HeadCommitSHA)
94+
}
95+
96+
// OwnerRepo returns the "{org}/{repo}" string.
97+
func (c ChangeID) OwnerRepo() string {
98+
return fmt.Sprintf("%s/%s", c.Org, c.Repo)
99+
}

‎entity/github/change_id_test.go‎

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
package github
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
func TestParseChangeID(t *testing.T) {
11+
tests := []struct {
12+
name string
13+
raw string
14+
want ChangeID
15+
wantErr bool
16+
}{
17+
{
18+
name: "valid github scheme",
19+
raw: "github://uber/submitqueue/123/abc123def",
20+
want: ChangeID{
21+
Scheme: "github",
22+
Org: "uber",
23+
Repo: "submitqueue",
24+
PRNumber: 123,
25+
HeadCommitSHA: "abc123def",
26+
},
27+
},
28+
{
29+
name: "valid ghe scheme",
30+
raw: "ghe://uber/monorepo/456/deadbeef",
31+
want: ChangeID{
32+
Scheme: "ghe",
33+
Org: "uber",
34+
Repo: "monorepo",
35+
PRNumber: 456,
36+
HeadCommitSHA: "deadbeef",
37+
},
38+
},
39+
{
40+
name: "valid ghes scheme",
41+
raw: "ghes://org/repo/1/sha1",
42+
want: ChangeID{
43+
Scheme: "ghes",
44+
Org: "org",
45+
Repo: "repo",
46+
PRNumber: 1,
47+
HeadCommitSHA: "sha1",
48+
},
49+
},
50+
{
51+
name: "nested org path",
52+
raw: "github://uber/frontend/webapp/42/abc123",
53+
want: ChangeID{
54+
Scheme: "github",
55+
Org: "uber/frontend",
56+
Repo: "webapp",
57+
PRNumber: 42,
58+
HeadCommitSHA: "abc123",
59+
},
60+
},
61+
{
62+
name: "missing separator",
63+
raw: "github/uber/submitqueue/123/abc123",
64+
wantErr: true,
65+
},
66+
{
67+
name: "empty scheme",
68+
raw: "://uber/submitqueue/123/abc123",
69+
wantErr: true,
70+
},
71+
{
72+
name: "too few segments",
73+
raw: "github://uber/123/abc123",
74+
wantErr: true,
75+
},
76+
{
77+
name: "only one segment",
78+
raw: "github://abc123",
79+
wantErr: true,
80+
},
81+
{
82+
name: "empty owner",
83+
raw: "github:///submitqueue/123/abc123",
84+
wantErr: true,
85+
},
86+
{
87+
name: "empty repo",
88+
raw: "github://uber//123/abc123",
89+
wantErr: true,
90+
},
91+
{
92+
name: "non-numeric PR number",
93+
raw: "github://uber/submitqueue/abc/abc123",
94+
wantErr: true,
95+
},
96+
{
97+
name: "empty SHA",
98+
raw: "github://uber/submitqueue/123/",
99+
wantErr: true,
100+
},
101+
{
102+
name: "empty string",
103+
raw: "",
104+
wantErr: true,
105+
},
106+
}
107+
108+
for _, tt := range tests {
109+
t.Run(tt.name, func(t *testing.T) {
110+
got, err := ParseChangeID(tt.raw)
111+
if tt.wantErr {
112+
require.Error(t, err)
113+
return
114+
}
115+
require.NoError(t, err)
116+
assert.Equal(t, tt.want, got)
117+
})
118+
}
119+
}
120+
121+
func TestChangeID_String(t *testing.T) {
122+
tests := []struct {
123+
name string
124+
id ChangeID
125+
want string
126+
}{
127+
{
128+
name: "github",
129+
id: ChangeID{
130+
Scheme: "github",
131+
Org: "uber",
132+
Repo: "submitqueue",
133+
PRNumber: 123,
134+
HeadCommitSHA: "abc123",
135+
},
136+
want: "github://uber/submitqueue/123/abc123",
137+
},
138+
{
139+
name: "ghe",
140+
id: ChangeID{
141+
Scheme: "ghe",
142+
Org: "corp",
143+
Repo: "app",
144+
PRNumber: 99,
145+
HeadCommitSHA: "deadbeef",
146+
},
147+
want: "ghe://corp/app/99/deadbeef",
148+
},
149+
{
150+
name: "ghes",
151+
id: ChangeID{
152+
Scheme: "ghes",
153+
Org: "org",
154+
Repo: "repo",
155+
PRNumber: 1,
156+
HeadCommitSHA: "sha1",
157+
},
158+
want: "ghes://org/repo/1/sha1",
159+
},
160+
}
161+
162+
for _, tt := range tests {
163+
t.Run(tt.name, func(t *testing.T) {
164+
assert.Equal(t, tt.want, tt.id.String())
165+
})
166+
}
167+
}
168+
169+
func TestChangeID_OwnerRepo(t *testing.T) {
170+
id := ChangeID{
171+
Scheme: "github",
172+
Org: "uber",
173+
Repo: "submitqueue",
174+
PRNumber: 1,
175+
HeadCommitSHA: "abc",
176+
}
177+
assert.Equal(t, "uber/submitqueue", id.OwnerRepo())
178+
}
179+
180+
func TestParseChangeID_RoundTrip(t *testing.T) {
181+
originals := []string{
182+
"github://uber/submitqueue/123/abc123def456",
183+
"ghe://corp/monorepo/99/deadbeef01234567",
184+
"ghes://org/repo/1/a1b2c3",
185+
}
186+
187+
for _, raw := range originals {
188+
t.Run(raw, func(t *testing.T) {
189+
parsed, err := ParseChangeID(raw)
190+
require.NoError(t, err)
191+
assert.Equal(t, raw, parsed.String())
192+
})
193+
}
194+
}

‎example/server/orchestrator/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ go_library(
1212
visibility = ["//visibility:private"],
1313
deps = [
1414
"//core/consumer",
15+
"//extension/mergechecker",
16+
"//extension/mergechecker/github",
1517
"//extension/queue",
1618
"//extension/queue/mysql",
1719
"//orchestrator/controller",

0 commit comments

Comments
 (0)