From f91e4b790e50fdbc5577ba8552a3f07aaffc0343 Mon Sep 17 00:00:00 2001 From: Nils Lehnen <30603423+iderex@users.noreply.github.com> Date: Sat, 22 Aug 2026 22:28:29 +0200 Subject: [PATCH] Resolve what a blocked issue says it waits for [#182] An issue on this board declaring itself blocked names what it waits for in its own body, and nothing read that. So a dependency written in prose could not be followed, and a dependency that had closed went on being asserted: the issue sat on the board reading as unavailable work while nothing was holding it. Four were in that state when this was measured, two of them for a fortnight. The blockers verb reads the tracker and resolves every reference. It refuses an issue carrying the label whose body names no number, refuses one whose named issues have all closed, and refuses a reference this board does not hold, and it keeps those three apart because their repairs are different. A reading that failed is a failure and never a pass, and a board that declares no such label is a failure too: an empty result there means the label was renamed rather than that nothing is blocked. It is not a leg of the gate, for the reason the pins and tokens comparisons are not legs. What it reads moves when somebody edits an issue rather than when this tree changes, so it runs on a schedule and a merge is never blocked by it. Every refusal is proved by a fixture that trips exactly it, and each was shown to bite by switching it off and watching the suite go red. Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com> --- .github/workflows/blockers.yml | 74 ++++++ internal/blockers/blockers.go | 350 +++++++++++++++++++++++++++++ internal/blockers/blockers_test.go | 284 +++++++++++++++++++++++ main.go | 15 ++ 4 files changed, 723 insertions(+) create mode 100644 .github/workflows/blockers.yml create mode 100644 internal/blockers/blockers.go create mode 100644 internal/blockers/blockers_test.go 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 `)