From d7465bdf3cb88a506a41c270cf4bcf3b51a4e8ff Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 8 Oct 2024 13:51:54 +0200 Subject: [PATCH 01/29] Initial testscript introduction --- acceptance/pr_test.go | 85 ++++++ acceptance/testdata/pr/pr-create-basic.txt | 24 ++ cmd/gh/main.go | 268 +---------------- go.mod | 2 + go.sum | 8 +- internal/ghcmd/cmd.go | 271 ++++++++++++++++++ .../ghcmd/cmd_test.go | 2 +- 7 files changed, 389 insertions(+), 271 deletions(-) create mode 100644 acceptance/pr_test.go create mode 100644 acceptance/testdata/pr/pr-create-basic.txt create mode 100644 internal/ghcmd/cmd.go rename cmd/gh/main_test.go => internal/ghcmd/cmd_test.go (99%) diff --git a/acceptance/pr_test.go b/acceptance/pr_test.go new file mode 100644 index 000000000..38f3e1f79 --- /dev/null +++ b/acceptance/pr_test.go @@ -0,0 +1,85 @@ +package acceptance_test + +import ( + "os" + "path" + "strings" + "testing" + + "math/rand" + + "github.com/cli/cli/v2/internal/ghcmd" + "github.com/rogpeppe/go-internal/testscript" +) + +func ghMain() int { + return int(ghcmd.Main()) +} + +func TestMain(m *testing.M) { + os.Exit(testscript.RunMain(m, map[string]func() int{ + "gh": ghMain, + })) +} + +func TestPullRequests(t *testing.T) { + testscript.Run(t, params("pr")) +} + +func params(dir string) testscript.Params { + return testscript.Params{ + Dir: path.Join("testdata", dir), + Files: []string{}, + Setup: sharedSetup, + Cmds: sharedCmds, + RequireExplicitExec: true, + RequireUniqueNames: true, + } +} + +var sharedSetup = func(ts *testscript.Env) error { + scriptName, ok := extractScriptName(ts.Vars) + if !ok { + ts.T().Fatal("script name not found") + } + ts.Setenv("SCRIPT_NAME", scriptName) + + ts.Setenv("HOME", ts.Cd) + ts.Setenv("GH_CONFIG_DIR", ts.Cd) + + ts.Setenv("GH_TOKEN", os.Getenv("GH_TOKEN")) + + ts.Setenv("ORG", os.Getenv("GH_ACCEPTANCE_ORG")) + + ts.Setenv("RANDOM_STRING", randomString(10)) + return nil +} + +var sharedCmds = map[string]func(ts *testscript.TestScript, neg bool, args []string){ + "defer": func(ts *testscript.TestScript, neg bool, args []string) { + ts.Defer(func() { + if err := ts.Exec(args[0], args[1:]...); err != nil { + ts.Fatalf("deferred command failed: %v", err) + } + }) + }} + +var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") + +func randomString(n int) string { + b := make([]rune, n) + for i := range b { + b[i] = letters[rand.Intn(len(letters))] + } + return string(b) +} + +func extractScriptName(vars []string) (string, bool) { + for _, kv := range vars { + if strings.HasPrefix(kv, "WORK=") { + v := strings.Split(kv, "=")[1] + return strings.CutPrefix(path.Base(v), "script-") + } + } + return "", false +} diff --git a/acceptance/testdata/pr/pr-create-basic.txt b/acceptance/testdata/pr/pr-create-basic.txt new file mode 100644 index 000000000..46d1de990 --- /dev/null +++ b/acceptance/testdata/pr/pr-create-basic.txt @@ -0,0 +1,24 @@ +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Clone the repo +exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Prepare a branch to PR +cd $SCRIPT_NAME-$RANDOM_STRING +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' + +# Check the PR is indeed created +exec gh pr list +stdout 'Feature Title' diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 0b9af0215..e167bc6f4 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -1,276 +1,12 @@ package main import ( - "context" - "errors" - "fmt" - "io" - "net" "os" - "os/exec" - "path/filepath" - "strings" - "time" - surveyCore "github.com/AlecAivazis/survey/v2/core" - "github.com/AlecAivazis/survey/v2/terminal" - "github.com/cli/cli/v2/api" - "github.com/cli/cli/v2/internal/build" - "github.com/cli/cli/v2/internal/config" - "github.com/cli/cli/v2/internal/config/migration" - "github.com/cli/cli/v2/internal/update" - "github.com/cli/cli/v2/pkg/cmd/factory" - "github.com/cli/cli/v2/pkg/cmd/root" - "github.com/cli/cli/v2/pkg/cmdutil" - "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/utils" - "github.com/cli/safeexec" - "github.com/mattn/go-isatty" - "github.com/mgutz/ansi" - "github.com/spf13/cobra" -) - -var updaterEnabled = "" - -type exitCode int - -const ( - exitOK exitCode = 0 - exitError exitCode = 1 - exitCancel exitCode = 2 - exitAuth exitCode = 4 - exitPending exitCode = 8 + "github.com/cli/cli/v2/internal/ghcmd" ) func main() { - code := mainRun() + code := ghcmd.Main() os.Exit(int(code)) } - -func mainRun() exitCode { - buildDate := build.Date - buildVersion := build.Version - hasDebug, _ := utils.IsDebugEnabled() - - cmdFactory := factory.New(buildVersion) - stderr := cmdFactory.IOStreams.ErrOut - - ctx := context.Background() - - if cfg, err := cmdFactory.Config(); err == nil { - var m migration.MultiAccount - if err := cfg.Migrate(m); err != nil { - fmt.Fprintln(stderr, err) - return exitError - } - } - - updateCtx, updateCancel := context.WithCancel(ctx) - defer updateCancel() - updateMessageChan := make(chan *update.ReleaseInfo) - go func() { - rel, err := checkForUpdate(updateCtx, cmdFactory, buildVersion) - if err != nil && hasDebug { - fmt.Fprintf(stderr, "warning: checking for update failed: %v", err) - } - updateMessageChan <- rel - }() - - if !cmdFactory.IOStreams.ColorEnabled() { - surveyCore.DisableColor = true - ansi.DisableColors(true) - } else { - // override survey's poor choice of color - surveyCore.TemplateFuncsWithColor["color"] = func(style string) string { - switch style { - case "white": - return ansi.ColorCode("default") - default: - return ansi.ColorCode(style) - } - } - } - - // Enable running gh from Windows File Explorer's address bar. Without this, the user is told to stop and run from a - // terminal. With this, a user can clone a repo (or take other actions) directly from explorer. - if len(os.Args) > 1 && os.Args[1] != "" { - cobra.MousetrapHelpText = "" - } - - rootCmd, err := root.NewCmdRoot(cmdFactory, buildVersion, buildDate) - if err != nil { - fmt.Fprintf(stderr, "failed to create root command: %s\n", err) - return exitError - } - - expandedArgs := []string{} - if len(os.Args) > 0 { - expandedArgs = os.Args[1:] - } - - // translate `gh help ` to `gh --help` for extensions. - if len(expandedArgs) >= 2 && expandedArgs[0] == "help" && isExtensionCommand(rootCmd, expandedArgs[1:]) { - expandedArgs = expandedArgs[1:] - expandedArgs = append(expandedArgs, "--help") - } - - rootCmd.SetArgs(expandedArgs) - - if cmd, err := rootCmd.ExecuteContextC(ctx); err != nil { - var pagerPipeError *iostreams.ErrClosedPagerPipe - var noResultsError cmdutil.NoResultsError - var extError *root.ExternalCommandExitError - var authError *root.AuthError - if err == cmdutil.SilentError { - return exitError - } else if err == cmdutil.PendingError { - return exitPending - } else if cmdutil.IsUserCancellation(err) { - if errors.Is(err, terminal.InterruptErr) { - // ensure the next shell prompt will start on its own line - fmt.Fprint(stderr, "\n") - } - return exitCancel - } else if errors.As(err, &authError) { - return exitAuth - } else if errors.As(err, &pagerPipeError) { - // ignore the error raised when piping to a closed pager - return exitOK - } else if errors.As(err, &noResultsError) { - if cmdFactory.IOStreams.IsStdoutTTY() { - fmt.Fprintln(stderr, noResultsError.Error()) - } - // no results is not a command failure - return exitOK - } else if errors.As(err, &extError) { - // pass on exit codes from extensions and shell aliases - return exitCode(extError.ExitCode()) - } - - printError(stderr, err, cmd, hasDebug) - - if strings.Contains(err.Error(), "Incorrect function") { - fmt.Fprintln(stderr, "You appear to be running in MinTTY without pseudo terminal support.") - fmt.Fprintln(stderr, "To learn about workarounds for this error, run: gh help mintty") - return exitError - } - - var httpErr api.HTTPError - if errors.As(err, &httpErr) && httpErr.StatusCode == 401 { - fmt.Fprintln(stderr, "Try authenticating with: gh auth login") - } else if u := factory.SSOURL(); u != "" { - // handles organization SAML enforcement error - fmt.Fprintf(stderr, "Authorize in your web browser: %s\n", u) - } else if msg := httpErr.ScopesSuggestion(); msg != "" { - fmt.Fprintln(stderr, msg) - } - - return exitError - } - if root.HasFailed() { - return exitError - } - - updateCancel() // if the update checker hasn't completed by now, abort it - newRelease := <-updateMessageChan - if newRelease != nil { - isHomebrew := isUnderHomebrew(cmdFactory.Executable()) - if isHomebrew && isRecentRelease(newRelease.PublishedAt) { - // do not notify Homebrew users before the version bump had a chance to get merged into homebrew-core - return exitOK - } - fmt.Fprintf(stderr, "\n\n%s %s → %s\n", - ansi.Color("A new release of gh is available:", "yellow"), - ansi.Color(strings.TrimPrefix(buildVersion, "v"), "cyan"), - ansi.Color(strings.TrimPrefix(newRelease.Version, "v"), "cyan")) - if isHomebrew { - fmt.Fprintf(stderr, "To upgrade, run: %s\n", "brew upgrade gh") - } - fmt.Fprintf(stderr, "%s\n\n", - ansi.Color(newRelease.URL, "yellow")) - } - - return exitOK -} - -// isExtensionCommand returns true if args resolve to an extension command. -func isExtensionCommand(rootCmd *cobra.Command, args []string) bool { - c, _, err := rootCmd.Find(args) - return err == nil && c != nil && c.GroupID == "extension" -} - -func printError(out io.Writer, err error, cmd *cobra.Command, debug bool) { - var dnsError *net.DNSError - if errors.As(err, &dnsError) { - fmt.Fprintf(out, "error connecting to %s\n", dnsError.Name) - if debug { - fmt.Fprintln(out, dnsError) - } - fmt.Fprintln(out, "check your internet connection or https://githubstatus.com") - return - } - - fmt.Fprintln(out, err) - - var flagError *cmdutil.FlagError - if errors.As(err, &flagError) || strings.HasPrefix(err.Error(), "unknown command ") { - if !strings.HasSuffix(err.Error(), "\n") { - fmt.Fprintln(out) - } - fmt.Fprintln(out, cmd.UsageString()) - } -} - -func shouldCheckForUpdate() bool { - if os.Getenv("GH_NO_UPDATE_NOTIFIER") != "" { - return false - } - if os.Getenv("CODESPACES") != "" { - return false - } - return updaterEnabled != "" && !isCI() && isTerminal(os.Stdout) && isTerminal(os.Stderr) -} - -func isTerminal(f *os.File) bool { - return isatty.IsTerminal(f.Fd()) || isatty.IsCygwinTerminal(f.Fd()) -} - -// based on https://github.com/watson/ci-info/blob/HEAD/index.js -func isCI() bool { - return os.Getenv("CI") != "" || // GitHub Actions, Travis CI, CircleCI, Cirrus CI, GitLab CI, AppVeyor, CodeShip, dsari - os.Getenv("BUILD_NUMBER") != "" || // Jenkins, TeamCity - os.Getenv("RUN_ID") != "" // TaskCluster, dsari -} - -func checkForUpdate(ctx context.Context, f *cmdutil.Factory, currentVersion string) (*update.ReleaseInfo, error) { - if !shouldCheckForUpdate() { - return nil, nil - } - httpClient, err := f.HttpClient() - if err != nil { - return nil, err - } - repo := updaterEnabled - stateFilePath := filepath.Join(config.StateDir(), "state.yml") - return update.CheckForUpdate(ctx, httpClient, stateFilePath, repo, currentVersion) -} - -func isRecentRelease(publishedAt time.Time) bool { - return !publishedAt.IsZero() && time.Since(publishedAt) < time.Hour*24 -} - -// Check whether the gh binary was found under the Homebrew prefix -func isUnderHomebrew(ghBinary string) bool { - brewExe, err := safeexec.LookPath("brew") - if err != nil { - return false - } - - brewPrefixBytes, err := exec.Command(brewExe, "--prefix").Output() - if err != nil { - return false - } - - brewBinPrefix := filepath.Join(strings.TrimSpace(string(brewPrefixBytes)), "bin") + string(filepath.Separator) - return strings.HasPrefix(ghBinary, brewBinPrefix) -} diff --git a/go.mod b/go.mod index e638459fa..42e3c84e6 100644 --- a/go.mod +++ b/go.mod @@ -125,6 +125,7 @@ require ( github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/rodaine/table v1.0.1 // indirect + github.com/rogpeppe/go-internal v1.13.1 github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/sagikazarmark/locafero v0.4.0 // indirect github.com/sagikazarmark/slog-shim v0.1.0 // indirect @@ -160,6 +161,7 @@ require ( golang.org/x/mod v0.20.0 // indirect golang.org/x/net v0.27.0 // indirect golang.org/x/sys v0.26.0 // indirect + golang.org/x/tools v0.22.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240520151616-dc85e6b867a5 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240520151616-dc85e6b867a5 // indirect gopkg.in/ini.v1 v1.67.0 // indirect diff --git a/go.sum b/go.sum index d1a85b60e..9a1fc23ee 100644 --- a/go.sum +++ b/go.sum @@ -366,8 +366,8 @@ github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= github.com/rodaine/table v1.0.1 h1:U/VwCnUxlVYxw8+NJiLIuCxA/xa6jL38MY3FYysVWWQ= github.com/rodaine/table v1.0.1/go.mod h1:UVEtfBsflpeEcD56nF4F5AocNFta0ZuolpSVdPtlmP4= -github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M= -github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA= +github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= +github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk= @@ -533,8 +533,8 @@ golang.org/x/time v0.5.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= -golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d h1:vU5i/LfpvrRCpgM/VPfJLg5KjxD3E+hfT1SH+d9zLwg= -golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d/go.mod h1:aiJjzUbINMkxbQROHiO6hDPo2LHcIPhhQsa9DLh0yGk= +golang.org/x/tools v0.22.0 h1:gqSGLZqv+AI9lIQzniJ0nZDRG5GBPsSi+DRNHWNz6yA= +golang.org/x/tools v0.22.0/go.mod h1:aCwcsjqvq7Yqt6TNyX7QMU2enbQ/Gt0bo6krSeEri+c= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.172.0 h1:/1OcMZGPmW1rX2LCu2CmGUD1KXK1+pfzxotxyRUCCdk= google.golang.org/api v0.172.0/go.mod h1:+fJZq6QXWfa9pXhnIzsjx4yI22d4aI9ZpLb58gvXjis= diff --git a/internal/ghcmd/cmd.go b/internal/ghcmd/cmd.go new file mode 100644 index 000000000..d5a674184 --- /dev/null +++ b/internal/ghcmd/cmd.go @@ -0,0 +1,271 @@ +package ghcmd + +import ( + "context" + "errors" + "fmt" + "io" + "net" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + surveyCore "github.com/AlecAivazis/survey/v2/core" + "github.com/AlecAivazis/survey/v2/terminal" + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/build" + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/config/migration" + "github.com/cli/cli/v2/internal/update" + "github.com/cli/cli/v2/pkg/cmd/factory" + "github.com/cli/cli/v2/pkg/cmd/root" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/utils" + "github.com/cli/safeexec" + "github.com/mattn/go-isatty" + "github.com/mgutz/ansi" + "github.com/spf13/cobra" +) + +var updaterEnabled = "" + +type exitCode int + +const ( + exitOK exitCode = 0 + exitError exitCode = 1 + exitCancel exitCode = 2 + exitAuth exitCode = 4 + exitPending exitCode = 8 +) + +func Main() exitCode { + buildDate := build.Date + buildVersion := build.Version + hasDebug, _ := utils.IsDebugEnabled() + + cmdFactory := factory.New(buildVersion) + stderr := cmdFactory.IOStreams.ErrOut + + ctx := context.Background() + + if cfg, err := cmdFactory.Config(); err == nil { + var m migration.MultiAccount + if err := cfg.Migrate(m); err != nil { + fmt.Fprintln(stderr, err) + return exitError + } + } + + updateCtx, updateCancel := context.WithCancel(ctx) + defer updateCancel() + updateMessageChan := make(chan *update.ReleaseInfo) + go func() { + rel, err := checkForUpdate(updateCtx, cmdFactory, buildVersion) + if err != nil && hasDebug { + fmt.Fprintf(stderr, "warning: checking for update failed: %v", err) + } + updateMessageChan <- rel + }() + + if !cmdFactory.IOStreams.ColorEnabled() { + surveyCore.DisableColor = true + ansi.DisableColors(true) + } else { + // override survey's poor choice of color + surveyCore.TemplateFuncsWithColor["color"] = func(style string) string { + switch style { + case "white": + return ansi.ColorCode("default") + default: + return ansi.ColorCode(style) + } + } + } + + // Enable running gh from Windows File Explorer's address bar. Without this, the user is told to stop and run from a + // terminal. With this, a user can clone a repo (or take other actions) directly from explorer. + if len(os.Args) > 1 && os.Args[1] != "" { + cobra.MousetrapHelpText = "" + } + + rootCmd, err := root.NewCmdRoot(cmdFactory, buildVersion, buildDate) + if err != nil { + fmt.Fprintf(stderr, "failed to create root command: %s\n", err) + return exitError + } + + expandedArgs := []string{} + if len(os.Args) > 0 { + expandedArgs = os.Args[1:] + } + + // translate `gh help ` to `gh --help` for extensions. + if len(expandedArgs) >= 2 && expandedArgs[0] == "help" && isExtensionCommand(rootCmd, expandedArgs[1:]) { + expandedArgs = expandedArgs[1:] + expandedArgs = append(expandedArgs, "--help") + } + + rootCmd.SetArgs(expandedArgs) + + if cmd, err := rootCmd.ExecuteContextC(ctx); err != nil { + var pagerPipeError *iostreams.ErrClosedPagerPipe + var noResultsError cmdutil.NoResultsError + var extError *root.ExternalCommandExitError + var authError *root.AuthError + if err == cmdutil.SilentError { + return exitError + } else if err == cmdutil.PendingError { + return exitPending + } else if cmdutil.IsUserCancellation(err) { + if errors.Is(err, terminal.InterruptErr) { + // ensure the next shell prompt will start on its own line + fmt.Fprint(stderr, "\n") + } + return exitCancel + } else if errors.As(err, &authError) { + return exitAuth + } else if errors.As(err, &pagerPipeError) { + // ignore the error raised when piping to a closed pager + return exitOK + } else if errors.As(err, &noResultsError) { + if cmdFactory.IOStreams.IsStdoutTTY() { + fmt.Fprintln(stderr, noResultsError.Error()) + } + // no results is not a command failure + return exitOK + } else if errors.As(err, &extError) { + // pass on exit codes from extensions and shell aliases + return exitCode(extError.ExitCode()) + } + + printError(stderr, err, cmd, hasDebug) + + if strings.Contains(err.Error(), "Incorrect function") { + fmt.Fprintln(stderr, "You appear to be running in MinTTY without pseudo terminal support.") + fmt.Fprintln(stderr, "To learn about workarounds for this error, run: gh help mintty") + return exitError + } + + var httpErr api.HTTPError + if errors.As(err, &httpErr) && httpErr.StatusCode == 401 { + fmt.Fprintln(stderr, "Try authenticating with: gh auth login") + } else if u := factory.SSOURL(); u != "" { + // handles organization SAML enforcement error + fmt.Fprintf(stderr, "Authorize in your web browser: %s\n", u) + } else if msg := httpErr.ScopesSuggestion(); msg != "" { + fmt.Fprintln(stderr, msg) + } + + return exitError + } + if root.HasFailed() { + return exitError + } + + updateCancel() // if the update checker hasn't completed by now, abort it + newRelease := <-updateMessageChan + if newRelease != nil { + isHomebrew := isUnderHomebrew(cmdFactory.Executable()) + if isHomebrew && isRecentRelease(newRelease.PublishedAt) { + // do not notify Homebrew users before the version bump had a chance to get merged into homebrew-core + return exitOK + } + fmt.Fprintf(stderr, "\n\n%s %s → %s\n", + ansi.Color("A new release of gh is available:", "yellow"), + ansi.Color(strings.TrimPrefix(buildVersion, "v"), "cyan"), + ansi.Color(strings.TrimPrefix(newRelease.Version, "v"), "cyan")) + if isHomebrew { + fmt.Fprintf(stderr, "To upgrade, run: %s\n", "brew upgrade gh") + } + fmt.Fprintf(stderr, "%s\n\n", + ansi.Color(newRelease.URL, "yellow")) + } + + return exitOK +} + +// isExtensionCommand returns true if args resolve to an extension command. +func isExtensionCommand(rootCmd *cobra.Command, args []string) bool { + c, _, err := rootCmd.Find(args) + return err == nil && c != nil && c.GroupID == "extension" +} + +func printError(out io.Writer, err error, cmd *cobra.Command, debug bool) { + var dnsError *net.DNSError + if errors.As(err, &dnsError) { + fmt.Fprintf(out, "error connecting to %s\n", dnsError.Name) + if debug { + fmt.Fprintln(out, dnsError) + } + fmt.Fprintln(out, "check your internet connection or https://githubstatus.com") + return + } + + fmt.Fprintln(out, err) + + var flagError *cmdutil.FlagError + if errors.As(err, &flagError) || strings.HasPrefix(err.Error(), "unknown command ") { + if !strings.HasSuffix(err.Error(), "\n") { + fmt.Fprintln(out) + } + fmt.Fprintln(out, cmd.UsageString()) + } +} + +func shouldCheckForUpdate() bool { + if os.Getenv("GH_NO_UPDATE_NOTIFIER") != "" { + return false + } + if os.Getenv("CODESPACES") != "" { + return false + } + return updaterEnabled != "" && !isCI() && isTerminal(os.Stdout) && isTerminal(os.Stderr) +} + +func isTerminal(f *os.File) bool { + return isatty.IsTerminal(f.Fd()) || isatty.IsCygwinTerminal(f.Fd()) +} + +// based on https://github.com/watson/ci-info/blob/HEAD/index.js +func isCI() bool { + return os.Getenv("CI") != "" || // GitHub Actions, Travis CI, CircleCI, Cirrus CI, GitLab CI, AppVeyor, CodeShip, dsari + os.Getenv("BUILD_NUMBER") != "" || // Jenkins, TeamCity + os.Getenv("RUN_ID") != "" // TaskCluster, dsari +} + +func checkForUpdate(ctx context.Context, f *cmdutil.Factory, currentVersion string) (*update.ReleaseInfo, error) { + if !shouldCheckForUpdate() { + return nil, nil + } + httpClient, err := f.HttpClient() + if err != nil { + return nil, err + } + repo := updaterEnabled + stateFilePath := filepath.Join(config.StateDir(), "state.yml") + return update.CheckForUpdate(ctx, httpClient, stateFilePath, repo, currentVersion) +} + +func isRecentRelease(publishedAt time.Time) bool { + return !publishedAt.IsZero() && time.Since(publishedAt) < time.Hour*24 +} + +// Check whether the gh binary was found under the Homebrew prefix +func isUnderHomebrew(ghBinary string) bool { + brewExe, err := safeexec.LookPath("brew") + if err != nil { + return false + } + + brewPrefixBytes, err := exec.Command(brewExe, "--prefix").Output() + if err != nil { + return false + } + + brewBinPrefix := filepath.Join(strings.TrimSpace(string(brewPrefixBytes)), "bin") + string(filepath.Separator) + return strings.HasPrefix(ghBinary, brewBinPrefix) +} diff --git a/cmd/gh/main_test.go b/internal/ghcmd/cmd_test.go similarity index 99% rename from cmd/gh/main_test.go rename to internal/ghcmd/cmd_test.go index 01552b2bd..08bbceb85 100644 --- a/cmd/gh/main_test.go +++ b/internal/ghcmd/cmd_test.go @@ -1,4 +1,4 @@ -package main +package ghcmd import ( "bytes" From 5ed237c65a9ca79afa8660447a18e653a518de4b Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 8 Oct 2024 14:14:31 +0200 Subject: [PATCH 02/29] Add pr view test script --- acceptance/testdata/pr/pr-view.txt | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 acceptance/testdata/pr/pr-view.txt diff --git a/acceptance/testdata/pr/pr-view.txt b/acceptance/testdata/pr/pr-view.txt new file mode 100644 index 000000000..01566edb8 --- /dev/null +++ b/acceptance/testdata/pr/pr-view.txt @@ -0,0 +1,24 @@ +# Setup git +exec gh auth setup-git + +# Create a repo +exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --private --add-readme + +# Defer cleanup +defer gh repo delete $ORG/$SCRIPT_NAME-$RANDOM_STRING --yes + +# Clone the repo +exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Prepare a branch for PR +cd $SCRIPT_NAME-$RANDOM_STRING +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty commit' +exec git push -u origin feature-branch + +# Create a PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' + +# View the PR (TODO: can we somehow get the stdout of the last command) +exec gh pr view +stdout 'Feature Title' From 37171628c6e7017442d9125270f74c5f9e24223c Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 8 Oct 2024 14:33:17 +0200 Subject: [PATCH 03/29] Acceptance test PR checkout --- acceptance/testdata/pr/pr-checkout.txt | 29 ++++++++++++++++++++++++++ acceptance/testdata/pr/pr-view.txt | 20 +++++++++--------- 2 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 acceptance/testdata/pr/pr-checkout.txt diff --git a/acceptance/testdata/pr/pr-checkout.txt b/acceptance/testdata/pr/pr-checkout.txt new file mode 100644 index 000000000..d02c42076 --- /dev/null +++ b/acceptance/testdata/pr/pr-checkout.txt @@ -0,0 +1,29 @@ +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Clone the repo +exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Prepare a branch to PR +cd $SCRIPT_NAME-$RANDOM_STRING +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' + +# Remove the local branch +exec git checkout main +exec git branch -D feature-branch +stdout 'Deleted branch feature-branch' + +# Checkout the PR +exec gh pr checkout 1 +stderr 'Switched to a new branch ''feature-branch''' diff --git a/acceptance/testdata/pr/pr-view.txt b/acceptance/testdata/pr/pr-view.txt index 01566edb8..c9dfcd3a6 100644 --- a/acceptance/testdata/pr/pr-view.txt +++ b/acceptance/testdata/pr/pr-view.txt @@ -1,24 +1,24 @@ -# Setup git +# Use gh as a credential helper exec gh auth setup-git -# Create a repo -exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --private --add-readme +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --add-readme --private -# Defer cleanup -defer gh repo delete $ORG/$SCRIPT_NAME-$RANDOM_STRING --yes +# Defer repo cleanup +defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING # Clone the repo exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING -# Prepare a branch for PR +# Prepare a branch to PR cd $SCRIPT_NAME-$RANDOM_STRING exec git checkout -b feature-branch -exec git commit --allow-empty -m 'Empty commit' +exec git commit --allow-empty -m 'Empty Commit' exec git push -u origin feature-branch -# Create a PR +# Create the PR exec gh pr create --title 'Feature Title' --body 'Feature Body' -# View the PR (TODO: can we somehow get the stdout of the last command) -exec gh pr view +# View the PR +exec gh pr view 1 stdout 'Feature Title' From 464a69ae87f8b36c40b429de0e9e283a74bf2897 Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 8 Oct 2024 15:12:13 +0200 Subject: [PATCH 04/29] Use stdout2env in PR acceptance tests --- acceptance/pr_test.go | 17 +++++++++++++---- acceptance/testdata/pr/pr-checkout.txt | 3 ++- acceptance/testdata/pr/pr-view.txt | 3 ++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/acceptance/pr_test.go b/acceptance/pr_test.go index 38f3e1f79..563526f6f 100644 --- a/acceptance/pr_test.go +++ b/acceptance/pr_test.go @@ -58,11 +58,20 @@ var sharedSetup = func(ts *testscript.Env) error { var sharedCmds = map[string]func(ts *testscript.TestScript, neg bool, args []string){ "defer": func(ts *testscript.TestScript, neg bool, args []string) { ts.Defer(func() { - if err := ts.Exec(args[0], args[1:]...); err != nil { - ts.Fatalf("deferred command failed: %v", err) - } + ts.Check(ts.Exec(args[0], args[1:]...)) }) - }} + }, + "stdout2env": func(ts *testscript.TestScript, neg bool, args []string) { + if neg { + ts.Fatalf("unsupported: ! stdout2env") + } + if len(args) != 1 { + ts.Fatalf("usage: stdout2env name") + } + + ts.Setenv(args[0], strings.TrimRight(ts.ReadFile("stdout"), "\n")) + }, +} var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") diff --git a/acceptance/testdata/pr/pr-checkout.txt b/acceptance/testdata/pr/pr-checkout.txt index d02c42076..4cfe96c1a 100644 --- a/acceptance/testdata/pr/pr-checkout.txt +++ b/acceptance/testdata/pr/pr-checkout.txt @@ -18,6 +18,7 @@ exec git push -u origin feature-branch # Create the PR exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout2env PR_URL # Remove the local branch exec git checkout main @@ -25,5 +26,5 @@ exec git branch -D feature-branch stdout 'Deleted branch feature-branch' # Checkout the PR -exec gh pr checkout 1 +exec gh pr checkout $PR_URL stderr 'Switched to a new branch ''feature-branch''' diff --git a/acceptance/testdata/pr/pr-view.txt b/acceptance/testdata/pr/pr-view.txt index c9dfcd3a6..6166d15ad 100644 --- a/acceptance/testdata/pr/pr-view.txt +++ b/acceptance/testdata/pr/pr-view.txt @@ -18,7 +18,8 @@ exec git push -u origin feature-branch # Create the PR exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout2env PR_URL # View the PR -exec gh pr view 1 +exec gh pr view $PR_URL stdout 'Feature Title' From 99f7e048f9b1d3642afbd01946e81dfecb5a6ace Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 8 Oct 2024 18:37:09 +0200 Subject: [PATCH 05/29] Support targeting other hosts in acceptance tests --- acceptance/{pr_test.go => acceptance_test.go} | 2 ++ 1 file changed, 2 insertions(+) rename acceptance/{pr_test.go => acceptance_test.go} (97%) diff --git a/acceptance/pr_test.go b/acceptance/acceptance_test.go similarity index 97% rename from acceptance/pr_test.go rename to acceptance/acceptance_test.go index 563526f6f..364d073b8 100644 --- a/acceptance/pr_test.go +++ b/acceptance/acceptance_test.go @@ -49,6 +49,8 @@ var sharedSetup = func(ts *testscript.Env) error { ts.Setenv("GH_TOKEN", os.Getenv("GH_TOKEN")) + ts.Setenv("GH_HOST", os.Getenv("GH_HOST")) + ts.Setenv("ORG", os.Getenv("GH_ACCEPTANCE_ORG")) ts.Setenv("RANDOM_STRING", randomString(10)) From eb9eeb10e13a824a108c7be30c631c307ed64800 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 11 Oct 2024 13:28:46 +0200 Subject: [PATCH 06/29] Use txtar extension for testscripts --- acceptance/testdata/pr/{pr-checkout.txt => pr-checkout.txtar} | 0 .../testdata/pr/{pr-create-basic.txt => pr-create-basic.txtar} | 0 acceptance/testdata/pr/{pr-view.txt => pr-view.txtar} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename acceptance/testdata/pr/{pr-checkout.txt => pr-checkout.txtar} (100%) rename acceptance/testdata/pr/{pr-create-basic.txt => pr-create-basic.txtar} (100%) rename acceptance/testdata/pr/{pr-view.txt => pr-view.txtar} (100%) diff --git a/acceptance/testdata/pr/pr-checkout.txt b/acceptance/testdata/pr/pr-checkout.txtar similarity index 100% rename from acceptance/testdata/pr/pr-checkout.txt rename to acceptance/testdata/pr/pr-checkout.txtar diff --git a/acceptance/testdata/pr/pr-create-basic.txt b/acceptance/testdata/pr/pr-create-basic.txtar similarity index 100% rename from acceptance/testdata/pr/pr-create-basic.txt rename to acceptance/testdata/pr/pr-create-basic.txtar diff --git a/acceptance/testdata/pr/pr-view.txt b/acceptance/testdata/pr/pr-view.txtar similarity index 100% rename from acceptance/testdata/pr/pr-view.txt rename to acceptance/testdata/pr/pr-view.txtar From 5745eb1c1c4acf4e21d380c0fa7d297f0490eddf Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 11 Oct 2024 13:49:28 +0200 Subject: [PATCH 07/29] Add initial acceptance test README --- acceptance/README.md | 46 +++++++++++++++++++++++++++++++++++ acceptance/acceptance_test.go | 2 ++ 2 files changed, 48 insertions(+) create mode 100644 acceptance/README.md diff --git a/acceptance/README.md b/acceptance/README.md new file mode 100644 index 000000000..132336d00 --- /dev/null +++ b/acceptance/README.md @@ -0,0 +1,46 @@ +## Acceptance Tests + +The acceptance tests are blackbox* tests that are expected to interact with resources on a real GitHub instance. + +*Note: they aren't strictly blackbox because `exec gh` commands delegate to a binary set up by `testscript` that calls into `ghcmd.Main`. However, since our real `func main` is an extremely thin adapter over `ghcmd.Main`, this is reasonable. This tradeoff avoids us building the binary ourselves for the tests, and allows us to get code coverage metrics. + +### Running the Acceptance Tests + +The acceptance tests currently require three environment variables to be set: + * `GH_HOST` + * `GH_ACCEPTANCE_ORG` + * `GH_TOKEN` + +While `GH_HOST` may not strictly be necessary because `gh` can choose a host, it is required to avoid ambiguity and unexpected results depending on the state of `gh`. + +The `GH_ACCEPTANCE_ORG` is an organization that the tests can manage resources in. + +The `GH_TOKEN` must already have the necessary scopes for each test, and must have permissions to act in the `GH_ACCEPTANCE_ORG`. See [Effective Test Authoring](#effective-test-authoring) for how tests must handle tokens without sufficient scopes. + +The acceptance tests have a build constraint of `//go:build acceptance`, this means that `go test ./...` will continue to work without any modifications. The `acceptance` tag must therefore be provided when running `go test`. + +A full example invocation can be found below: + +``` +GH_HOST= GH_ACCEPTANCE_ORG= GH_TOKEN= test -tags=acceptance ./acceptance +``` + +### Effective Test Authoring + +This section will likely extend over time. + +#### Test Isolation + +#### Scope Validation + +#### Limit Custom Commands + +... + +### Further Reading + +https://bitfieldconsulting.com/posts/test-scripts + +https://atlasgo.io/blog/2024/09/09/how-go-tests-go-test + +https://encore.dev/blog/testscript-hidden-testing-gem diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 364d073b8..ade3517af 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -1,3 +1,5 @@ +//go:build acceptance + package acceptance_test import ( From 502daff2a1e0a0144697e799dbbfef10b182c3b5 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 11 Oct 2024 15:07:32 +0200 Subject: [PATCH 08/29] Refactor acceptance test environment handling --- acceptance/README.md | 12 +++++++++ acceptance/acceptance_test.go | 50 +++++++++++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/acceptance/README.md b/acceptance/README.md index 132336d00..b8dacadfd 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -25,6 +25,18 @@ A full example invocation can be found below: GH_HOST= GH_ACCEPTANCE_ORG= GH_TOKEN= test -tags=acceptance ./acceptance ``` +### Acceptance Test VS Code Support + +Due to the `//go:build acceptance` build constraint, some functionality is limited because `gopls` isn't being informed about the tag. To resolve this, set the following in your `settings.json`: + +```json + "gopls": { + "buildFlags": [ + "-tags=acceptance" + ] + }, +``` + ### Effective Test Authoring This section will likely extend over time. diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index ade3517af..ab6432304 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -3,6 +3,7 @@ package acceptance_test import ( + "fmt" "os" "path" "strings" @@ -25,10 +26,16 @@ func TestMain(m *testing.M) { } func TestPullRequests(t *testing.T) { - testscript.Run(t, params("pr")) + var tsEnv testScriptEnv + if err := tsEnv.fromEnv(); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + + testscript.Run(t, testScriptParamsFor("pr")) } -func params(dir string) testscript.Params { +func testScriptParamsFor(dir string) testscript.Params { return testscript.Params{ Dir: path.Join("testdata", dir), Files: []string{}, @@ -96,3 +103,42 @@ func extractScriptName(vars []string) (string, bool) { } return "", false } + +type missingEnvError struct { + missingEnvs []string +} + +func (e missingEnvError) Error() string { + return fmt.Sprintf("missing environment variables: %s", strings.Join(e.missingEnvs, ", ")) +} + +type testScriptEnv struct { + host string + org string + token string +} + +func (e *testScriptEnv) fromEnv() error { + envMap := map[string]string{} + + var missingEnvs []string + for _, key := range []string{"GH_HOST", "GH_ACCEPTANCE_ORG", "GH_TOKEN"} { + val, ok := os.LookupEnv(key) + if !ok { + missingEnvs = append(missingEnvs, key) + continue + } + + envMap[key] = val + } + + if len(missingEnvs) > 0 { + return missingEnvError{missingEnvs: missingEnvs} + } + + e.host = envMap["GH_HOST"] + e.org = envMap["GH_ACCEPTANCE_ORG"] + e.token = envMap["GH_TOKEN"] + + return nil +} From 320c5aba4a783799d87d6101ce602851922dae0f Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 11 Oct 2024 15:27:31 +0200 Subject: [PATCH 09/29] Note syntax highlighting support for txtar files --- acceptance/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/acceptance/README.md b/acceptance/README.md index b8dacadfd..dada532ff 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -37,6 +37,8 @@ Due to the `//go:build acceptance` build constraint, some functionality is limit }, ``` +You can install the `txtar` or `vscode-testscript` extensions to get syntax highlighting. + ### Effective Test Authoring This section will likely extend over time. From 99a9b35410d28bd3f47cbd5ef98756c458227abd Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 11 Oct 2024 15:27:49 +0200 Subject: [PATCH 10/29] Acceptance test PR merge and rebase --- .../testdata/pr/pr-merge-merge-strategy.txtar | 45 +++++++++++++++++++ .../pr/pr-merge-rebase-strategy.txtar | 45 +++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 acceptance/testdata/pr/pr-merge-merge-strategy.txtar create mode 100644 acceptance/testdata/pr/pr-merge-rebase-strategy.txtar diff --git a/acceptance/testdata/pr/pr-merge-merge-strategy.txtar b/acceptance/testdata/pr/pr-merge-merge-strategy.txtar new file mode 100644 index 000000000..7acf6da2b --- /dev/null +++ b/acceptance/testdata/pr/pr-merge-merge-strategy.txtar @@ -0,0 +1,45 @@ +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Clone the repo +exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Prepare a branch to PR with a single file +cd $SCRIPT_NAME-$RANDOM_STRING +exec git checkout -b feature-branch +mv ../file.txt file.txt +exec git add . +exec git commit -m 'Add file.txt' +exec git push -u origin feature-branch + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout2env PR_URL + +# Check that the file doesn't exist on the main branch +exec git checkout main +! exists file.txt + +# Merge the PR +exec gh pr merge $PR_URL --merge + +# Check that the state of the PR is now merged +exec gh pr view $PR_URL +stdout 'state:\tMERGED' + +# Pull and check the file exists on the main branch +exec git pull -r +exists file.txt + +# And check we had a merge commit +exec git show HEAD +stdout 'Merge pull request #1' + +-- file.txt -- +Unimportant contents diff --git a/acceptance/testdata/pr/pr-merge-rebase-strategy.txtar b/acceptance/testdata/pr/pr-merge-rebase-strategy.txtar new file mode 100644 index 000000000..73acd238c --- /dev/null +++ b/acceptance/testdata/pr/pr-merge-rebase-strategy.txtar @@ -0,0 +1,45 @@ +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Clone the repo +exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Prepare a branch to PR with a single file +cd $SCRIPT_NAME-$RANDOM_STRING +exec git checkout -b feature-branch +mv ../file.txt file.txt +exec git add . +exec git commit -m 'Add file.txt' +exec git push -u origin feature-branch + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout2env PR_URL + +# Check that the file doesn't exist on the main branch +exec git checkout main +! exists file.txt + +# Merge the PR +exec gh pr merge $PR_URL --rebase + +# Check that the state of the PR is now merged +exec gh pr view $PR_URL +stdout 'state:\tMERGED' + +# Pull and check the file exists on the main branch +exec git pull -r +exists file.txt + +# And check our commit was rebased +exec git show HEAD +stdout 'Add file.txt' + +-- file.txt -- +Unimportant contents From ee5709a4b6fa9b80acf52ec216151826a6ca3326 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 11 Oct 2024 15:35:31 +0200 Subject: [PATCH 11/29] Acceptance test PR comment --- acceptance/testdata/pr/pr-comment.txtar | 28 +++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 acceptance/testdata/pr/pr-comment.txtar diff --git a/acceptance/testdata/pr/pr-comment.txtar b/acceptance/testdata/pr/pr-comment.txtar new file mode 100644 index 000000000..2c4d488b5 --- /dev/null +++ b/acceptance/testdata/pr/pr-comment.txtar @@ -0,0 +1,28 @@ +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Clone the repo +exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Prepare a branch to PR +cd $SCRIPT_NAME-$RANDOM_STRING +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' +stdout2env PR_URL + +# Comment on the PR +exec gh pr comment $PR_URL --body 'Looks like a great feature!' + +# View the PR +exec gh pr view $PR_URL --comments +stdout 'Looks like a great feature!' From 64f5b3c2060223c07e6314d24717ce47ca875943 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 11 Oct 2024 15:57:21 +0200 Subject: [PATCH 12/29] Add Debug and Authoring sections to Acceptance README --- acceptance/README.md | 65 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/acceptance/README.md b/acceptance/README.md index dada532ff..0105c8891 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -39,17 +39,74 @@ Due to the `//go:build acceptance` build constraint, some functionality is limit You can install the `txtar` or `vscode-testscript` extensions to get syntax highlighting. +### Debugging Tests + +When tests fail they fail like this: + +``` +➜ go test -tags=acceptance ./acceptance +--- FAIL: TestPullRequests (0.00s) + --- FAIL: TestPullRequests/pr-merge (11.07s) + testscript.go:584: WORK=/private/var/folders/45/sdnm1hp10nj1s9q57dp3bc5h0000gn/T/go-test-script2778137936/script-pr-merge + # Use gh as a credential helper (0.693s) + # Create a repository with a file so it has a default branch (1.155s) + # Defer repo cleanup (0.000s) + # Clone the repo (1.551s) + # Prepare a branch to PR with a single file (1.168s) + # Create the PR (1.903s) + # Check that the file doesn't exist on the main branch (0.059s) + # Merge the PR (2.426s) + # Check that the state of the PR is now merged (0.571s) + # Pull and check the file exists on the main branch (1.074s) + # And check we had a merge commit (0.462s) + > exec git show HEAD + [stdout] + commit 85d32c1a83ace270f6754c61f3f7e14956be0a47 + Author: William Martin + Date: Fri Oct 11 15:23:56 2024 +0200 + + Add file.txt + + diff --git a/file.txt b/file.txt + new file mode 100644 + index 0000000..7449899 + --- /dev/null + +++ b/file.txt + @@ -0,0 +1 @@ + +Unimportant contents + > stdout 'Merge pull request #1' + FAIL: testdata/pr/pr-merge.txtar:42: no match for `Merge pull request #1` found in stdout +``` + +This is generally enough information to understand why a test has failed. However, we can get more information by providing the `-v` flag to `go test`, which turns on verbose mode and shows each command and any associated `stdio`. + +> [!WARNING] +> Verbose mode dumps the `testscript` environment variables, including the `GH_TOKEN`, so be careful. + +Finally, the `testscript.Params` struct has a `TestWork` field; when `TestWork` is set to true, the `WORK` directory will not be cleaned up, allowing for manual investigation of state in the shell. However, `defer` statements will still be run. + +TODO: Probably we want to add a `NO_CLEANUP` flag so `defer` is not run. Maybe this implies we want `defer` to be called `cleanup`? It might not be so clear in its intent though. Maybe `NO_CLEANUP_WORK` and `NO_DEFER`? + ### Effective Test Authoring -This section will likely extend over time. +This section is to be expanded over time as we write more tests and learn more. #### Test Isolation +The `testscript` library creates a somewhat isolated environment for each script. Each script gets a directory with limited environment variables by default. As far as reasonable, we should look to write scripts that depend on nothing more than themselves, the GitHub resources they manage, and limited additional environmental injection from our own `testscript` setup. + +Here are some guidelines around test isolation: + * Favour duplication in test setup over abstracting a new `testscript` command + * Favour a `testscript` owning an entire resource lifecycle over shared resource until we see a performance or rate limiting issue + * Use the `RANDOM_STRING` env var for globally visible resources to avoid conflicts + +### Debris + +Since these scripts are creating resources on a GitHub instance, we should try our best to cleanup after them. Use the `defer` keyword to ensure a command runs at the end of a test even in the case of failure. + #### Scope Validation -#### Limit Custom Commands - -... +TODO: I believe tests should early exit if the correct scopes aren't in place to execute the entire lifecycle. It's extremely annoying if a `defer` fails to clean up resources because there's no `delete_repo` scope for example. However, I'm not sure yet whether this scope checking should be in the Go tests or in the scripts themselves. It seems very cool to understand required scopes for a script just by looking at the script itself. ### Further Reading From dc7c66c1420bd7482f9d87d18f4e4266443ef3ec Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 11 Oct 2024 16:01:02 +0200 Subject: [PATCH 13/29] Add Writing Tests section to Acceptance README --- acceptance/README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/acceptance/README.md b/acceptance/README.md index 0105c8891..1f43bdc6a 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -25,6 +25,20 @@ A full example invocation can be found below: GH_HOST= GH_ACCEPTANCE_ORG= GH_TOKEN= test -tags=acceptance ./acceptance ``` +### Writing Tests + +This section is to be expanded over time as we write more tests and learn more. + +#### Environment Variables + +The following custom environment variables are made available to the scripts: + * `GH_TOKEN`: Set to the value of the `GH_TOKEN` env var provided to `go test` + * `ORG`: Set to the value of the `GH_ACCEPTANCE_ORG` env var provided to `go test` + * `RANDOM_STRING`: Set to a length 10 random string of letters to help isolate globally visible resources + * `SCRIPT_NAME`: Set to the name of the `testscript` currently running, without extension e.g. `pr-view` + * `HOME`: Set to the initial working directory. Required for `git` operations + * `GH_CONFIG_DIR`: Set to the initial working directory. Required for `gh` operations + ### Acceptance Test VS Code Support Due to the `//go:build acceptance` build constraint, some functionality is limited because `gopls` isn't being informed about the tag. To resolve this, set the following in your `settings.json`: From 9d569b3c118c7569e114c04727b0a4bd07fed6fc Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 11 Oct 2024 16:25:36 +0200 Subject: [PATCH 14/29] Isolate acceptance env vars --- acceptance/README.md | 32 +++++++++++++--------- acceptance/acceptance_test.go | 51 +++++++++++++++++++---------------- 2 files changed, 47 insertions(+), 36 deletions(-) diff --git a/acceptance/README.md b/acceptance/README.md index 1f43bdc6a..7d39cebba 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -6,23 +6,28 @@ The acceptance tests are blackbox* tests that are expected to interact with reso ### Running the Acceptance Tests -The acceptance tests currently require three environment variables to be set: - * `GH_HOST` - * `GH_ACCEPTANCE_ORG` - * `GH_TOKEN` - -While `GH_HOST` may not strictly be necessary because `gh` can choose a host, it is required to avoid ambiguity and unexpected results depending on the state of `gh`. - -The `GH_ACCEPTANCE_ORG` is an organization that the tests can manage resources in. - -The `GH_TOKEN` must already have the necessary scopes for each test, and must have permissions to act in the `GH_ACCEPTANCE_ORG`. See [Effective Test Authoring](#effective-test-authoring) for how tests must handle tokens without sufficient scopes. - The acceptance tests have a build constraint of `//go:build acceptance`, this means that `go test ./...` will continue to work without any modifications. The `acceptance` tag must therefore be provided when running `go test`. +The following environment variables are required: + +#### `GH_ACCEPTANCE_HOST` + +The GitHub host to target e.g. `github.com` + +#### `GH_ACCEPTANCE_ORG` + +The organization in which the acceptance tests can manage resources in. + +#### `GH_ACCEPTANCE_TOKEN` + +The token to use for authenticatin with the `GH_HOST`. This must already have the necessary scopes for each test, and must have permissions to act in the `GH_ACCEPTANCE_ORG`. See [Effective Test Authoring](#effective-test-authoring) for how tests must handle tokens without sufficient scopes. + +--- + A full example invocation can be found below: ``` -GH_HOST= GH_ACCEPTANCE_ORG= GH_TOKEN= test -tags=acceptance ./acceptance +GH_ACCEPTANCE_HOST= GH_ACCEPTANCE_ORG= GH_ACCEPTANCE_TOKEN= test -tags=acceptance ./acceptance ``` ### Writing Tests @@ -32,8 +37,9 @@ This section is to be expanded over time as we write more tests and learn more. #### Environment Variables The following custom environment variables are made available to the scripts: - * `GH_TOKEN`: Set to the value of the `GH_TOKEN` env var provided to `go test` + * `GH_HOST`: Set to value of the `GH_ACCEPTANCE_ORG` env var provided to `go test` * `ORG`: Set to the value of the `GH_ACCEPTANCE_ORG` env var provided to `go test` + * `GH_TOKEN`: Set to the value of the `GH_ACCEPTANCE_TOKEN` env var provided to `go test` * `RANDOM_STRING`: Set to a length 10 random string of letters to help isolate globally visible resources * `SCRIPT_NAME`: Set to the name of the `testscript` currently running, without extension e.g. `pr-view` * `HOME`: Set to the initial working directory. Required for `git` operations diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index ab6432304..fdad1e576 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -32,38 +32,37 @@ func TestPullRequests(t *testing.T) { os.Exit(1) } - testscript.Run(t, testScriptParamsFor("pr")) + testscript.Run(t, testScriptParamsFor(tsEnv, "pr")) } -func testScriptParamsFor(dir string) testscript.Params { +func testScriptParamsFor(tsEnv testScriptEnv, dir string) testscript.Params { return testscript.Params{ Dir: path.Join("testdata", dir), Files: []string{}, - Setup: sharedSetup, + Setup: sharedSetup(tsEnv), Cmds: sharedCmds, RequireExplicitExec: true, RequireUniqueNames: true, } } -var sharedSetup = func(ts *testscript.Env) error { - scriptName, ok := extractScriptName(ts.Vars) - if !ok { - ts.T().Fatal("script name not found") +func sharedSetup(tsEnv testScriptEnv) func(ts *testscript.Env) error { + return func(ts *testscript.Env) error { + scriptName, ok := extractScriptName(ts.Vars) + if !ok { + ts.T().Fatal("script name not found") + } + ts.Setenv("SCRIPT_NAME", scriptName) + ts.Setenv("HOME", ts.Cd) + ts.Setenv("GH_CONFIG_DIR", ts.Cd) + + ts.Setenv("GH_HOST", tsEnv.host) + ts.Setenv("ORG", tsEnv.org) + ts.Setenv("GH_TOKEN", tsEnv.token) + + ts.Setenv("RANDOM_STRING", randomString(10)) + return nil } - ts.Setenv("SCRIPT_NAME", scriptName) - - ts.Setenv("HOME", ts.Cd) - ts.Setenv("GH_CONFIG_DIR", ts.Cd) - - ts.Setenv("GH_TOKEN", os.Getenv("GH_TOKEN")) - - ts.Setenv("GH_HOST", os.Getenv("GH_HOST")) - - ts.Setenv("ORG", os.Getenv("GH_ACCEPTANCE_ORG")) - - ts.Setenv("RANDOM_STRING", randomString(10)) - return nil } var sharedCmds = map[string]func(ts *testscript.TestScript, neg bool, args []string){ @@ -121,8 +120,14 @@ type testScriptEnv struct { func (e *testScriptEnv) fromEnv() error { envMap := map[string]string{} + requiredEnvVars := []string{ + "GH_ACCEPTANCE_HOST", + "GH_ACCEPTANCE_ORG", + "GH_ACCEPTANCE_TOKEN", + } + var missingEnvs []string - for _, key := range []string{"GH_HOST", "GH_ACCEPTANCE_ORG", "GH_TOKEN"} { + for _, key := range requiredEnvVars { val, ok := os.LookupEnv(key) if !ok { missingEnvs = append(missingEnvs, key) @@ -136,9 +141,9 @@ func (e *testScriptEnv) fromEnv() error { return missingEnvError{missingEnvs: missingEnvs} } - e.host = envMap["GH_HOST"] + e.host = envMap["GH_ACCEPTANCE_HOST"] e.org = envMap["GH_ACCEPTANCE_ORG"] - e.token = envMap["GH_TOKEN"] + e.token = envMap["GH_ACCEPTANCE_TOKEN"] return nil } From f9b24990d6a91f71cd89f5c14ed530692be64864 Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 11 Oct 2024 16:27:40 +0200 Subject: [PATCH 15/29] Add codecoverage to Acceptance README --- acceptance/README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/acceptance/README.md b/acceptance/README.md index 7d39cebba..13cbfe86e 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -30,6 +30,14 @@ A full example invocation can be found below: GH_ACCEPTANCE_HOST= GH_ACCEPTANCE_ORG= GH_ACCEPTANCE_TOKEN= test -tags=acceptance ./acceptance ``` +#### Code Coverage + +To get code coverage, `go test` can be invoked with `coverpkg` and `coverprofile` like so: + +``` +GH_ACCEPTANCE_HOST= GH_ACCEPTANCE_ORG= GH_ACCEPTANCE_TOKEN= test -tags=acceptance -coverprofile=coverage.out -coverpkg=./... ./acceptance +``` + ### Writing Tests This section is to be expanded over time as we write more tests and learn more. From fd665555109c457ba932a72095c1089ce41c688e Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 11 Oct 2024 17:51:17 +0200 Subject: [PATCH 16/29] Error if acceptance tests are targeting github or cli orgs --- acceptance/acceptance_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index fdad1e576..7b249aaca 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -141,6 +141,10 @@ func (e *testScriptEnv) fromEnv() error { return missingEnvError{missingEnvs: missingEnvs} } + if envMap["GH_ACCEPTANCE_ORG"] == "github" || envMap["GH_ACCEPTANCE_ORG"] == "cli" { + return fmt.Errorf("GH_ACCEPTANCE_ORG cannot be 'github' or 'cli'") + } + e.host = envMap["GH_ACCEPTANCE_HOST"] e.org = envMap["GH_ACCEPTANCE_ORG"] e.token = envMap["GH_ACCEPTANCE_TOKEN"] From 846a39d7be91b709025fee38e03d347a694feb8f Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 11 Oct 2024 17:57:08 +0200 Subject: [PATCH 17/29] Add go to test instructions in Acceptance README --- acceptance/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/acceptance/README.md b/acceptance/README.md index 13cbfe86e..9828bfe74 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -27,7 +27,7 @@ The token to use for authenticatin with the `GH_HOST`. This must already have th A full example invocation can be found below: ``` -GH_ACCEPTANCE_HOST= GH_ACCEPTANCE_ORG= GH_ACCEPTANCE_TOKEN= test -tags=acceptance ./acceptance +GH_ACCEPTANCE_HOST= GH_ACCEPTANCE_ORG= GH_ACCEPTANCE_TOKEN= go test -tags=acceptance ./acceptance ``` #### Code Coverage @@ -35,7 +35,7 @@ GH_ACCEPTANCE_HOST= GH_ACCEPTANCE_ORG= GH_ACCEPTANCE_TOKEN= te To get code coverage, `go test` can be invoked with `coverpkg` and `coverprofile` like so: ``` -GH_ACCEPTANCE_HOST= GH_ACCEPTANCE_ORG= GH_ACCEPTANCE_TOKEN= test -tags=acceptance -coverprofile=coverage.out -coverpkg=./... ./acceptance +GH_ACCEPTANCE_HOST= GH_ACCEPTANCE_ORG= GH_ACCEPTANCE_TOKEN= go test -tags=acceptance -coverprofile=coverage.out -coverpkg=./... ./acceptance ``` ### Writing Tests From 0d7ec4489593973eda7142319ffe90646dd9ee7b Mon Sep 17 00:00:00 2001 From: William Martin Date: Fri, 11 Oct 2024 18:08:42 +0200 Subject: [PATCH 18/29] Validate required env vars not-empty for Acceptance tests --- acceptance/acceptance_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 7b249aaca..6a5e32ff7 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -108,7 +108,7 @@ type missingEnvError struct { } func (e missingEnvError) Error() string { - return fmt.Sprintf("missing environment variables: %s", strings.Join(e.missingEnvs, ", ")) + return fmt.Sprintf("environment variables %s must be set and non-empty", strings.Join(e.missingEnvs, ", ")) } type testScriptEnv struct { @@ -129,7 +129,7 @@ func (e *testScriptEnv) fromEnv() error { var missingEnvs []string for _, key := range requiredEnvVars { val, ok := os.LookupEnv(key) - if !ok { + if val == "" || !ok { missingEnvs = append(missingEnvs, key) continue } From 503659f11cd6b8c1e1a8ff03e1b680d662664bbd Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 14 Oct 2024 12:56:27 +0200 Subject: [PATCH 19/29] Add host recommendation to Acceptance test docs --- acceptance/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acceptance/README.md b/acceptance/README.md index 9828bfe74..07e51f688 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -16,7 +16,7 @@ The GitHub host to target e.g. `github.com` #### `GH_ACCEPTANCE_ORG` -The organization in which the acceptance tests can manage resources in. +The organization in which the acceptance tests can manage resources in. Consider using `gh-acceptance-testing` on `github.com`. #### `GH_ACCEPTANCE_TOKEN` From fbc72fd2befda6818105a916bbdd67fc54e3e987 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 14 Oct 2024 13:01:13 +0200 Subject: [PATCH 20/29] Suggest using legacy PAT for acceptance tests --- acceptance/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/acceptance/README.md b/acceptance/README.md index 07e51f688..cc3af0f98 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -22,6 +22,8 @@ The organization in which the acceptance tests can manage resources in. Consider The token to use for authenticatin with the `GH_HOST`. This must already have the necessary scopes for each test, and must have permissions to act in the `GH_ACCEPTANCE_ORG`. See [Effective Test Authoring](#effective-test-authoring) for how tests must handle tokens without sufficient scopes. +It's recommended to create and use a Legacy PAT for this; Fine-Grained PATs do not offer all the necessary privileges required. You can use an OAuth token provided via `gh auth login --web` and can provide it to the acceptance tests via `GH_ACCEPTANCE_TOKEN=$(gh auth token --hostname )` but this can be a bit confusing and annoying if you `gh auth login` again without `-s` and lose the required scopes. + --- A full example invocation can be found below: From 4d986aaed44fea6ca42772c6736e8091f60eea91 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 14 Oct 2024 13:54:31 +0200 Subject: [PATCH 21/29] Acceptance test PR creation with metadata --- .../testdata/pr/pr-create-with-metadata.txtar | 26 +++++++++++++++++++ .../testdata/pr/pr-merge-merge-strategy.txtar | 2 +- .../pr/pr-merge-rebase-strategy.txtar | 2 +- 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 acceptance/testdata/pr/pr-create-with-metadata.txtar diff --git a/acceptance/testdata/pr/pr-create-with-metadata.txtar b/acceptance/testdata/pr/pr-create-with-metadata.txtar new file mode 100644 index 000000000..765c84b67 --- /dev/null +++ b/acceptance/testdata/pr/pr-create-with-metadata.txtar @@ -0,0 +1,26 @@ +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Clone the repo +exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Prepare a branch to PR +cd $SCRIPT_NAME-$RANDOM_STRING +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' --assignee '@me' --label 'bug' +stdout2env PR_URL + +# Check the PR is indeed created +exec gh pr view $PR_URL +stdout 'assignees:\t.*$' +stdout 'labels:\tbug$' diff --git a/acceptance/testdata/pr/pr-merge-merge-strategy.txtar b/acceptance/testdata/pr/pr-merge-merge-strategy.txtar index 7acf6da2b..1d8355506 100644 --- a/acceptance/testdata/pr/pr-merge-merge-strategy.txtar +++ b/acceptance/testdata/pr/pr-merge-merge-strategy.txtar @@ -31,7 +31,7 @@ exec gh pr merge $PR_URL --merge # Check that the state of the PR is now merged exec gh pr view $PR_URL -stdout 'state:\tMERGED' +stdout 'state:\tMERGED$' # Pull and check the file exists on the main branch exec git pull -r diff --git a/acceptance/testdata/pr/pr-merge-rebase-strategy.txtar b/acceptance/testdata/pr/pr-merge-rebase-strategy.txtar index 73acd238c..f26338c4a 100644 --- a/acceptance/testdata/pr/pr-merge-rebase-strategy.txtar +++ b/acceptance/testdata/pr/pr-merge-rebase-strategy.txtar @@ -31,7 +31,7 @@ exec gh pr merge $PR_URL --rebase # Check that the state of the PR is now merged exec gh pr view $PR_URL -stdout 'state:\tMERGED' +stdout 'state:\tMERGED$' # Pull and check the file exists on the main branch exec git pull -r From bfa5b6afa5bb1f39990dda9ddbb03e639cdfd8ab Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 14 Oct 2024 13:59:55 +0200 Subject: [PATCH 22/29] Support skipping Acceptance test cleanup --- acceptance/README.md | 4 +-- acceptance/acceptance_test.go | 49 +++++++++++++++++++++++------------ 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/acceptance/README.md b/acceptance/README.md index cc3af0f98..1311704d4 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -113,9 +113,7 @@ This is generally enough information to understand why a test has failed. Howeve > [!WARNING] > Verbose mode dumps the `testscript` environment variables, including the `GH_TOKEN`, so be careful. -Finally, the `testscript.Params` struct has a `TestWork` field; when `TestWork` is set to true, the `WORK` directory will not be cleaned up, allowing for manual investigation of state in the shell. However, `defer` statements will still be run. - -TODO: Probably we want to add a `NO_CLEANUP` flag so `defer` is not run. Maybe this implies we want `defer` to be called `cleanup`? It might not be so clear in its intent though. Maybe `NO_CLEANUP_WORK` and `NO_DEFER`? +By default `testscript` removes the directory in which it was running the script, and if you've been a conscientious engineer, you should be cleaning up resources using the `defer` statement. However, this can be an impediment to debugging. As such you can set `GH_ACCEPTANCE_PRESERVE_WORK_DIR=true` and `GH_ACCEPTANCE_SKIP_DEFER=true` to skip these cleanup steps. ### Effective Test Authoring diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 6a5e32ff7..8d4d4b302 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -40,9 +40,10 @@ func testScriptParamsFor(tsEnv testScriptEnv, dir string) testscript.Params { Dir: path.Join("testdata", dir), Files: []string{}, Setup: sharedSetup(tsEnv), - Cmds: sharedCmds, + Cmds: sharedCmds(tsEnv), RequireExplicitExec: true, RequireUniqueNames: true, + TestWork: tsEnv.preserveWorkDir, } } @@ -65,22 +66,32 @@ func sharedSetup(tsEnv testScriptEnv) func(ts *testscript.Env) error { } } -var sharedCmds = map[string]func(ts *testscript.TestScript, neg bool, args []string){ - "defer": func(ts *testscript.TestScript, neg bool, args []string) { - ts.Defer(func() { - ts.Check(ts.Exec(args[0], args[1:]...)) - }) - }, - "stdout2env": func(ts *testscript.TestScript, neg bool, args []string) { - if neg { - ts.Fatalf("unsupported: ! stdout2env") - } - if len(args) != 1 { - ts.Fatalf("usage: stdout2env name") - } +func sharedCmds(tsEnv testScriptEnv) map[string]func(ts *testscript.TestScript, neg bool, args []string) { + return map[string]func(ts *testscript.TestScript, neg bool, args []string){ + "defer": func(ts *testscript.TestScript, neg bool, args []string) { + if neg { + ts.Fatalf("unsupported: ! defer") + } - ts.Setenv(args[0], strings.TrimRight(ts.ReadFile("stdout"), "\n")) - }, + if tsEnv.skipDefer { + return + } + + ts.Defer(func() { + ts.Check(ts.Exec(args[0], args[1:]...)) + }) + }, + "stdout2env": func(ts *testscript.TestScript, neg bool, args []string) { + if neg { + ts.Fatalf("unsupported: ! stdout2env") + } + if len(args) != 1 { + ts.Fatalf("usage: stdout2env name") + } + + ts.Setenv(args[0], strings.TrimRight(ts.ReadFile("stdout"), "\n")) + }, + } } var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") @@ -115,6 +126,9 @@ type testScriptEnv struct { host string org string token string + + skipDefer bool + preserveWorkDir bool } func (e *testScriptEnv) fromEnv() error { @@ -149,5 +163,8 @@ func (e *testScriptEnv) fromEnv() error { e.org = envMap["GH_ACCEPTANCE_ORG"] e.token = envMap["GH_ACCEPTANCE_TOKEN"] + e.preserveWorkDir = os.Getenv("GH_ACCEPTANCE_PRESERVE_WORK_DIR") == "true" + e.skipDefer = os.Getenv("GH_ACCEPTANCE_SKIP_DEFER") == "true" + return nil } From 1f94cf9dac2284cc4642275abf2f725da9ce28ea Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 14 Oct 2024 14:31:24 +0200 Subject: [PATCH 23/29] Acceptance test PR list --- acceptance/testdata/pr/pr-create-basic.txtar | 2 +- acceptance/testdata/pr/pr-list.txtar | 24 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 acceptance/testdata/pr/pr-list.txtar diff --git a/acceptance/testdata/pr/pr-create-basic.txtar b/acceptance/testdata/pr/pr-create-basic.txtar index 46d1de990..98bb2faa9 100644 --- a/acceptance/testdata/pr/pr-create-basic.txtar +++ b/acceptance/testdata/pr/pr-create-basic.txtar @@ -20,5 +20,5 @@ exec git push -u origin feature-branch exec gh pr create --title 'Feature Title' --body 'Feature Body' # Check the PR is indeed created -exec gh pr list +exec gh pr view stdout 'Feature Title' diff --git a/acceptance/testdata/pr/pr-list.txtar b/acceptance/testdata/pr/pr-list.txtar new file mode 100644 index 000000000..6fcd8e6b7 --- /dev/null +++ b/acceptance/testdata/pr/pr-list.txtar @@ -0,0 +1,24 @@ +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Clone the repo +exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Prepare a branch to PR +cd $SCRIPT_NAME-$RANDOM_STRING +exec git checkout -b feature-branch +exec git commit --allow-empty -m 'Empty Commit' +exec git push -u origin feature-branch + +# Create the PR +exec gh pr create --title 'Feature Title' --body 'Feature Body' + +# List PRs and see the new PR is in the list +exec gh pr list +stdout 'Feature Title\tfeature-branch\tOPEN' From c2c88b293e44e70ef7d98c434df2bda749613666 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 14 Oct 2024 14:32:05 +0200 Subject: [PATCH 24/29] Fix GH_HOST / GH_ACCEPTANCE_HOST misuse --- acceptance/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acceptance/README.md b/acceptance/README.md index 1311704d4..c15d439fd 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -20,7 +20,7 @@ The organization in which the acceptance tests can manage resources in. Consider #### `GH_ACCEPTANCE_TOKEN` -The token to use for authenticatin with the `GH_HOST`. This must already have the necessary scopes for each test, and must have permissions to act in the `GH_ACCEPTANCE_ORG`. See [Effective Test Authoring](#effective-test-authoring) for how tests must handle tokens without sufficient scopes. +The token to use for authenticatin with the `GH_ACCEPTANCE_HOST`. This must already have the necessary scopes for each test, and must have permissions to act in the `GH_ACCEPTANCE_ORG`. See [Effective Test Authoring](#effective-test-authoring) for how tests must handle tokens without sufficient scopes. It's recommended to create and use a Legacy PAT for this; Fine-Grained PATs do not offer all the necessary privileges required. You can use an OAuth token provided via `gh auth login --web` and can provide it to the acceptance tests via `GH_ACCEPTANCE_TOKEN=$(gh auth token --hostname )` but this can be a bit confusing and annoying if you `gh auth login` again without `-s` and lose the required scopes. From f3589b2573eefa66060427f0567c7b3bcb05d3aa Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 14 Oct 2024 14:33:17 +0200 Subject: [PATCH 25/29] Add VSCode extension links to Acceptance README Co-authored-by: Andy Feller --- acceptance/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acceptance/README.md b/acceptance/README.md index c15d439fd..3f2fa2f8d 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -67,7 +67,7 @@ Due to the `//go:build acceptance` build constraint, some functionality is limit }, ``` -You can install the `txtar` or `vscode-testscript` extensions to get syntax highlighting. +You can install the [`txtar`](https://marketplace.visualstudio.com/items?itemName=brody715.txtar) or [`vscode-testscript`](https://marketplace.visualstudio.com/items?itemName=twpayne.vscode-testscript) extensions to get syntax highlighting. ### Debugging Tests From b095d6bd582dc239ea5aa150dd4bcc859ce3ebca Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 14 Oct 2024 14:33:49 +0200 Subject: [PATCH 26/29] Add link to testscript pkg documentation Co-authored-by: Andy Feller --- acceptance/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acceptance/README.md b/acceptance/README.md index 3f2fa2f8d..4160f5e6d 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -1,6 +1,6 @@ ## Acceptance Tests -The acceptance tests are blackbox* tests that are expected to interact with resources on a real GitHub instance. +The acceptance tests are blackbox* tests that are expected to interact with resources on a real GitHub instance. They are built on top of [`go-internal/testscript` module](https://pkg.go.dev/github.com/rogpeppe/go-internal/testscript), which supports a variety of commands for defining filesystem-based tests. *Note: they aren't strictly blackbox because `exec gh` commands delegate to a binary set up by `testscript` that calls into `ghcmd.Main`. However, since our real `func main` is an extremely thin adapter over `ghcmd.Main`, this is reasonable. This tradeoff avoids us building the binary ourselves for the tests, and allows us to get code coverage metrics. From f7b279db1134c94e0f8442fc99af465ee0c48af1 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 14 Oct 2024 14:35:35 +0200 Subject: [PATCH 27/29] Correct testscript description in Acceptance readme --- acceptance/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acceptance/README.md b/acceptance/README.md index 4160f5e6d..1a7f6a93d 100644 --- a/acceptance/README.md +++ b/acceptance/README.md @@ -1,6 +1,6 @@ ## Acceptance Tests -The acceptance tests are blackbox* tests that are expected to interact with resources on a real GitHub instance. They are built on top of [`go-internal/testscript` module](https://pkg.go.dev/github.com/rogpeppe/go-internal/testscript), which supports a variety of commands for defining filesystem-based tests. +The acceptance tests are blackbox* tests that are expected to interact with resources on a real GitHub instance. They are built on top of the [`go-internal/testscript`](https://pkg.go.dev/github.com/rogpeppe/go-internal/testscript) package, which provides a framework for building tests for command line tools. *Note: they aren't strictly blackbox because `exec gh` commands delegate to a binary set up by `testscript` that calls into `ghcmd.Main`. However, since our real `func main` is an extremely thin adapter over `ghcmd.Main`, this is reasonable. This tradeoff avoids us building the binary ourselves for the tests, and allows us to get code coverage metrics. From 5e023267929d1c5ed771adc3b208ec736a7f5153 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 14 Oct 2024 14:36:34 +0200 Subject: [PATCH 28/29] Document sharedCmds func in acceptance tests Co-authored-by: Andy Feller --- acceptance/acceptance_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 8d4d4b302..bcc27faca 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -66,6 +66,7 @@ func sharedSetup(tsEnv testScriptEnv) func(ts *testscript.Env) error { } } +// sharedCmds defines a collection of custom testscript commands for our use. func sharedCmds(tsEnv testScriptEnv) map[string]func(ts *testscript.TestScript, neg bool, args []string) { return map[string]func(ts *testscript.TestScript, neg bool, args []string){ "defer": func(ts *testscript.TestScript, neg bool, args []string) { From 2a0be61d5e811ed0561a5a482b83fa93b0f2768b Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 14 Oct 2024 14:50:59 +0200 Subject: [PATCH 29/29] Ensure pr create with metadata has assignment --- acceptance/testdata/pr/pr-create-with-metadata.txtar | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acceptance/testdata/pr/pr-create-with-metadata.txtar b/acceptance/testdata/pr/pr-create-with-metadata.txtar index 765c84b67..3e06b533b 100644 --- a/acceptance/testdata/pr/pr-create-with-metadata.txtar +++ b/acceptance/testdata/pr/pr-create-with-metadata.txtar @@ -22,5 +22,5 @@ stdout2env PR_URL # Check the PR is indeed created exec gh pr view $PR_URL -stdout 'assignees:\t.*$' +stdout 'assignees:\t.+$' stdout 'labels:\tbug$'