Skip to content

Commit 431df8c

Browse files
committed
cmd/gerritbot: update gerritChangeRE for new Gerrit output format
Gerrit has changed the format it uses for the Change URL in its output. It used to be something like: remote: SUCCESS remote: remote: New Changes: remote: https://go-review.googlesource.com/#/c/dl/+/134117 remove blank line from codereview.cfg Now it is: remote: SUCCESS remote: remote: New Changes: remote: https://go-review.googlesource.com/c/dl/+/134117 remove blank line from codereview.cfg (The '#' path element in the Change URL is removed.) Update the regexp to match, and add a test for it. This fix will likely cause the previously unposted "This PR has been imported to Gerrit for code review" comments to get posted during next pull request activity, since they haven't already been. Fixes golang/go#27561. Updates golang/go#27504. Change-Id: I37a75ddb8d879017ebd887c651ca74a6ad6b892d Reviewed-on: https://go-review.googlesource.com/134175 Reviewed-by: Andrew Bonventre <[email protected]>
1 parent a196f5a commit 431df8c

File tree

2 files changed

+17
-1
lines changed

2 files changed

+17
-1
lines changed

cmd/gerritbot/gerritbot.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ func runCmd(c *exec.Cmd) error {
530530
const gerritHostBase = "https://go.googlesource.com/"
531531

532532
// gerritChangeRE matches the URL to the Change within the git output when pushing to Gerrit.
533-
var gerritChangeRE = regexp.MustCompile(`https:\/\/go-review\.googlesource\.com\/#\/c\/\w+\/\+\/\d+`)
533+
var gerritChangeRE = regexp.MustCompile(`https:\/\/go-review\.googlesource\.com\/c\/\w+\/\+\/\d+`)
534534

535535
// importGerritChangeFromPR mirrors the latest state of pr to cl. If cl is nil,
536536
// then a new Gerrit Change is created.

cmd/gerritbot/gerritbot_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,3 +115,19 @@ GitHub-Pull-Request: golang/go#42
115115
})
116116
}
117117
}
118+
119+
// Test that gerritChangeRE matches the URL to the Change within
120+
// the git output from Gerrit after successfully creating a new CL.
121+
// Whenever Gerrit changes the Change URL format in its output,
122+
// we need to update gerritChangeRE and this test accordingly.
123+
//
124+
// See https://golang.org/issue/27561.
125+
func TestFindChangeURL(t *testing.T) {
126+
// Sample git output from Gerrit, extracted from production logs on 2018/09/07.
127+
const out = "remote: \rremote: Processing changes: new: 1 (\\)\rremote: Processing changes: new: 1 (|)\rremote: Processing changes: refs: 1, new: 1 (|)\rremote: Processing changes: refs: 1, new: 1 (|) \rremote: Processing changes: refs: 1, new: 1, done \nremote: \nremote: SUCCESS \nremote: \nremote: New Changes: \nremote: https://go-review.googlesource.com/c/dl/+/134117 remove blank line from codereview.cfg \nTo https://go.googlesource.com/dl\n * [new branch] HEAD -> refs/for/master"
128+
got := gerritChangeRE.FindString(out)
129+
want := "https://go-review.googlesource.com/c/dl/+/134117"
130+
if got != want {
131+
t.Errorf("could not find change URL in command output: %q\n\ngot %q, want %q", out, got, want)
132+
}
133+
}

0 commit comments

Comments
 (0)