diff --git a/.github/workflows/blockers.yml b/.github/workflows/blockers.yml new file mode 100644 index 0000000..fcd7794 --- /dev/null +++ b/.github/workflows/blockers.yml @@ -0,0 +1,74 @@ +# The reading of this board's own tracker that says whether an issue declaring +# itself blocked still points at anything. +# +# It runs on a schedule rather than on a pull request, for the reason the two +# comparisons beside it do: what it reads is the tracker, so its verdict moves +# when somebody edits an issue rather than when this tree changes, and a merge +# blocked by that is a gate punishing the wrong change. There is no half of +# this that needs no network, because the subject is the tracker and nothing +# else. +# +# The half that matters is the second one. An issue whose dependencies have all +# closed is available work that reads as unavailable, and nothing else on this +# board can tell: the label is correct on the day it goes on and there is no +# moment afterwards at which anything looks again. Four issues were in that +# state when this was written, two of them for a fortnight. +# +# It reports and does not edit. What it finds is repaired by somebody deciding +# what an issue waits for, and a run that stripped a label on its own would be +# taking that decision from a regular expression. The last step is what holds +# it to that. +name: Blockers + +on: + schedule: + # Daily rather than the weekly cadence the two comparisons of a pinned copy + # use. What those read moves when somebody else publishes; what this reads + # moves when work on this board closes, which is the thing that happens + # every day, and a week of an issue reading as unavailable is the cost this + # exists to stop paying. + - cron: "11 6 * * *" + workflow_dispatch: + +# Deny everything at the top level; the job below grants the one scope it needs. +permissions: {} + +jobs: + blockers: + name: Read every blocked issue and resolve what it names + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read # checkout only; nothing here writes + issues: read # the subject of the run, read and never edited + steps: + - name: Checkout Repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Install the toolchain go.mod pins + uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: go.mod + cache: false + + - name: Resolve what every blocked issue names + env: + # The listing is public and the run works without this. What the + # credential buys is the rate limit, which an anonymous run of this + # size spends most of, and the scope granted above is read only. + GITHUB_TOKEN: ${{ github.token }} + run: go run . blockers + + - name: Prove the run wrote nothing + # After the reading whether it passed or failed, because a run that + # edited an issue on its way to a red verdict is the failure this step + # exists for and a red verdict is exactly when it would be missed. + if: always() + shell: bash + run: | + set -euo pipefail + git status --porcelain=v1 + git diff --exit-code + test -z "$(git status --porcelain=v1)" diff --git a/internal/blockers/blockers.go b/internal/blockers/blockers.go new file mode 100644 index 0000000..2cd9160 --- /dev/null +++ b/internal/blockers/blockers.go @@ -0,0 +1,350 @@ +// Package blockers reads this board's own tracker and says whether an issue +// declaring itself blocked still points at anything. +// +// One label on this board means "waits on work tracked by another open issue +// on this board". The label cannot say which issue, so the body says it, and +// for most of the population the body said it in prose: "depends on the roster +// parser issue". A dependency written as a description is a dependency nothing +// can follow. Nobody can tell when it stops being true, so it goes on being +// asserted after the thing it names has closed, and the issue sits on the +// board reading as unavailable work while nothing is holding it. Four on this +// board were in that state when the reading that produced this package was +// taken, two of them for a fortnight. +// +// So the check asks two questions of every issue carrying the label, and they +// fail in opposite directions. An issue whose body names no number at all is +// refused: nothing can be resolved for it and no later run will do better. An +// issue whose named issues have all closed is refused too, and that is the +// half that matters, because that issue is available work every reader walks +// past. +// +// The states stay apart rather than collapsing into one count, for the reason +// the comparison over the pins keeps three: a reference this board does not +// hold is unresolved, which is a failure and never a pass. A run that resolved +// nothing and reported agreement is what this exists to prevent. +// +// The reading is the whole tracker rather than one query per issue. A body +// names a number that has closed more often than one that has not, and a +// closed issue is absent from the listing an open-issue query answers with, so +// resolving per reference spends the rate limit on the population this run is +// least able to predict the size of. +package blockers + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "regexp" + "sort" + "strconv" + "strings" + "time" +) + +// Label is the one this check reads. It is the string the tracker carries +// rather than a description of it, because what the run compares against is +// the label as declared. +const Label = "blocked-on-another-issue" + +// Owner and Repo are the board this reads. It is this repository's own +// tracker: the label's own description says "another open issue on this +// board", so a reference resolves here or it does not resolve at all. +const ( + Owner = "Flowfin" + Repo = "site" +) + +// An Issue is one row of the tracker. A pull request is one too, because the +// tracker answers with both under one numbering and a body naming a number +// cannot say which of the two it meant. +type Issue struct { + Number int + Title string + State string + Body string + Labels []string + PullRequest bool +} + +// A Board is everything one reading of the tracker returns. +// +// Labels is read as well as the issues, and it is not decoration. An issue +// listing filtered by a label nobody declares comes back empty, and an empty +// result is indistinguishable from a board on which nothing is blocked. One of +// those is the check working and the other is the check reading a name that no +// longer exists, so the run asks which one it has rather than passing on both. +type Board struct { + Labels []string + Issues []Issue +} + +// A Reader answers with one whole reading. It is a parameter so that the suite +// can produce every state the run has to keep apart, including the ones a +// tracker cannot be asked for on demand. +type Reader func() (Board, error) + +// reference matches a number the way a body means one. The leading class is +// what keeps two other shapes out: an escaped character reference, which is +// how a body carries an apostrophe and would otherwise read as issue 39, and a +// fragment on the end of an address, which names a heading and not an issue. +var reference = regexp.MustCompile(`(^|[^0-9A-Za-z_&/#-])#([0-9]+)`) + +// References is every issue number a body names, sorted and each one counted +// once. +// +// What it cannot do is judge whether a reference is the dependency or a +// mention of something else, so an issue naming a closed number in passing +// beside its open dependency reads here as still blocked. That direction is +// the safe one: it under-reports the issues this run asks somebody to look at +// rather than sending them to an issue that is genuinely waiting. +func References(body string) []int { + seen := map[int]bool{} + for _, m := range reference.FindAllStringSubmatchIndex(body, -1) { + start, end := m[4], m[5] + // What follows the digits decides it as much as what precedes them, + // and it is read here rather than in the pattern because a trailing + // class consumes the byte the next reference needs in front of it. + // A colour literal is the shape this keeps out: the digits of #1a2b3c + // are a prefix of a number nobody wrote. + if end < len(body) && isWordByte(body[end]) { + continue + } + n, err := strconv.Atoi(body[start:end]) + if err != nil || n == 0 { + continue + } + seen[n] = true + } + out := make([]int, 0, len(seen)) + for n := range seen { + out = append(out, n) + } + sort.Ints(out) + return out +} + +func isWordByte(b byte) bool { + switch { + case b >= '0' && b <= '9', b >= 'A' && b <= 'Z', b >= 'a' && b <= 'z': + return true + } + return b == '_' || b == '-' +} + +// Run takes one reading and reports what every blocked issue points at. +// +// It writes nothing anywhere. The repair for everything it finds is an edit to +// an issue, which is somebody deciding what that issue waits for, and a run +// that stripped a label on its own would be taking that decision from a +// regular expression. +func Run(read Reader, out io.Writer) error { + board, err := read() + if err != nil { + return fmt.Errorf("blockers: the tracker could not be read, so this run resolved nothing: %w", err) + } + + declared := false + for _, l := range board.Labels { + if l == Label { + declared = true + break + } + } + if !declared { + return fmt.Errorf("blockers: %s/%s declares no label %q, so an empty result here would mean the label had been renamed rather than that nothing is blocked", Owner, Repo, Label) + } + + held := map[int]Issue{} + for _, i := range board.Issues { + held[i.Number] = i + } + + var blocked []Issue + for _, i := range board.Issues { + if i.PullRequest || i.State != "open" { + continue + } + for _, l := range i.Labels { + if l == Label { + blocked = append(blocked, i) + break + } + } + } + sort.Slice(blocked, func(a, b int) bool { return blocked[a].Number < blocked[b].Number }) + + fmt.Fprintf(out, "blockers: %d open issue(s) carry %s, out of %d row(s) the tracker holds\n", len(blocked), Label, len(board.Issues)) + + nameless, cleared, unresolved := 0, 0, 0 + for _, i := range blocked { + var open, closed, missing []string + for _, n := range References(i.Body) { + if n == i.Number { + continue + } + ref, ok := held[n] + switch { + case !ok: + missing = append(missing, "#"+strconv.Itoa(n)) + case ref.State == "open": + open = append(open, describe(n, ref)) + default: + closed = append(closed, describe(n, ref)) + } + } + switch { + case len(open)+len(closed)+len(missing) == 0: + nameless++ + fmt.Fprintf(out, " #%d: NAMES NO ISSUE, it carries %s and its body has no number to resolve\n", i.Number, Label) + case len(missing) > 0: + unresolved++ + fmt.Fprintf(out, " #%d: UNRESOLVED, it names %s and this board holds no such row\n", i.Number, strings.Join(missing, ", ")) + case len(open) == 0: + cleared++ + fmt.Fprintf(out, " #%d: NO LONGER BLOCKED, everything it names has closed: %s\n", i.Number, strings.Join(closed, ", ")) + default: + fmt.Fprintf(out, " #%d: blocked, waiting on %s\n", i.Number, strings.Join(open, ", ")) + } + } + + fmt.Fprintf(out, "%d issue(s) read, %d naming no issue, %d no longer blocked, %d unresolved.\n", + len(blocked), nameless, cleared, unresolved) + + var wrong []string + if nameless > 0 { + wrong = append(wrong, fmt.Sprintf("%d name no issue by number", nameless)) + } + if cleared > 0 { + wrong = append(wrong, fmt.Sprintf("%d are no longer blocked and still say they are", cleared)) + } + if unresolved > 0 { + wrong = append(wrong, fmt.Sprintf("%d name a number this board does not hold", unresolved)) + } + if len(wrong) > 0 { + return fmt.Errorf("blockers: %s", strings.Join(wrong, ", ")) + } + return nil +} + +// describe names a reference the way a reader has to see it, because a body +// naming a merged pull request and a body naming a closed issue are the same +// four characters and are not the same statement. +func describe(n int, ref Issue) string { + kind := "issue" + if ref.PullRequest { + kind = "pull request" + } + return fmt.Sprintf("#%d (%s %s)", n, ref.State, kind) +} + +// Tracker is the reader the scheduled run uses. It asks for issues and pull +// requests in both states, because a reference this run has to resolve is most +// often to something that has closed. +func Tracker() (Board, error) { + var board Board + + if err := pages("/labels", func(body []byte) (int, error) { + var page []struct { + Name string `json:"name"` + } + if err := json.Unmarshal(body, &page); err != nil { + return 0, err + } + for _, l := range page { + board.Labels = append(board.Labels, l.Name) + } + return len(page), nil + }); err != nil { + return Board{}, err + } + + if err := pages("/issues?state=all", func(body []byte) (int, error) { + var page []struct { + Number int `json:"number"` + Title string `json:"title"` + State string `json:"state"` + Body string `json:"body"` + PullRequest *struct { + URL string `json:"url"` + } `json:"pull_request"` + Labels []struct { + Name string `json:"name"` + } `json:"labels"` + } + if err := json.Unmarshal(body, &page); err != nil { + return 0, err + } + for _, row := range page { + i := Issue{ + Number: row.Number, + Title: row.Title, + State: row.State, + Body: row.Body, + PullRequest: row.PullRequest != nil, + } + for _, l := range row.Labels { + i.Labels = append(i.Labels, l.Name) + } + board.Issues = append(board.Issues, i) + } + return len(page), nil + }); err != nil { + return Board{}, err + } + + if len(board.Issues) == 0 { + return Board{}, fmt.Errorf("%s/%s answered with no issue at all, which is not a board this check can read", Owner, Repo) + } + return board, nil +} + +// pages walks the paged listing at path and stops when a page comes back +// short. The cap is what keeps a tracker answering forever from turning a +// scheduled run into an unbounded one, and reaching it is a failure rather +// than a result, because what it would return is a prefix. +func pages(path string, take func([]byte) (int, error)) error { + const perPage = 100 + client := &http.Client{Timeout: 30 * time.Second} + separator := "?" + if strings.Contains(path, "?") { + separator = "&" + } + for page := 1; page <= 50; page++ { + address := fmt.Sprintf("https://api.github.com/repos/%s/%s%s%sper_page=%d&page=%d", + Owner, Repo, path, separator, perPage, page) + req, err := http.NewRequest(http.MethodGet, address, nil) + if err != nil { + return err + } + req.Header.Set("Accept", "application/vnd.github+json") + // The listing is public, so this run works without a credential and + // spends the anonymous rate limit when it has none. Where one is in + // the environment it is used, because the limit that buys is what + // makes the run survive a board of this size. + if token := strings.TrimSpace(os.Getenv("GITHUB_TOKEN")); token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + resp, err := client.Do(req) + if err != nil { + return err + } + body, err := io.ReadAll(io.LimitReader(resp.Body, 8<<20)) + resp.Body.Close() + if err != nil { + return err + } + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("%s answered %s", address, resp.Status) + } + n, err := take(body) + if err != nil { + return fmt.Errorf("reading %s: %w", address, err) + } + if n < perPage { + return nil + } + } + return fmt.Errorf("%s%s did not stop paging, so this run read a prefix rather than the whole listing", Repo, path) +} diff --git a/internal/blockers/blockers_test.go b/internal/blockers/blockers_test.go new file mode 100644 index 0000000..453493f --- /dev/null +++ b/internal/blockers/blockers_test.go @@ -0,0 +1,284 @@ +// The suite over the reference reader and the run. +// +// Every state the run keeps apart is produced by a reader that returns that +// state on demand. A tracker cannot be asked for a board whose label has been +// renamed, or for an issue naming a number nobody ever opened, and those are +// the states this run exists to keep apart, which is why the reading is a +// parameter rather than a call inside the run. +package blockers + +import ( + "bytes" + "errors" + "fmt" + "strings" + "testing" +) + +// board is a reading carrying the label and the rows given. +func board(rows ...Issue) Reader { + return func() (Board, error) { + return Board{Labels: []string{"documentation", Label, "security"}, Issues: rows}, nil + } +} + +func blockedIssue(number int, body string) Issue { + return Issue{Number: number, Title: fmt.Sprintf("issue %d", number), State: "open", Body: body, Labels: []string{"enhancement", Label}} +} + +func openIssue(number int) Issue { + return Issue{Number: number, Title: fmt.Sprintf("issue %d", number), State: "open"} +} + +func closedIssue(number int) Issue { + return Issue{Number: number, Title: fmt.Sprintf("issue %d", number), State: "closed"} +} + +func run(t *testing.T, read Reader) (string, error) { + t.Helper() + var out bytes.Buffer + err := Run(read, &out) + return out.String(), err +} + +func TestReferencesReadsEveryNumberOnceAndInOrder(t *testing.T) { + got := References("Depends on #26, the landing page, and on #5 which has closed. Again #26.") + want := []int{5, 26} + if fmt.Sprint(got) != fmt.Sprint(want) { + t.Fatalf("References returned %v, want %v", got, want) + } +} + +func TestReferencesKeepsOutTheShapesThatAreNotIssueNumbers(t *testing.T) { + // An escaped apostrophe, an address fragment and a colour literal are the + // three ways a body carries a hash followed by digits without meaning an + // issue. Reading any of them as one turns a blocked issue into an + // unresolved reference and reds a run for nothing. + for _, body := range []string{ + "the reader's browser", + "https://example.test/docs/parity.md#12", + "the token is #1a2b3c and nothing else", + "see decisions/0002-what-docker-is-for.md#3", + } { + if got := References(body); len(got) != 0 { + t.Errorf("References(%q) returned %v, want nothing", body, got) + } + } +} + +func TestReferencesReadsANumberAtTheStartOfTheBody(t *testing.T) { + if got := References("#53 decides the version scheme."); fmt.Sprint(got) != fmt.Sprint([]int{53}) { + t.Fatalf("References returned %v, want [53]", got) + } +} + +func TestRunPassesWhenEveryBlockedIssueNamesSomethingStillOpen(t *testing.T) { + out, err := run(t, board( + blockedIssue(36, "Depends on #35, the rendered timing budget."), + openIssue(35), + )) + if err != nil { + t.Fatalf("a board where every blocked issue points at an open one should pass, got %v\n%s", err, out) + } + if !strings.Contains(out, "#36: blocked, waiting on #35 (open issue)") { + t.Fatalf("the run should say what #36 waits on, got:\n%s", out) + } + if !strings.Contains(out, "1 issue(s) read, 0 naming no issue, 0 no longer blocked, 0 unresolved.") { + t.Fatalf("the run should count what it read, got:\n%s", out) + } +} + +func TestRunRefusesABlockedIssueWhoseBodyNamesNoNumber(t *testing.T) { + out, err := run(t, board( + blockedIssue(75, "Depends on the roster parser issue."), + closedIssue(21), + )) + if err == nil { + t.Fatalf("an issue naming its dependency only in prose should red the run, got:\n%s", out) + } + if !strings.Contains(out, "#75: NAMES NO ISSUE") { + t.Fatalf("the failure should name #75, got:\n%s", out) + } + if !strings.Contains(err.Error(), "1 name no issue by number") { + t.Fatalf("the error should count it, got %v", err) + } +} + +func TestRunRefusesABlockedIssueWhoseNamedIssuesHaveAllClosed(t *testing.T) { + // The half this check exists for. Nothing is holding #50 and every reader + // of the board walks past it, because the label says otherwise. + out, err := run(t, board( + blockedIssue(50, "Depends on #48, the privacy page, and on #37, the external origin refusal."), + closedIssue(48), + closedIssue(37), + )) + if err == nil { + t.Fatalf("an issue whose dependencies have all closed should red the run, got:\n%s", out) + } + if !strings.Contains(out, "#50: NO LONGER BLOCKED, everything it names has closed: #37 (closed issue), #48 (closed issue)") { + t.Fatalf("the failure should name #50 and both closed dependencies, got:\n%s", out) + } + if !strings.Contains(err.Error(), "1 are no longer blocked and still say they are") { + t.Fatalf("the error should count it, got %v", err) + } +} + +func TestRunKeepsAnIssueBlockedWhenOneOfSeveralIsStillOpen(t *testing.T) { + // The near miss for the row above. One closed dependency beside one open + // one is the ordinary state of a blocked issue, and a run reading "some + // have closed" as "no longer blocked" would red on most of the board. + out, err := run(t, board( + blockedIssue(58, "Depends on #53, the release workflow, and on #3 which has closed."), + openIssue(53), + closedIssue(3), + )) + if err != nil { + t.Fatalf("one open dependency should keep #58 blocked, got %v\n%s", err, out) + } + if !strings.Contains(out, "#58: blocked, waiting on #53 (open issue)") { + t.Fatalf("the run should name the open half only, got:\n%s", out) + } +} + +func TestRunReportsAReferenceThisBoardDoesNotHoldAsUnresolved(t *testing.T) { + // A number nobody opened resolves to nothing, and reading it as closed + // would report the issue as available work on the strength of a typo. + out, err := run(t, board( + blockedIssue(80, "Depends on #79 and on #4242."), + closedIssue(79), + )) + if err == nil { + t.Fatalf("a reference this board does not hold should red the run, got:\n%s", out) + } + if !strings.Contains(out, "#80: UNRESOLVED, it names #4242") { + t.Fatalf("the failure should name the reference, got:\n%s", out) + } + if !strings.Contains(err.Error(), "1 name a number this board does not hold") { + t.Fatalf("the error should count it, got %v", err) + } + if strings.Contains(out, "NO LONGER BLOCKED") { + t.Fatalf("an unresolved reference is not a cleared one, got:\n%s", out) + } +} + +func TestRunIgnoresAnIssueNamingItsOwnNumber(t *testing.T) { + // An issue that mentions its own number is open by definition, so + // counting it as a dependency makes it block itself and it can never be + // reported as cleared. This one waits on nothing: #35 has closed. + out, err := run(t, board( + blockedIssue(90, "As #90 says, this depends on #35."), + closedIssue(35), + )) + if err == nil { + t.Fatalf("#90 waits on nothing but itself and should be reported as cleared, got:\n%s", out) + } + if !strings.Contains(out, "#90: NO LONGER BLOCKED, everything it names has closed: #35 (closed issue)") { + t.Fatalf("the run should name only the real dependency, got:\n%s", out) + } + if strings.Contains(out, "#90 (open issue)") { + t.Fatalf("an issue should not appear as its own dependency, got:\n%s", out) + } +} + +func TestRunNamesAPullRequestAsOneRatherThanAsAnIssue(t *testing.T) { + out, err := run(t, board( + blockedIssue(72, "Landed by #165."), + Issue{Number: 165, Title: "a change", State: "closed", PullRequest: true}, + )) + if err == nil { + t.Fatalf("a merged pull request is a closed reference and should red the run, got:\n%s", out) + } + if !strings.Contains(out, "#165 (closed pull request)") { + t.Fatalf("the run should say the reference is a pull request, got:\n%s", out) + } +} + +func TestRunDoesNotJudgeAPullRequestCarryingTheLabel(t *testing.T) { + out, err := run(t, board( + Issue{Number: 179, Title: "a change", State: "open", Body: "no number here", PullRequest: true, Labels: []string{Label}}, + blockedIssue(36, "Depends on #35."), + openIssue(35), + )) + if err != nil { + t.Fatalf("a pull request is not an issue this check judges, got %v\n%s", err, out) + } + if strings.Contains(out, "#179") { + t.Fatalf("the run should not report on a pull request, got:\n%s", out) + } +} + +func TestRunDoesNotJudgeAClosedIssue(t *testing.T) { + out, err := run(t, board( + Issue{Number: 25, Title: "done", State: "closed", Body: "Depends on the roster parser issue.", Labels: []string{Label}}, + blockedIssue(36, "Depends on #35."), + openIssue(35), + )) + if err != nil { + t.Fatalf("a closed issue is not work and is not judged, got %v\n%s", err, out) + } + if strings.Contains(out, "#25") { + t.Fatalf("the run should not report on a closed issue, got:\n%s", out) + } +} + +func TestRunPassesABoardOnWhichNothingIsBlocked(t *testing.T) { + out, err := run(t, board(openIssue(35), closedIssue(3))) + if err != nil { + t.Fatalf("a board where nothing carries the label is a real zero, got %v\n%s", err, out) + } + if !strings.Contains(out, "0 issue(s) read, 0 naming no issue, 0 no longer blocked, 0 unresolved.") { + t.Fatalf("the run should say it read nothing, got:\n%s", out) + } +} + +func TestRunRefusesToPassABoardThatDeclaresNoSuchLabel(t *testing.T) { + // The near miss for the row above, and the reason the reading carries the + // labels at all. A renamed label answers every issue query with nothing, + // which is the same empty result as a board on which nothing is blocked. + read := func() (Board, error) { + return Board{Labels: []string{"documentation", "security"}, Issues: []Issue{openIssue(35)}}, nil + } + out, err := run(t, read) + if err == nil { + t.Fatalf("a board declaring no such label should red the run, got:\n%s", out) + } + if !strings.Contains(err.Error(), "declares no label") { + t.Fatalf("the error should say the label is absent, got %v", err) + } +} + +func TestRunRefusesToPassAReadingThatFailed(t *testing.T) { + read := func() (Board, error) { return Board{}, errors.New("the tracker answered 403 Forbidden") } + out, err := run(t, read) + if err == nil { + t.Fatalf("a reading that failed is not a board on which nothing is wrong, got:\n%s", out) + } + if !strings.Contains(err.Error(), "resolved nothing") { + t.Fatalf("the error should say the run resolved nothing, got %v", err) + } +} + +func TestRunCountsEveryStateSeparatelyOnOneBoard(t *testing.T) { + // The three failures are separate counts rather than one, because the + // repairs are different: a body to edit, a label to remove, and a number + // that is wrong. + out, err := run(t, board( + blockedIssue(75, "Depends on the roster parser issue."), + blockedIssue(50, "Depends on #48."), + blockedIssue(80, "Depends on #4242."), + blockedIssue(36, "Depends on #35."), + closedIssue(48), + openIssue(35), + )) + if err == nil { + t.Fatalf("three of these four are wrong and the run should be red, got:\n%s", out) + } + if !strings.Contains(out, "4 issue(s) read, 1 naming no issue, 1 no longer blocked, 1 unresolved.") { + t.Fatalf("the run should count the three states apart, got:\n%s", out) + } + for _, want := range []string{"1 name no issue by number", "1 are no longer blocked and still say they are", "1 name a number this board does not hold"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("the error should carry %q, got %v", want, err) + } + } +} diff --git a/main.go b/main.go index 5d838ac..5c95542 100644 --- a/main.go +++ b/main.go @@ -10,6 +10,7 @@ import ( "io" "os" + "github.com/Flowfin/site/internal/blockers" "github.com/Flowfin/site/internal/bom" "github.com/Flowfin/site/internal/changelog" "github.com/Flowfin/site/internal/gate" @@ -119,6 +120,17 @@ func run(args []string, out, errOut io.Writer) error { // the description is owed, and it runs this before it creates // anything. return changelog.Run(".", out) + case "blockers": + if len(args) != 1 { + usage(errOut) + return errors.New("blockers takes no argument") + } + // Deliberately not a leg of the gate, for the reason the two verbs + // above are not legs: what it reads is the tracker over the + // network, so its verdict moves when somebody edits an issue + // rather than when this tree changes, and a merge blocked by that + // would punish the wrong change. + return blockers.Run(blockers.Tracker, out) case "hygiene": return hygiene.Run(args[1:], out) default: @@ -147,6 +159,9 @@ func usage(w io.Writer) { go run . changelog refuse a version that `+changelog.File+` does not describe, which is what the release run asks before it creates a tag + go run . blockers + read the tracker and refuse an issue that says it is blocked + and names no issue, or whose named issues have all closed go run . hygiene [-origin=internal|external] judge the commit messages in a range `)