diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 381d4b0ce..e5ad39f5c 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -26,6 +26,8 @@ Contributions to this project are [released][legal] to the public under the [pro Please note that this project adheres to a [Contributor Code of Conduct][code-of-conduct]. By participating in this project you agree to abide by its terms. +We generate manual pages from source on every release! You do not need to submit PRs for those specifically; the docs will get updated if your PR gets accepted. + ## Resources - [How to Contribute to Open Source](https://opensource.guide/how-to-contribute/) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 19e727fdf..16e5348f1 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -1,6 +1,9 @@ --- name: "\U0001F41B Bug report" about: Report a bug or unexpected behavior while using GitHub CLI +title: '' +labels: bug +assignees: '' --- diff --git a/.github/ISSUE_TEMPLATE/submit-a-request.md b/.github/ISSUE_TEMPLATE/submit-a-request.md index 585afa21e..4f66ac457 100644 --- a/.github/ISSUE_TEMPLATE/submit-a-request.md +++ b/.github/ISSUE_TEMPLATE/submit-a-request.md @@ -1,6 +1,9 @@ --- -name: "\U00002B50 Submit a request" +name: "⭐ Submit a request" about: Surface a feature or problem that you think should be solved +title: '' +labels: enhancement +assignees: '' --- diff --git a/.github/PULL_REQUEST_TEMPLATE/bug_fix.md b/.github/PULL_REQUEST_TEMPLATE/bug_fix.md new file mode 100644 index 000000000..5659ce40b --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE/bug_fix.md @@ -0,0 +1,13 @@ + + +## Summary + +closes #[issue number] + +## Details + +- diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 9800d2827..557f87049 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -1,5 +1,5 @@ name: Tests -on: [push] +on: [push, pull_request] jobs: build: strategy: diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index e8a3c0143..f241dc1d9 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -25,8 +25,6 @@ jobs: version: latest args: release --release-notes=CHANGELOG.md env: - GH_OAUTH_CLIENT_ID: 178c6fc778ccc68e1d6a - GH_OAUTH_CLIENT_SECRET: ${{secrets.OAUTH_CLIENT_SECRET}} GITHUB_TOKEN: ${{secrets.UPLOAD_GITHUB_TOKEN}} msi: needs: goreleaser diff --git a/.gitignore b/.gitignore index 76ddfcc68..b384e3d5c 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,9 @@ /site .github/**/node_modules /CHANGELOG.md + +# VS Code +.vscode + +# macOS +.DS_Store \ No newline at end of file diff --git a/.goreleaser.yml b/.goreleaser.yml index 68691514a..619180def 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -13,8 +13,6 @@ builds: main: ./cmd/gh ldflags: - -s -w -X github.com/cli/cli/command.Version={{.Version}} -X github.com/cli/cli/command.BuildDate={{time "2006-01-02"}} - - -X github.com/cli/cli/context.oauthClientID={{.Env.GH_OAUTH_CLIENT_ID}} - - -X github.com/cli/cli/context.oauthClientSecret={{.Env.GH_OAUTH_CLIENT_SECRET}} - -X main.updaterEnabled=cli/cli id: macos goos: [darwin] @@ -87,5 +85,6 @@ scoop: name: vilmibm email: vilmibm@github.com homepage: https://github.com/cli/cli + skip_upload: auto description: GitHub CLI license: MIT diff --git a/Makefile b/Makefile index be2483cdc..3b8aba5ea 100644 --- a/Makefile +++ b/Makefile @@ -2,15 +2,21 @@ BUILD_FILES = $(shell go list -f '{{range .GoFiles}}{{$$.Dir}}/{{.}}\ {{end}}' ./...) GH_VERSION ?= $(shell git describe --tags 2>/dev/null || git rev-parse --short HEAD) +DATE_FMT = +%Y-%m-%d +ifdef SOURCE_DATE_EPOCH + BUILD_DATE ?= $(shell date -u -d "@$(SOURCE_DATE_EPOCH)" "$(DATE_FMT)" 2>/dev/null || date -u -r "$(SOURCE_DATE_EPOCH)" "$(DATE_FMT)" 2>/dev/null || date -u "$(DATE_FMT)") +else + BUILD_DATE ?= $(shell date "$(DATE_FMT)") +endif LDFLAGS := -X github.com/cli/cli/command.Version=$(GH_VERSION) $(LDFLAGS) -LDFLAGS := -X github.com/cli/cli/command.BuildDate=$(shell date +%Y-%m-%d) $(LDFLAGS) +LDFLAGS := -X github.com/cli/cli/command.BuildDate=$(BUILD_DATE) $(LDFLAGS) ifdef GH_OAUTH_CLIENT_SECRET LDFLAGS := -X github.com/cli/cli/context.oauthClientID=$(GH_OAUTH_CLIENT_ID) $(LDFLAGS) LDFLAGS := -X github.com/cli/cli/context.oauthClientSecret=$(GH_OAUTH_CLIENT_SECRET) $(LDFLAGS) endif bin/gh: $(BUILD_FILES) - @go build -ldflags "$(LDFLAGS)" -o "$@" ./cmd/gh + @go build -trimpath -ldflags "$(LDFLAGS)" -o "$@" ./cmd/gh test: go test ./... diff --git a/README.md b/README.md index 5a19f287c..c98b207d0 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,10 @@ the terminal next to where you are already working with `git` and your code. ![screenshot](https://user-images.githubusercontent.com/98482/73286699-9f922180-41bd-11ea-87c9-60a2d31fd0ac.png) +## Availability + +While in beta, GitHub CLI is available for repos hosted on GitHub.com only. It does not currently support repositories hosted on GitHub Enterprise Server or other hosting providers. + ## We need your feedback GitHub CLI is currently early in its development, and we're hoping to get feedback from people using it. @@ -40,7 +44,9 @@ Upgrade: `brew update && brew upgrade gh` ### Windows -`gh` is available via [scoop][]: +`gh` is available via [scoop][], [Chocolatey][], and as downloadable MSI. + +#### scoop Install: @@ -51,7 +57,23 @@ scoop install gh Upgrade: `scoop update gh` -Signed MSI installers are also available on the [releases page][]. +#### Chocolatey + +Install: + +``` +choco install gh +``` + +Upgrade: + +``` +choco upgrade gh +``` + +#### Signed MSI + +MSI installers are available for download on the [releases page][]. ### Debian/Ubuntu Linux @@ -67,15 +89,30 @@ Install and upgrade: 1. Download the `.rpm` file from the [releases page][] 2. `sudo yum localinstall gh_*_linux_amd64.rpm` install the downloaded file +### openSUSE/SUSE Linux + +Install and upgrade: + +1. Download the `.rpm` file from the [releases page][] +2. `sudo zypper in gh_*_linux_amd64.rpm` install the downloaded file + +### Arch Linux + +Arch Linux users can install from the AUR: https://aur.archlinux.org/packages/github-cli/ + +```bash +$ yay -S github-cli +``` + ### Other platforms Install a prebuilt binary from the [releases page][] ### [Build from source](/source.md) - -[docs]: https://cli.github.io/cli/gh +[docs]: https://cli.github.com/manual [scoop]: https://scoop.sh +[Chocolatey]: https://chocolatey.org [releases page]: https://github.com/cli/cli/releases/latest [hub]: https://github.com/github/hub [contributing page]: https://github.com/cli/cli/blob/master/.github/CONTRIBUTING.md diff --git a/api/fake_http.go b/api/fake_http.go index 96e38aab3..32b7d56eb 100644 --- a/api/fake_http.go +++ b/api/fake_http.go @@ -1,6 +1,7 @@ package api import ( + "bytes" "fmt" "io" "io/ioutil" @@ -35,3 +36,23 @@ func (f *FakeHTTP) RoundTrip(req *http.Request) (*http.Response, error) { f.Requests = append(f.Requests, req) return resp, nil } + +func (f *FakeHTTP) StubRepoResponse(owner, repo string) { + body := bytes.NewBufferString(fmt.Sprintf(` + { "data": { "repo_000": { + "id": "REPOID", + "name": "%s", + "owner": {"login": "%s"}, + "defaultBranchRef": { + "name": "master", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `, repo, owner)) + resp := &http.Response{ + StatusCode: 200, + Body: ioutil.NopCloser(body), + } + f.responseStubs = append(f.responseStubs, resp) +} diff --git a/auth/oauth.go b/auth/oauth.go index 54bd5137b..6e85575bf 100644 --- a/auth/oauth.go +++ b/auth/oauth.go @@ -2,6 +2,7 @@ package auth import ( "crypto/rand" + "encoding/hex" "errors" "fmt" "io" @@ -20,7 +21,7 @@ func randomString(length int) (string, error) { if err != nil { return "", err } - return fmt.Sprintf("%x", b), nil + return hex.EncodeToString(b), nil } // OAuthFlow represents the setup for authenticating with GitHub diff --git a/command/issue.go b/command/issue.go index ea35d9393..1c66bee99 100644 --- a/command/issue.go +++ b/command/issue.go @@ -83,7 +83,7 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := ctx.BaseRepo() + baseRepo, err := determineBaseRepo(cmd, ctx) if err != nil { return err } @@ -108,9 +108,9 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - fmt.Fprintf(colorableErr(cmd), "\nIssues for %s\n\n", ghrepo.FullName(baseRepo)) + fmt.Fprintf(colorableErr(cmd), "\nIssues for %s\n\n", ghrepo.FullName(*baseRepo)) - issues, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit) + issues, err := api.IssueList(apiClient, *baseRepo, state, labels, assignee, limit) if err != nil { return err } @@ -158,7 +158,7 @@ func issueStatus(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := ctx.BaseRepo() + baseRepo, err := determineBaseRepo(cmd, ctx) if err != nil { return err } @@ -168,7 +168,7 @@ func issueStatus(cmd *cobra.Command, args []string) error { return err } - issuePayload, err := api.IssueStatus(apiClient, baseRepo, currentUser) + issuePayload, err := api.IssueStatus(apiClient, *baseRepo, currentUser) if err != nil { return err } @@ -176,7 +176,7 @@ func issueStatus(cmd *cobra.Command, args []string) error { out := colorableOut(cmd) fmt.Fprintln(out, "") - fmt.Fprintf(out, "Relevant issues in %s\n", ghrepo.FullName(baseRepo)) + fmt.Fprintf(out, "Relevant issues in %s\n", ghrepo.FullName(*baseRepo)) fmt.Fprintln(out, "") printHeader(out, "Issues assigned to you") @@ -210,16 +210,17 @@ func issueStatus(cmd *cobra.Command, args []string) error { func issueView(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) - baseRepo, err := ctx.BaseRepo() - if err != nil { - return err - } apiClient, err := apiClientForContext(ctx) if err != nil { return err } - issue, err := issueFromArg(apiClient, baseRepo, args[0]) + baseRepo, err := determineBaseRepo(cmd, ctx) + if err != nil { + return err + } + + issue, err := issueFromArg(apiClient, *baseRepo, args[0]) if err != nil { return err } @@ -253,13 +254,17 @@ func printIssuePreview(out io.Writer, issue *api.Issue) error { utils.Pluralize(issue.Comments.TotalCount, "comment"), coloredLabels, ))) - fmt.Fprintln(out) - md, err := utils.RenderMarkdown(issue.Body) - if err != nil { - return err + + if issue.Body != "" { + fmt.Fprintln(out) + md, err := utils.RenderMarkdown(issue.Body) + if err != nil { + return err + } + fmt.Fprintln(out, md) + fmt.Fprintln(out) } - fmt.Fprintln(out, md) - fmt.Fprintln(out) + fmt.Fprintf(out, utils.Gray("View this issue on GitHub: %s\n"), issue.URL) return nil } @@ -282,22 +287,30 @@ func issueFromArg(apiClient *api.Client, baseRepo ghrepo.Interface, arg string) func issueCreate(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) - baseRepo, err := ctx.BaseRepo() + // NB no auto forking like over in pr create + baseRepo, err := determineBaseRepo(cmd, ctx) if err != nil { return err } - fmt.Fprintf(colorableErr(cmd), "\nCreating issue in %s\n\n", ghrepo.FullName(baseRepo)) + fmt.Fprintf(colorableErr(cmd), "\nCreating issue in %s\n\n", ghrepo.FullName(*baseRepo)) + + baseOverride, err := cmd.Flags().GetString("repo") + if err != nil { + return err + } var templateFiles []string - if rootDir, err := git.ToplevelDir(); err == nil { - // TODO: figure out how to stub this in tests - templateFiles = githubtemplate.Find(rootDir, "ISSUE_TEMPLATE") + if baseOverride == "" { + if rootDir, err := git.ToplevelDir(); err == nil { + // TODO: figure out how to stub this in tests + templateFiles = githubtemplate.Find(rootDir, "ISSUE_TEMPLATE") + } } if isWeb, err := cmd.Flags().GetBool("web"); err == nil && isWeb { // TODO: move URL generation into GitHubRepository - openURL := fmt.Sprintf("https://github.com/%s/issues/new", ghrepo.FullName(baseRepo)) + openURL := fmt.Sprintf("https://github.com/%s/issues/new", ghrepo.FullName(*baseRepo)) if len(templateFiles) > 1 { openURL += "/choose" } @@ -310,12 +323,12 @@ func issueCreate(cmd *cobra.Command, args []string) error { return err } - repo, err := api.GitHubRepo(apiClient, baseRepo) + repo, err := api.GitHubRepo(apiClient, *baseRepo) if err != nil { return err } if !repo.HasIssuesEnabled { - return fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(baseRepo)) + return fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(*baseRepo)) } action := SubmitAction @@ -356,7 +369,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { if action == PreviewAction { openURL := fmt.Sprintf( "https://github.com/%s/issues/new/?title=%s&body=%s", - ghrepo.FullName(baseRepo), + ghrepo.FullName(*baseRepo), url.QueryEscape(title), url.QueryEscape(body), ) diff --git a/command/issue_test.go b/command/issue_test.go index 98ba6119e..c09de2c62 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -15,6 +15,7 @@ import ( func TestIssueStatus(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/issueStatus.json") defer jsonFile.Close() @@ -43,6 +44,7 @@ func TestIssueStatus(t *testing.T) { func TestIssueStatus_blankSlate(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -79,6 +81,7 @@ Issues opened by you func TestIssueStatus_disabledIssues(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -95,6 +98,7 @@ func TestIssueStatus_disabledIssues(t *testing.T) { func TestIssueList(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/issueList.json") defer jsonFile.Close() @@ -122,6 +126,7 @@ func TestIssueList(t *testing.T) { func TestIssueList_withFlags(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -142,7 +147,7 @@ Issues for OWNER/REPO No issues match your search `) - bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) reqBody := struct { Variables struct { Assignee string @@ -160,6 +165,7 @@ No issues match your search func TestIssueList_nullAssigneeLabels(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -173,7 +179,7 @@ func TestIssueList_nullAssigneeLabels(t *testing.T) { t.Errorf("error running command `issue list`: %v", err) } - bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) reqBody := struct { Variables map[string]interface{} }{} @@ -188,6 +194,7 @@ func TestIssueList_nullAssigneeLabels(t *testing.T) { func TestIssueList_disabledIssues(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -204,6 +211,7 @@ func TestIssueList_disabledIssues(t *testing.T) { func TestIssueView(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, "issue": { @@ -237,6 +245,7 @@ func TestIssueView(t *testing.T) { func TestIssueView_preview(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, "issue": { @@ -279,6 +288,51 @@ func TestIssueView_preview(t *testing.T) { } } +func TestIssueView_previewWithEmptyBody(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 123, + "body": "", + "title": "ix of coins", + "author": { + "login": "marseilles" + }, + "labels": { + "nodes": [ + {"name": "tarot"} + ] + }, + "comments": { + "totalCount": 9 + }, + "url": "https://github.com/OWNER/REPO/issues/123" + } } } } + `)) + + output, err := RunCommand(issueViewCmd, "issue view -p 123") + if err != nil { + t.Errorf("error running command `issue view`: %v", err) + } + + eq(t, output.Stderr(), "") + + expectedLines := []*regexp.Regexp{ + regexp.MustCompile(`ix of coins`), + regexp.MustCompile(`opened by marseilles. 9 comments. \(tarot\)`), + regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), + } + for _, r := range expectedLines { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } + } +} + func TestIssueView_notFound(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() @@ -309,6 +363,7 @@ func TestIssueView_notFound(t *testing.T) { func TestIssueView_disabledIssues(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -326,6 +381,7 @@ func TestIssueView_disabledIssues(t *testing.T) { func TestIssueView_urlArg(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "hasIssuesEnabled": true, "issue": { @@ -358,6 +414,7 @@ func TestIssueView_urlArg(t *testing.T) { func TestIssueCreate(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -376,7 +433,7 @@ func TestIssueCreate(t *testing.T) { t.Errorf("error running command `issue create`: %v", err) } - bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) reqBody := struct { Variables struct { Input struct { @@ -398,6 +455,7 @@ func TestIssueCreate(t *testing.T) { func TestIssueCreate_disabledIssues(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -414,7 +472,8 @@ func TestIssueCreate_disabledIssues(t *testing.T) { func TestIssueCreate_web(t *testing.T) { initBlankContext("OWNER/REPO", "master") - initFakeHTTP() + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") var seenCmd *exec.Cmd restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { diff --git a/command/pr.go b/command/pr.go index 7676b09a7..cd2bf91e9 100644 --- a/command/pr.go +++ b/command/pr.go @@ -1,11 +1,8 @@ package command import ( - "errors" "fmt" "io" - "os" - "os/exec" "regexp" "strconv" "strings" @@ -46,17 +43,6 @@ A pull request can be supplied as argument in any of the following formats: - by URL, e.g. "https://github.com/OWNER/REPO/pull/123"; or - by the name of its head branch, e.g. "patch-1" or "OWNER:patch-1".`, } -var prCheckoutCmd = &cobra.Command{ - Use: "checkout { | | }", - Short: "Check out a pull request in Git", - Args: func(cmd *cobra.Command, args []string) error { - if len(args) < 1 { - return errors.New("requires a PR number as an argument") - } - return nil - }, - RunE: prCheckout, -} var prListCmd = &cobra.Command{ Use: "list", Short: "List and filter pull requests in this repository", @@ -84,10 +70,6 @@ func prStatus(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := ctx.BaseRepo() - if err != nil { - return err - } currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx) if err != nil { return err @@ -97,7 +79,12 @@ func prStatus(cmd *cobra.Command, args []string) error { return err } - prPayload, err := api.PullRequests(apiClient, baseRepo, currentPRNumber, currentPRHeadRef, currentUser) + baseRepo, err := determineBaseRepo(cmd, ctx) + if err != nil { + return err + } + + prPayload, err := api.PullRequests(apiClient, *baseRepo, currentPRNumber, currentPRHeadRef, currentUser) if err != nil { return err } @@ -105,7 +92,7 @@ func prStatus(cmd *cobra.Command, args []string) error { out := colorableOut(cmd) fmt.Fprintln(out, "") - fmt.Fprintf(out, "Relevant pull requests in %s\n", ghrepo.FullName(baseRepo)) + fmt.Fprintf(out, "Relevant pull requests in %s\n", ghrepo.FullName(*baseRepo)) fmt.Fprintln(out, "") printHeader(out, "Current branch") @@ -143,12 +130,12 @@ func prList(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := ctx.BaseRepo() + baseRepo, err := determineBaseRepo(cmd, ctx) if err != nil { return err } - fmt.Fprintf(colorableErr(cmd), "\nPull requests for %s\n\n", ghrepo.FullName(baseRepo)) + fmt.Fprintf(colorableErr(cmd), "\nPull requests for %s\n\n", ghrepo.FullName(*baseRepo)) limit, err := cmd.Flags().GetInt("limit") if err != nil { @@ -186,8 +173,8 @@ func prList(cmd *cobra.Command, args []string) error { } params := map[string]interface{}{ - "owner": baseRepo.RepoOwner(), - "repo": baseRepo.RepoName(), + "owner": (*baseRepo).RepoOwner(), + "repo": (*baseRepo).RepoName(), "state": graphqlState, } if len(labels) > 0 { @@ -206,7 +193,7 @@ func prList(cmd *cobra.Command, args []string) error { } if len(prs) == 0 { - colorErr := colorableErr(cmd) // Send to stderr because otherwise when piping this command it would seem like the "no open prs" message is acually a pr + colorErr := colorableErr(cmd) // Send to stderr because otherwise when piping this command it would seem like the "no open prs" message is actually a pr msg := "There are no open pull requests" userSetFlags := false @@ -254,12 +241,13 @@ func colorFuncForState(state string) func(string) string { func prView(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) - baseRepo, err := ctx.BaseRepo() + + apiClient, err := apiClientForContext(ctx) if err != nil { return err } - apiClient, err := apiClientForContext(ctx) + baseRepo, err := determineBaseRepo(cmd, ctx) if err != nil { return err } @@ -272,7 +260,7 @@ func prView(cmd *cobra.Command, args []string) error { var openURL string var pr *api.PullRequest if len(args) > 0 { - pr, err = prFromArg(apiClient, baseRepo, args[0]) + pr, err = prFromArg(apiClient, *baseRepo, args[0]) if err != nil { return err } @@ -284,15 +272,15 @@ func prView(cmd *cobra.Command, args []string) error { } if prNumber > 0 { - openURL = fmt.Sprintf("https://github.com/%s/pull/%d", ghrepo.FullName(baseRepo), prNumber) + openURL = fmt.Sprintf("https://github.com/%s/pull/%d", ghrepo.FullName(*baseRepo), prNumber) if preview { - pr, err = api.PullRequestByNumber(apiClient, baseRepo, prNumber) + pr, err = api.PullRequestByNumber(apiClient, *baseRepo, prNumber) if err != nil { return err } } } else { - pr, err = api.PullRequestForBranch(apiClient, baseRepo, branchWithOwner) + pr, err = api.PullRequestForBranch(apiClient, *baseRepo, branchWithOwner) if err != nil { return err } @@ -319,13 +307,16 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) error { pr.BaseRefName, pr.HeadRefName, ))) - fmt.Fprintln(out) - md, err := utils.RenderMarkdown(pr.Body) - if err != nil { - return err + if pr.Body != "" { + fmt.Fprintln(out) + md, err := utils.RenderMarkdown(pr.Body) + if err != nil { + return err + } + fmt.Fprintln(out, md) + fmt.Fprintln(out) } - fmt.Fprintln(out, md) - fmt.Fprintln(out) + fmt.Fprintf(out, utils.Gray("View this pull request on GitHub: %s\n"), pr.URL) return nil } @@ -390,95 +381,6 @@ func prSelectorForCurrentBranch(ctx context.Context) (prNumber int, prHeadRef st return } -func prCheckout(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - currentBranch, _ := ctx.Branch() - remotes, err := ctx.Remotes() - if err != nil { - return err - } - // FIXME: duplicates logic from fsContext.BaseRepo - baseRemote, err := remotes.FindByName("upstream", "github", "origin", "*") - if err != nil { - return err - } - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - pr, err := prFromArg(apiClient, baseRemote, args[0]) - if err != nil { - return err - } - - headRemote := baseRemote - if pr.IsCrossRepository { - headRemote, _ = remotes.FindByRepo(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name) - } - - cmdQueue := [][]string{} - - newBranchName := pr.HeadRefName - if headRemote != nil { - // there is an existing git remote for PR head - remoteBranch := fmt.Sprintf("%s/%s", headRemote.Name, pr.HeadRefName) - refSpec := fmt.Sprintf("+refs/heads/%s:refs/remotes/%s", pr.HeadRefName, remoteBranch) - - cmdQueue = append(cmdQueue, []string{"git", "fetch", headRemote.Name, refSpec}) - - // local branch already exists - if git.VerifyRef("refs/heads/" + newBranchName) { - cmdQueue = append(cmdQueue, []string{"git", "checkout", newBranchName}) - cmdQueue = append(cmdQueue, []string{"git", "merge", "--ff-only", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) - } else { - cmdQueue = append(cmdQueue, []string{"git", "checkout", "-b", newBranchName, "--no-track", remoteBranch}) - cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), headRemote.Name}) - cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), "refs/heads/" + pr.HeadRefName}) - } - } else { - // no git remote for PR head - - // avoid naming the new branch the same as the default branch - if newBranchName == pr.HeadRepository.DefaultBranchRef.Name { - newBranchName = fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, newBranchName) - } - - ref := fmt.Sprintf("refs/pull/%d/head", pr.Number) - if newBranchName == currentBranch { - // PR head matches currently checked out branch - cmdQueue = append(cmdQueue, []string{"git", "fetch", baseRemote.Name, ref}) - cmdQueue = append(cmdQueue, []string{"git", "merge", "--ff-only", "FETCH_HEAD"}) - } else { - // create a new branch - cmdQueue = append(cmdQueue, []string{"git", "fetch", baseRemote.Name, fmt.Sprintf("%s:%s", ref, newBranchName)}) - cmdQueue = append(cmdQueue, []string{"git", "checkout", newBranchName}) - } - - remote := baseRemote.Name - mergeRef := ref - if pr.MaintainerCanModify { - remote = fmt.Sprintf("https://github.com/%s/%s.git", pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name) - mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName) - } - if mc, err := git.Config(fmt.Sprintf("branch.%s.merge", newBranchName)); err != nil || mc == "" { - cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), remote}) - cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), mergeRef}) - } - } - - for _, args := range cmdQueue { - cmd := exec.Command(args[0], args[1:]...) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - if err := utils.PrepareCmd(cmd).Run(); err != nil { - return err - } - } - - return nil -} - func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) { for _, pr := range prs { prNumber := fmt.Sprintf("#%d", pr.Number) @@ -507,11 +409,11 @@ func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) { } if reviews.ChangesRequested { - fmt.Fprintf(w, " - %s", utils.Red("changes requested")) + fmt.Fprintf(w, " - %s", utils.Red("Changes requested")) } else if reviews.ReviewRequired { - fmt.Fprintf(w, " - %s", utils.Yellow("review required")) + fmt.Fprintf(w, " - %s", utils.Yellow("Review required")) } else if reviews.Approved { - fmt.Fprintf(w, " - %s", utils.Green("approved")) + fmt.Fprintf(w, " - %s", utils.Green("Approved")) } fmt.Fprint(w, "\n") diff --git a/command/pr_checkout.go b/command/pr_checkout.go new file mode 100644 index 000000000..43556c67f --- /dev/null +++ b/command/pr_checkout.go @@ -0,0 +1,113 @@ +package command + +import ( + "errors" + "fmt" + "os" + "os/exec" + + "github.com/cli/cli/git" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +func prCheckout(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + currentBranch, _ := ctx.Branch() + remotes, err := ctx.Remotes() + if err != nil { + return err + } + // FIXME: duplicates logic from fsContext.BaseRepo + baseRemote, err := remotes.FindByName("upstream", "github", "origin", "*") + if err != nil { + return err + } + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + pr, err := prFromArg(apiClient, baseRemote, args[0]) + if err != nil { + return err + } + + headRemote := baseRemote + if pr.IsCrossRepository { + headRemote, _ = remotes.FindByRepo(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name) + } + + cmdQueue := [][]string{} + + newBranchName := pr.HeadRefName + if headRemote != nil { + // there is an existing git remote for PR head + remoteBranch := fmt.Sprintf("%s/%s", headRemote.Name, pr.HeadRefName) + refSpec := fmt.Sprintf("+refs/heads/%s:refs/remotes/%s", pr.HeadRefName, remoteBranch) + + cmdQueue = append(cmdQueue, []string{"git", "fetch", headRemote.Name, refSpec}) + + // local branch already exists + if git.VerifyRef("refs/heads/" + newBranchName) { + cmdQueue = append(cmdQueue, []string{"git", "checkout", newBranchName}) + cmdQueue = append(cmdQueue, []string{"git", "merge", "--ff-only", fmt.Sprintf("refs/remotes/%s", remoteBranch)}) + } else { + cmdQueue = append(cmdQueue, []string{"git", "checkout", "-b", newBranchName, "--no-track", remoteBranch}) + cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), headRemote.Name}) + cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), "refs/heads/" + pr.HeadRefName}) + } + } else { + // no git remote for PR head + + // avoid naming the new branch the same as the default branch + if newBranchName == pr.HeadRepository.DefaultBranchRef.Name { + newBranchName = fmt.Sprintf("%s/%s", pr.HeadRepositoryOwner.Login, newBranchName) + } + + ref := fmt.Sprintf("refs/pull/%d/head", pr.Number) + if newBranchName == currentBranch { + // PR head matches currently checked out branch + cmdQueue = append(cmdQueue, []string{"git", "fetch", baseRemote.Name, ref}) + cmdQueue = append(cmdQueue, []string{"git", "merge", "--ff-only", "FETCH_HEAD"}) + } else { + // create a new branch + cmdQueue = append(cmdQueue, []string{"git", "fetch", baseRemote.Name, fmt.Sprintf("%s:%s", ref, newBranchName)}) + cmdQueue = append(cmdQueue, []string{"git", "checkout", newBranchName}) + } + + remote := baseRemote.Name + mergeRef := ref + if pr.MaintainerCanModify { + remote = fmt.Sprintf("https://github.com/%s/%s.git", pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name) + mergeRef = fmt.Sprintf("refs/heads/%s", pr.HeadRefName) + } + if mc, err := git.Config(fmt.Sprintf("branch.%s.merge", newBranchName)); err != nil || mc == "" { + cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), remote}) + cmdQueue = append(cmdQueue, []string{"git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), mergeRef}) + } + } + + for _, args := range cmdQueue { + cmd := exec.Command(args[0], args[1:]...) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + if err := utils.PrepareCmd(cmd).Run(); err != nil { + return err + } + } + + return nil +} + +var prCheckoutCmd = &cobra.Command{ + Use: "checkout { | | }", + Short: "Check out a pull request in Git", + Args: func(cmd *cobra.Command, args []string) error { + if len(args) < 1 { + return errors.New("requires a PR number as an argument") + } + return nil + }, + RunE: prCheckout, +} diff --git a/command/pr_create.go b/command/pr_create.go index 5e2190cc0..7c1109b09 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -1,10 +1,8 @@ package command import ( - "errors" "fmt" "net/url" - "sort" "time" "github.com/cli/cli/api" @@ -29,7 +27,7 @@ func prCreate(cmd *cobra.Command, _ []string) error { } baseRepoOverride, _ := cmd.Flags().GetString("repo") - repoContext, err := resolveRemotesToRepos(remotes, client, baseRepoOverride) + repoContext, err := context.ResolveRemotesToRepos(remotes, client, baseRepoOverride) if err != nil { return err } @@ -178,12 +176,17 @@ func prCreate(cmd *cobra.Command, _ []string) error { return fmt.Errorf("pull request title must not be blank") } + headRefName := headBranch + if !ghrepo.IsSame(headRemote, baseRepo) { + headRefName = fmt.Sprintf("%s:%s", headRemote.RepoOwner(), headBranch) + } + params := map[string]interface{}{ "title": title, "body": body, "draft": isDraft, "baseRefName": baseBranch, - "headRefName": headBranch, + "headRefName": headRefName, } pr, err := api.CreatePullRequest(client, baseRepo, params) @@ -229,97 +232,3 @@ func init() { "The branch into which you want your code merged") prCreateCmd.Flags().BoolP("web", "w", false, "Open the web browser to create a pull request") } - -// cap the number of git remotes looked up, since the user might have an -// unusally large number of git remotes -const maxRemotesForLookup = 5 - -func resolveRemotesToRepos(remotes context.Remotes, client *api.Client, base string) (resolvedRemotes, error) { - sort.Stable(remotes) - lenRemotesForLookup := len(remotes) - if lenRemotesForLookup > maxRemotesForLookup { - lenRemotesForLookup = maxRemotesForLookup - } - - hasBaseOverride := base != "" - baseOverride := ghrepo.FromFullName(base) - foundBaseOverride := false - repos := []ghrepo.Interface{} - for _, r := range remotes[:lenRemotesForLookup] { - repos = append(repos, r) - if ghrepo.IsSame(r, baseOverride) { - foundBaseOverride = true - } - } - if hasBaseOverride && !foundBaseOverride { - // additionally, look up the explicitly specified base repo if it's not - // already covered by git remotes - repos = append(repos, baseOverride) - } - - result := resolvedRemotes{remotes: remotes} - if hasBaseOverride { - result.baseOverride = baseOverride - } - networkResult, err := api.RepoNetwork(client, repos) - if err != nil { - return result, err - } - result.network = networkResult - return result, nil -} - -type resolvedRemotes struct { - baseOverride ghrepo.Interface - remotes context.Remotes - network api.RepoNetworkResult -} - -// BaseRepo is the first found repository in the "upstream", "github", "origin" -// git remote order, resolved to the parent repo if the git remote points to a fork -func (r resolvedRemotes) BaseRepo() (*api.Repository, error) { - if r.baseOverride != nil { - for _, repo := range r.network.Repositories { - if repo != nil && ghrepo.IsSame(repo, r.baseOverride) { - return repo, nil - } - } - return nil, fmt.Errorf("failed looking up information about the '%s' repository", - ghrepo.FullName(r.baseOverride)) - } - - for _, repo := range r.network.Repositories { - if repo == nil { - continue - } - if repo.IsFork() { - return repo.Parent, nil - } - return repo, nil - } - - return nil, errors.New("not found") -} - -// HeadRepo is the first found repository that has push access -func (r resolvedRemotes) HeadRepo() (*api.Repository, error) { - for _, repo := range r.network.Repositories { - if repo != nil && repo.ViewerCanPush() { - return repo, nil - } - } - return nil, errors.New("none of the repositories have push access") -} - -// RemoteForRepo finds the git remote that points to a repository -func (r resolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (*context.Remote, error) { - for i, remote := range r.remotes { - if ghrepo.IsSame(remote, repo) || - // additionally, look up the resolved repository name in case this - // git remote points to this repository via a redirect - (r.network.Repositories[i] != nil && ghrepo.IsSame(r.network.Repositories[i], repo)) { - return remote, nil - } - } - return nil, errors.New("not found") -} diff --git a/command/pr_create_test.go b/command/pr_create_test.go index e6e8c370f..72cbdce94 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -10,10 +10,8 @@ import ( "strings" "testing" - "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/git" - "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/test" "github.com/cli/cli/utils" ) @@ -43,28 +41,9 @@ func TestPrCreateHelperProcess(*testing.T) { } func TestPRCreate(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("feature") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "feature") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" @@ -104,28 +83,9 @@ func TestPRCreate(t *testing.T) { } func TestPRCreate_web(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("feature") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "feature") http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") ranCommands := [][]string{} restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { @@ -146,28 +106,10 @@ func TestPRCreate_web(t *testing.T) { } func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { - ctx := context.NewBlank() - ctx.SetBranch("feature") - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) - initContext = func() context.Context { - return ctx - } + initBlankContext("OWNER/REPO", "feature") http := initFakeHTTP() - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "master", - "target": {"oid": "deadbeef"} - }, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" @@ -190,111 +132,85 @@ Creating pull request for feature into master in OWNER/REPO `) } +func TestPRCreate_cross_repo_same_branch(t *testing.T) { + ctx := context.NewBlank() + ctx.SetBranch("default") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + "fork": "MYSELF/REPO", + }) + initContext = func() context.Context { + return ctx + } + http := initFakeHTTP() + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repo_000": { + "id": "REPOID0", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "default", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "READ" + }, + "repo_001" : { + "parent": { + "id": "REPOID0", + "name": "REPO", + "owner": {"login": "OWNER"}, + "defaultBranchRef": { + "name": "default", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "READ" + }, + "id": "REPOID1", + "name": "REPO", + "owner": {"login": "MYSELF"}, + "defaultBranchRef": { + "name": "default", + "target": {"oid": "deadbeef"} + }, + "viewerPermission": "WRITE" + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "createPullRequest": { "pullRequest": { + "URL": "https://github.com/OWNER/REPO/pull/12" + } } } } + `)) -func Test_resolvedRemotes_clonedFork(t *testing.T) { - resolved := resolvedRemotes{ - baseOverride: nil, - remotes: context.Remotes{ - &context.Remote{ - Remote: &git.Remote{Name: "origin"}, - Owner: "OWNER", - Repo: "REPO", - }, - }, - network: api.RepoNetworkResult{ - Repositories: []*api.Repository{ - &api.Repository{ - Name: "REPO", - Owner: api.RepositoryOwner{Login: "OWNER"}, - ViewerPermission: "ADMIN", - Parent: &api.Repository{ - Name: "REPO", - Owner: api.RepositoryOwner{Login: "PARENTOWNER"}, - ViewerPermission: "READ", - }, - }, - }, - }, - } + origGitCommand := git.GitCommand + git.GitCommand = test.StubExecCommand("TestPrCreateHelperProcess", "clean") + defer func() { + git.GitCommand = origGitCommand + }() - baseRepo, err := resolved.BaseRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(baseRepo), "PARENTOWNER/REPO") - baseRemote, err := resolved.RemoteForRepo(baseRepo) - if baseRemote != nil || err == nil { - t.Error("did not expect any remote for base") - } + output, err := RunCommand(prCreateCmd, `pr create -t "cross repo" -b "same branch"`) + eq(t, err, nil) - headRepo, err := resolved.HeadRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(headRepo), "OWNER/REPO") - headRemote, err := resolved.RemoteForRepo(headRepo) - if err != nil { - t.Fatalf("got %v", err) - } - if headRemote.Name != "origin" { - t.Errorf("got remote %q", headRemote.Name) - } -} - -func Test_resolvedRemotes_triangularSetup(t *testing.T) { - resolved := resolvedRemotes{ - baseOverride: nil, - remotes: context.Remotes{ - &context.Remote{ - Remote: &git.Remote{Name: "origin"}, - Owner: "OWNER", - Repo: "REPO", - }, - &context.Remote{ - Remote: &git.Remote{Name: "fork"}, - Owner: "MYSELF", - Repo: "REPO", - }, - }, - network: api.RepoNetworkResult{ - Repositories: []*api.Repository{ - &api.Repository{ - Name: "NEWNAME", - Owner: api.RepositoryOwner{Login: "NEWOWNER"}, - ViewerPermission: "READ", - }, - &api.Repository{ - Name: "REPO", - Owner: api.RepositoryOwner{Login: "MYSELF"}, - ViewerPermission: "ADMIN", - }, - }, - }, - } - - baseRepo, err := resolved.BaseRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(baseRepo), "NEWOWNER/NEWNAME") - baseRemote, err := resolved.RemoteForRepo(baseRepo) - if err != nil { - t.Fatalf("got %v", err) - } - if baseRemote.Name != "origin" { - t.Errorf("got remote %q", baseRemote.Name) - } - - headRepo, err := resolved.HeadRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(headRepo), "MYSELF/REPO") - headRemote, err := resolved.RemoteForRepo(headRepo) - if err != nil { - t.Fatalf("got %v", err) - } - if headRemote.Name != "fork" { - t.Errorf("got remote %q", headRemote.Name) - } + bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) + reqBody := struct { + Variables struct { + Input struct { + RepositoryID string + Title string + Body string + BaseRefName string + HeadRefName string + } + } + }{} + json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.Input.RepositoryID, "REPOID0") + eq(t, reqBody.Variables.Input.Title, "cross repo") + eq(t, reqBody.Variables.Input.Body, "same branch") + eq(t, reqBody.Variables.Input.BaseRefName, "default") + eq(t, reqBody.Variables.Input.HeadRefName, "MYSELF:default") + + eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") + + // goal: only care that gql is formatted properly } diff --git a/command/pr_test.go b/command/pr_test.go index 3fbfb0b8e..d621d447b 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -73,6 +73,7 @@ func RunCommand(cmd *cobra.Command, args string) (*cmdOut, error) { func TestPRStatus(t *testing.T) { initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prStatus.json") defer jsonFile.Close() @@ -100,6 +101,7 @@ func TestPRStatus(t *testing.T) { func TestPRStatus_reviewsAndChecks(t *testing.T) { initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prStatusChecks.json") defer jsonFile.Close() @@ -111,9 +113,9 @@ func TestPRStatus_reviewsAndChecks(t *testing.T) { } expected := []string{ - "- Checks passing - changes requested", - "- Checks pending - approved", - "- 1/3 checks failing - review required", + "- Checks passing - Changes requested", + "- Checks pending - Approved", + "- 1/3 checks failing - Review required", } for _, line := range expected { @@ -126,6 +128,7 @@ func TestPRStatus_reviewsAndChecks(t *testing.T) { func TestPRStatus_blankSlate(t *testing.T) { initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": {} } @@ -157,6 +160,7 @@ Requesting a code review from you func TestPRList(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prList.json") defer jsonFile.Close() @@ -176,6 +180,7 @@ func TestPRList(t *testing.T) { func TestPRList_filtering(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) @@ -192,7 +197,7 @@ Pull requests for OWNER/REPO No pull requests match your search `) - bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) reqBody := struct { Variables struct { State []string @@ -208,6 +213,7 @@ No pull requests match your search func TestPRList_filteringAssignee(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) @@ -217,7 +223,7 @@ func TestPRList_filteringAssignee(t *testing.T) { t.Fatal(err) } - bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) reqBody := struct { Variables struct { Q string @@ -231,6 +237,7 @@ func TestPRList_filteringAssignee(t *testing.T) { func TestPRList_filteringAssigneeLabels(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") respBody := bytes.NewBufferString(`{ "data": {} }`) http.StubResponse(200, respBody) @@ -244,6 +251,7 @@ func TestPRList_filteringAssigneeLabels(t *testing.T) { func TestPRView_preview(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prViewPreview.json") defer jsonFile.Close() @@ -273,6 +281,7 @@ func TestPRView_preview(t *testing.T) { func TestPRView_previewCurrentBranch(t *testing.T) { initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prView.json") defer jsonFile.Close() @@ -304,9 +313,44 @@ func TestPRView_previewCurrentBranch(t *testing.T) { } } +func TestPRView_previewCurrentBranchWithEmptyBody(t *testing.T) { + initBlankContext("OWNER/REPO", "blueberries") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + jsonFile, _ := os.Open("../test/fixtures/prView_EmptyBody.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + return &outputStub{} + }) + defer restoreCmd() + + output, err := RunCommand(prViewCmd, "pr view -p") + if err != nil { + t.Errorf("error running command `pr view`: %v", err) + } + + eq(t, output.Stderr(), "") + + expectedLines := []*regexp.Regexp{ + regexp.MustCompile(`Blueberries are a good fruit`), + regexp.MustCompile(`nobody wants to merge 8 commits into master from blueberries`), + regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`), + } + for _, r := range expectedLines { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } + } +} + func TestPRView_currentBranch(t *testing.T) { initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prView.json") defer jsonFile.Close() @@ -344,6 +388,7 @@ func TestPRView_currentBranch(t *testing.T) { func TestPRView_noResultsForBranch(t *testing.T) { initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prView_NoActiveBranch.json") defer jsonFile.Close() @@ -374,6 +419,7 @@ func TestPRView_noResultsForBranch(t *testing.T) { func TestPRView_numberArg(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { @@ -405,6 +451,7 @@ func TestPRView_numberArg(t *testing.T) { func TestPRView_urlArg(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { @@ -436,6 +483,7 @@ func TestPRView_urlArg(t *testing.T) { func TestPRView_branchArg(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes": [ @@ -469,6 +517,7 @@ func TestPRView_branchArg(t *testing.T) { func TestPRView_branchWithOwnerArg(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes": [ diff --git a/command/root.go b/command/root.go index 1329569e4..4f9d5ba09 100644 --- a/command/root.go +++ b/command/root.go @@ -5,25 +5,37 @@ import ( "io" "os" "regexp" + "runtime/debug" "strings" "github.com/cli/cli/api" "github.com/cli/cli/context" + "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/utils" "github.com/spf13/cobra" ) -// Version is dynamically set at build time in the Makefile +// Version is dynamically set by the toolchain or overriden by the Makefile. var Version = "DEV" -// BuildDate is dynamically set at build time in the Makefile -var BuildDate = "YYYY-MM-DD" +// BuildDate is dynamically set at build time in the Makefile. +var BuildDate = "" // YYYY-MM-DD var versionOutput = "" func init() { - RootCmd.Version = fmt.Sprintf("%s (%s)", strings.TrimPrefix(Version, "v"), BuildDate) + if Version == "DEV" { + if info, ok := debug.ReadBuildInfo(); ok && info.Main.Version != "(devel)" { + Version = info.Main.Version + } + } + Version = strings.TrimPrefix(Version, "v") + if BuildDate == "" { + RootCmd.Version = Version + } else { + RootCmd.Version = fmt.Sprintf("%s (%s)", Version, BuildDate) + } versionOutput = fmt.Sprintf("gh version %s\n%s\n", RootCmd.Version, changelogURL(Version)) RootCmd.AddCommand(versionCmd) RootCmd.SetVersionTemplate(versionOutput) @@ -151,3 +163,33 @@ func changelogURL(version string) string { url := fmt.Sprintf("%s/releases/tag/v%s", path, strings.TrimPrefix(version, "v")) return url } + +func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (*ghrepo.Interface, error) { + apiClient, err := apiClientForContext(ctx) + if err != nil { + return nil, err + } + + baseOverride, err := cmd.Flags().GetString("repo") + if err != nil { + return nil, err + } + + remotes, err := ctx.Remotes() + if err != nil { + return nil, err + } + + repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, baseOverride) + if err != nil { + return nil, err + } + + var baseRepo ghrepo.Interface + baseRepo, err = repoContext.BaseRepo() + if err != nil { + return nil, err + } + + return &baseRepo, nil +} diff --git a/command/testing.go b/command/testing.go index 370ba7b53..206c8517d 100644 --- a/command/testing.go +++ b/command/testing.go @@ -12,6 +12,9 @@ func initBlankContext(repo, branch string) { ctx := context.NewBlank() ctx.SetBaseRepo(repo) ctx.SetBranch(branch) + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) return ctx } } diff --git a/context/config_setup.go b/context/config_setup.go index 4d91a4c62..6cb8ddb6c 100644 --- a/context/config_setup.go +++ b/context/config_setup.go @@ -18,10 +18,10 @@ const ( ) var ( - // The GitHub app that is meant for development - oauthClientID = "4d747ba5675d5d66553f" + // The "GitHub CLI" OAuth app + oauthClientID = "178c6fc778ccc68e1d6a" // This value is safe to be embedded in version control - oauthClientSecret = "d4fee7b3f9c2ef4284a5ca7be0ee200cf15b6f8d" + oauthClientSecret = "34ddeff2b558a23d38fba8a6de74f086ede1cc0b" ) // TODO: have a conversation about whether this belongs in the "context" package diff --git a/context/context.go b/context/context.go index 1ffd6bce6..fed589a8d 100644 --- a/context/context.go +++ b/context/context.go @@ -1,8 +1,12 @@ package context import ( + "errors" + "fmt" "path" + "sort" + "github.com/cli/cli/api" "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" "github.com/mitchellh/go-homedir" @@ -20,6 +24,100 @@ type Context interface { SetBaseRepo(string) } +// cap the number of git remotes looked up, since the user might have an +// unusally large number of git remotes +const maxRemotesForLookup = 5 + +func ResolveRemotesToRepos(remotes Remotes, client *api.Client, base string) (ResolvedRemotes, error) { + sort.Stable(remotes) + lenRemotesForLookup := len(remotes) + if lenRemotesForLookup > maxRemotesForLookup { + lenRemotesForLookup = maxRemotesForLookup + } + + hasBaseOverride := base != "" + baseOverride := ghrepo.FromFullName(base) + foundBaseOverride := false + repos := []ghrepo.Interface{} + for _, r := range remotes[:lenRemotesForLookup] { + repos = append(repos, r) + if ghrepo.IsSame(r, baseOverride) { + foundBaseOverride = true + } + } + if hasBaseOverride && !foundBaseOverride { + // additionally, look up the explicitly specified base repo if it's not + // already covered by git remotes + repos = append(repos, baseOverride) + } + + result := ResolvedRemotes{Remotes: remotes} + if hasBaseOverride { + result.BaseOverride = baseOverride + } + networkResult, err := api.RepoNetwork(client, repos) + if err != nil { + return result, err + } + result.Network = networkResult + return result, nil +} + +type ResolvedRemotes struct { + BaseOverride ghrepo.Interface + Remotes Remotes + Network api.RepoNetworkResult +} + +// BaseRepo is the first found repository in the "upstream", "github", "origin" +// git remote order, resolved to the parent repo if the git remote points to a fork +func (r ResolvedRemotes) BaseRepo() (*api.Repository, error) { + if r.BaseOverride != nil { + for _, repo := range r.Network.Repositories { + if repo != nil && ghrepo.IsSame(repo, r.BaseOverride) { + return repo, nil + } + } + return nil, fmt.Errorf("failed looking up information about the '%s' repository", + ghrepo.FullName(r.BaseOverride)) + } + + for _, repo := range r.Network.Repositories { + if repo == nil { + continue + } + if repo.IsFork() { + return repo.Parent, nil + } + return repo, nil + } + + return nil, errors.New("not found") +} + +// HeadRepo is the first found repository that has push access +func (r ResolvedRemotes) HeadRepo() (*api.Repository, error) { + for _, repo := range r.Network.Repositories { + if repo != nil && repo.ViewerCanPush() { + return repo, nil + } + } + return nil, errors.New("none of the repositories have push access") +} + +// RemoteForRepo finds the git remote that points to a repository +func (r ResolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (*Remote, error) { + for i, remote := range r.Remotes { + if ghrepo.IsSame(remote, repo) || + // additionally, look up the resolved repository name in case this + // git remote points to this repository via a redirect + (r.Network.Repositories[i] != nil && ghrepo.IsSame(r.Network.Repositories[i], repo)) { + return remote, nil + } + } + return nil, errors.New("not found") +} + // New initializes a Context that reads from the filesystem func New() Context { return &fsContext{} diff --git a/context/remote_test.go b/context/remote_test.go index 727ddf078..04ccbc56c 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -5,7 +5,9 @@ import ( "net/url" "testing" + "github.com/cli/cli/api" "github.com/cli/cli/git" + "github.com/cli/cli/internal/ghrepo" ) func Test_Remotes_FindByName(t *testing.T) { @@ -57,3 +59,111 @@ func Test_translateRemotes(t *testing.T) { t.Errorf("got %q", result[0].RepoName()) } } + +func Test_resolvedRemotes_triangularSetup(t *testing.T) { + resolved := ResolvedRemotes{ + BaseOverride: nil, + Remotes: Remotes{ + &Remote{ + Remote: &git.Remote{Name: "origin"}, + Owner: "OWNER", + Repo: "REPO", + }, + &Remote{ + Remote: &git.Remote{Name: "fork"}, + Owner: "MYSELF", + Repo: "REPO", + }, + }, + Network: api.RepoNetworkResult{ + Repositories: []*api.Repository{ + &api.Repository{ + Name: "NEWNAME", + Owner: api.RepositoryOwner{Login: "NEWOWNER"}, + ViewerPermission: "READ", + }, + &api.Repository{ + Name: "REPO", + Owner: api.RepositoryOwner{Login: "MYSELF"}, + ViewerPermission: "ADMIN", + }, + }, + }, + } + + baseRepo, err := resolved.BaseRepo() + if err != nil { + t.Fatalf("got %v", err) + } + eq(t, ghrepo.FullName(baseRepo), "NEWOWNER/NEWNAME") + baseRemote, err := resolved.RemoteForRepo(baseRepo) + if err != nil { + t.Fatalf("got %v", err) + } + if baseRemote.Name != "origin" { + t.Errorf("got remote %q", baseRemote.Name) + } + + headRepo, err := resolved.HeadRepo() + if err != nil { + t.Fatalf("got %v", err) + } + eq(t, ghrepo.FullName(headRepo), "MYSELF/REPO") + headRemote, err := resolved.RemoteForRepo(headRepo) + if err != nil { + t.Fatalf("got %v", err) + } + if headRemote.Name != "fork" { + t.Errorf("got remote %q", headRemote.Name) + } +} + +func Test_resolvedRemotes_clonedFork(t *testing.T) { + resolved := ResolvedRemotes{ + BaseOverride: nil, + Remotes: Remotes{ + &Remote{ + Remote: &git.Remote{Name: "origin"}, + Owner: "OWNER", + Repo: "REPO", + }, + }, + Network: api.RepoNetworkResult{ + Repositories: []*api.Repository{ + &api.Repository{ + Name: "REPO", + Owner: api.RepositoryOwner{Login: "OWNER"}, + ViewerPermission: "ADMIN", + Parent: &api.Repository{ + Name: "REPO", + Owner: api.RepositoryOwner{Login: "PARENTOWNER"}, + ViewerPermission: "READ", + }, + }, + }, + }, + } + + baseRepo, err := resolved.BaseRepo() + if err != nil { + t.Fatalf("got %v", err) + } + eq(t, ghrepo.FullName(baseRepo), "PARENTOWNER/REPO") + baseRemote, err := resolved.RemoteForRepo(baseRepo) + if baseRemote != nil || err == nil { + t.Error("did not expect any remote for base") + } + + headRepo, err := resolved.HeadRepo() + if err != nil { + t.Fatalf("got %v", err) + } + eq(t, ghrepo.FullName(headRepo), "OWNER/REPO") + headRemote, err := resolved.RemoteForRepo(headRepo) + if err != nil { + t.Fatalf("got %v", err) + } + if headRemote.Name != "origin" { + t.Errorf("got remote %q", headRemote.Name) + } +} diff --git a/git/remote.go b/git/remote.go index 0df077efb..f98606a2e 100644 --- a/git/remote.go +++ b/git/remote.go @@ -91,7 +91,7 @@ func AddRemote(name, initURL, finalURL string) (*Remote, error) { } } - finalURLParsed, err := url.Parse(initURL) + finalURLParsed, err := url.Parse(finalURL) if err != nil { return nil, err } diff --git a/pkg/surveyext/editor.go b/pkg/surveyext/editor.go index e49e9a6a7..8256e454d 100644 --- a/pkg/surveyext/editor.go +++ b/pkg/surveyext/editor.go @@ -25,11 +25,11 @@ var ( func init() { if runtime.GOOS == "windows" { editor = "notepad" - } - if v := os.Getenv("VISUAL"); v != "" { + } else if g := os.Getenv("GIT_EDITOR"); g != "" { + editor = g + } else if v := os.Getenv("VISUAL"); v != "" { editor = v - } - if e := os.Getenv("EDITOR"); e != "" { + } else if e := os.Getenv("EDITOR"); e != "" { editor = e } } diff --git a/source.md b/source.md index da27f29fe..75d1e1c5a 100644 --- a/source.md +++ b/source.md @@ -1,5 +1,11 @@ # Installation from source +0. Verify that you have Go 1.13+ installed +``` +$ go version +go version go1.13.7 +``` + 1. Clone cli into `~/.githubcli` ``` $ git clone https://github.com/cli/cli.git ~/.githubcli diff --git a/test/fixtures/prView_EmptyBody.json b/test/fixtures/prView_EmptyBody.json new file mode 100644 index 000000000..651f49ce0 --- /dev/null +++ b/test/fixtures/prView_EmptyBody.json @@ -0,0 +1,46 @@ +{ + "data": { + "repository": { + "pullRequests": { + "nodes": [ + { + "number": 12, + "title": "Blueberries are from a fork", + "body": "yeah", + "url": "https://github.com/OWNER/REPO/pull/12", + "headRefName": "blueberries", + "baseRefName": "master", + "headRepositoryOwner": { + "login": "hubot" + }, + "commits": { + "totalCount": 12 + }, + "author": { + "login": "nobody" + }, + "isCrossRepository": true + }, + { + "number": 10, + "title": "Blueberries are a good fruit", + "body": "", + "url": "https://github.com/OWNER/REPO/pull/10", + "baseRefName": "master", + "headRefName": "blueberries", + "author": { + "login": "nobody" + }, + "headRepositoryOwner": { + "login": "OWNER" + }, + "commits": { + "totalCount": 8 + }, + "isCrossRepository": false + } + ] + } + } + } +} diff --git a/update/update.go b/update/update.go index 42eb5955f..5a96bed56 100644 --- a/update/update.go +++ b/update/update.go @@ -21,7 +21,7 @@ type StateEntry struct { LatestRelease ReleaseInfo `yaml:"latest_release"` } -// CheckForUpdate checks whether this software has had a newer relase on GitHub +// CheckForUpdate checks whether this software has had a newer release on GitHub func CheckForUpdate(client *api.Client, stateFilePath, repo, currentVersion string) (*ReleaseInfo, error) { latestRelease, err := getLatestReleaseInfo(client, stateFilePath, repo, currentVersion) if err != nil { diff --git a/utils/table_printer.go b/utils/table_printer.go index 649e246e1..e1bf9bc36 100644 --- a/utils/table_printer.go +++ b/utils/table_printer.go @@ -176,7 +176,10 @@ func (t *tsvTablePrinter) Render() error { func truncate(maxLength int, title string) string { if len(title) > maxLength { - return title[0:maxLength-3] + "..." + if maxLength > 3 { + return title[0:maxLength-3] + "..." + } + return title[0:maxLength] } return title } diff --git a/utils/table_printer_test.go b/utils/table_printer_test.go new file mode 100644 index 000000000..2e3e4e803 --- /dev/null +++ b/utils/table_printer_test.go @@ -0,0 +1,31 @@ +package utils + +import ( + "bytes" + "testing" +) + +func Test_ttyTablePrinter_truncate(t *testing.T) { + buf := bytes.Buffer{} + tp := &ttyTablePrinter{ + out: &buf, + maxWidth: 5, + } + + tp.AddField("1", nil, nil) + tp.AddField("hello", nil, nil) + tp.EndRow() + tp.AddField("2", nil, nil) + tp.AddField("world", nil, nil) + tp.EndRow() + + err := tp.Render() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expected := "1 he\n2 wo\n" + if buf.String() != expected { + t.Errorf("expected: %q, got: %q", expected, buf.String()) + } +}