diff options
| -rw-r--r-- | tools/issue_reviver/github/BUILD | 9 | ||||
| -rw-r--r-- | tools/issue_reviver/github/github.go | 32 | ||||
| -rw-r--r-- | tools/issue_reviver/github/github_test.go | 55 | ||||
| -rw-r--r-- | tools/issue_reviver/reviver/reviver.go | 2 |
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 |