summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorIan Lewis <ianlewis@google.com>2020-08-14 16:21:29 -0700
committergVisor bot <gvisor-bot@google.com>2020-08-14 16:23:34 -0700
commit3f523b3bbcf5ef7f37bb247bd4c5727711c70ba9 (patch)
tree0908ef59b37ed7d25a08f4a1810a91739da84332
parentaf433e159dbf07b084c0f05e65ec646c056033a7 (diff)
Handle URLs better in issue reviver
- Handle urls ending in / - Add some url parsing tests PiperOrigin-RevId: 326750183
-rw-r--r--tools/issue_reviver/github/BUILD9
-rw-r--r--tools/issue_reviver/github/github.go32
-rw-r--r--tools/issue_reviver/github/github_test.go55
-rw-r--r--tools/issue_reviver/reviver/reviver.go2
4 files changed, 86 insertions, 12 deletions
diff --git a/tools/issue_reviver/github/BUILD b/tools/issue_reviver/github/BUILD
index 8b1c717df..0eabc2835 100644
--- a/tools/issue_reviver/github/BUILD
+++ b/tools/issue_reviver/github/BUILD
@@ -1,4 +1,4 @@
-load("//tools:defs.bzl", "go_library")
+load("//tools:defs.bzl", "go_library", "go_test")
package(licenses = ["notice"])
@@ -15,3 +15,10 @@ go_library(
"@org_golang_x_oauth2//:go_default_library",
],
)
+
+go_test(
+ name = "github_test",
+ size = "small",
+ srcs = ["github_test.go"],
+ library = ":github",
+)
diff --git a/tools/issue_reviver/github/github.go b/tools/issue_reviver/github/github.go
index e07949c8f..8ffd7e606 100644
--- a/tools/issue_reviver/github/github.go
+++ b/tools/issue_reviver/github/github.go
@@ -85,18 +85,13 @@ func (b *Bugger) load(token string) error {
// Activate implements reviver.Bugger.
func (b *Bugger) Activate(todo *reviver.Todo) (bool, error) {
- const prefix = "gvisor.dev/issue/"
-
- // First check if I can handle the TODO.
- idStr := strings.TrimPrefix(todo.Issue, prefix)
- if len(todo.Issue) == len(idStr) {
- return false, nil
- }
-
- id, err := strconv.Atoi(idStr)
+ id, err := parseIssueNo(todo.Issue)
if err != nil {
return true, err
}
+ if id <= 0 {
+ return false, nil
+ }
// Check against active issues cache.
if _, ok := b.issues[id]; ok {
@@ -115,7 +110,7 @@ func (b *Bugger) Activate(todo *reviver.Todo) (bool, error) {
l.File, l.Line, b.owner, b.repo, l.File, l.Line, l.Comment)
}
fmt.Fprintf(&comment,
- "\n\nSearch [TODO](https://github.com/%s/%s/search?q=%%22%s%d%%22)", b.owner, b.repo, prefix, id)
+ "\n\nSearch [TODO](https://github.com/%s/%s/search?q=%%22%s%%22)", b.owner, b.repo, todo.Issue)
if b.dryRun {
fmt.Printf("[dry-run: skipping change to issue %d]\n%s\n=======================\n", id, comment.String())
@@ -140,6 +135,23 @@ func (b *Bugger) Activate(todo *reviver.Todo) (bool, error) {
return true, nil
}
+// parseIssueNo parses the issue number out of the issue url.
+func parseIssueNo(url string) (int, error) {
+ const prefix = "gvisor.dev/issue/"
+
+ // First check if I can handle the TODO.
+ idStr := strings.TrimPrefix(url, prefix)
+ if len(url) == len(idStr) {
+ return 0, nil
+ }
+
+ id, err := strconv.ParseInt(strings.TrimRight(idStr, "/"), 10, 64)
+ if err != nil {
+ return 0, err
+ }
+ return int(id), nil
+}
+
func processAllPages(fn func(github.ListOptions) (*github.Response, error)) error {
opts := github.ListOptions{PerPage: 1000}
for {
diff --git a/tools/issue_reviver/github/github_test.go b/tools/issue_reviver/github/github_test.go
new file mode 100644
index 000000000..a78b230ef
--- /dev/null
+++ b/tools/issue_reviver/github/github_test.go
@@ -0,0 +1,55 @@
+// Copyright 2020 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package github
+
+import (
+ "testing"
+)
+
+func TestParseIssueNo(t *testing.T) {
+ testCases := []struct {
+ issue string
+ expectErr bool
+ expected int
+ }{
+ {
+ issue: "gvisor.dev/issue/123",
+ expected: 123,
+ },
+ {
+ issue: "gvisor.dev/issue/123/",
+ expected: 123,
+ },
+ {
+ issue: "not a url",
+ expected: 0,
+ },
+ {
+ issue: "gvisor.dev/issue//",
+ expectErr: true,
+ },
+ }
+
+ for _, tc := range testCases {
+ t.Run(tc.issue, func(t *testing.T) {
+ id, err := parseIssueNo(tc.issue)
+ if err != nil && !tc.expectErr {
+ t.Errorf("got error: %v", err)
+ } else if tc.expected != id {
+ t.Errorf("got: %v, want: %v", id, tc.expected)
+ }
+ })
+ }
+}
diff --git a/tools/issue_reviver/reviver/reviver.go b/tools/issue_reviver/reviver/reviver.go
index 682db0c01..2af7f0d59 100644
--- a/tools/issue_reviver/reviver/reviver.go
+++ b/tools/issue_reviver/reviver/reviver.go
@@ -26,7 +26,7 @@ import (
"sync"
)
-// This is how a TODO looks like.
+// regexTodo matches a TODO or FIXME comment.
var regexTodo = regexp.MustCompile(`(\/\/|#)\s*(TODO|FIXME)\(([a-zA-Z0-9.\/]+)\):\s*(.+)`)
// Bugger interface is called for every TODO found in the code. If it can handle