From a408f24d3ac9b53a8c33879a4471709ec18a6190 Mon Sep 17 00:00:00 2001 From: Nick Cooper Date: Wed, 14 Jul 2021 11:01:45 -0400 Subject: [PATCH 01/19] auth: add noninteractive flow to login/refresh --- internal/authflow/flow.go | 37 +++++++++++++++++----------- pkg/cmd/auth/refresh/refresh.go | 8 +++--- pkg/cmd/auth/refresh/refresh_test.go | 2 +- pkg/cmd/auth/shared/login_flow.go | 2 +- 4 files changed, 28 insertions(+), 21 deletions(-) diff --git a/internal/authflow/flow.go b/internal/authflow/flow.go index bfe9d1299..1c6bb951f 100644 --- a/internal/authflow/flow.go +++ b/internal/authflow/flow.go @@ -27,13 +27,13 @@ type iconfig interface { Write() error } -func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice string, additionalScopes []string) (string, error) { +func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice string, additionalScopes []string, interactive bool) (string, error) { // TODO this probably shouldn't live in this package. It should probably be in a new package that // depends on both iostreams and config. stderr := IO.ErrOut cs := IO.ColorScheme() - token, userLogin, err := authFlow(hostname, IO, notice, additionalScopes) + token, userLogin, err := authFlow(hostname, IO, notice, additionalScopes, interactive) if err != nil { return "", err } @@ -52,14 +52,17 @@ func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice s return "", err } - fmt.Fprintf(stderr, "%s Authentication complete. %s to continue...\n", - cs.SuccessIcon(), cs.Bold("Press Enter")) - _ = waitForEnter(IO.In) - + if interactive { + fmt.Fprintf(stderr, "%s Authentication complete. %s to continue...\n", + cs.SuccessIcon(), cs.Bold("Press Enter")) + _ = waitForEnter(IO.In) + } else { + fmt.Fprintf(stderr, "%s Authentication complete.\n", cs.SuccessIcon()) + } return token, nil } -func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string) (string, string, error) { +func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string, interactive bool) (string, string, error) { w := IO.ErrOut cs := IO.ColorScheme() @@ -90,15 +93,19 @@ func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, addition return nil }, BrowseURL: func(url string) error { - fmt.Fprintf(w, "- %s to open %s in your browser... ", cs.Bold("Press Enter"), oauthHost) - _ = waitForEnter(IO.In) + if interactive { + fmt.Fprintf(w, "- %s to open %s in your browser... ", cs.Bold("Press Enter"), oauthHost) + _ = waitForEnter(IO.In) - // FIXME: read the browser from cmd Factory rather than recreating it - browser := cmdutil.NewBrowser(os.Getenv("BROWSER"), IO.Out, IO.ErrOut) - if err := browser.Browse(url); err != nil { - fmt.Fprintf(w, "%s Failed opening a web browser at %s\n", cs.Red("!"), url) - fmt.Fprintf(w, " %s\n", err) - fmt.Fprint(w, " Please try entering the URL in your browser manually\n") + // FIXME: read the browser from cmd Factory rather than recreating it + browser := cmdutil.NewBrowser(os.Getenv("BROWSER"), IO.Out, IO.ErrOut) + if err := browser.Browse(url); err != nil { + fmt.Fprintf(w, "%s Failed opening a web browser at %s\n", cs.Red("!"), url) + fmt.Fprintf(w, " %s\n", err) + fmt.Fprint(w, " Please try entering the URL in your browser manually\n") + } + } else { + fmt.Fprintf(w, "- Open this URL in your browser: %s", cs.Bold(url)) } return nil }, diff --git a/pkg/cmd/auth/refresh/refresh.go b/pkg/cmd/auth/refresh/refresh.go index f2b9cbbb4..02cb224c3 100644 --- a/pkg/cmd/auth/refresh/refresh.go +++ b/pkg/cmd/auth/refresh/refresh.go @@ -26,7 +26,7 @@ type RefreshOptions struct { Hostname string Scopes []string - AuthFlow func(config.Config, *iostreams.IOStreams, string, []string) error + AuthFlow func(config.Config, *iostreams.IOStreams, string, []string, bool) error Interactive bool } @@ -35,8 +35,8 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra. opts := &RefreshOptions{ IO: f.IOStreams, Config: f.Config, - AuthFlow: func(cfg config.Config, io *iostreams.IOStreams, hostname string, scopes []string) error { - _, err := authflow.AuthFlowWithConfig(cfg, io, hostname, "", scopes) + AuthFlow: func(cfg config.Config, io *iostreams.IOStreams, hostname string, scopes []string, interactive bool) error { + _, err := authflow.AuthFlowWithConfig(cfg, io, hostname, "", scopes, interactive) return err }, httpClient: http.DefaultClient, @@ -154,7 +154,7 @@ func refreshRun(opts *RefreshOptions) error { additionalScopes = append(additionalScopes, credentialFlow.Scopes()...) } - if err := opts.AuthFlow(cfg, opts.IO, hostname, append(opts.Scopes, additionalScopes...)); err != nil { + if err := opts.AuthFlow(cfg, opts.IO, hostname, append(opts.Scopes, additionalScopes...), opts.Interactive); err != nil { return err } diff --git a/pkg/cmd/auth/refresh/refresh_test.go b/pkg/cmd/auth/refresh/refresh_test.go index dbdae26af..9b6f15ba8 100644 --- a/pkg/cmd/auth/refresh/refresh_test.go +++ b/pkg/cmd/auth/refresh/refresh_test.go @@ -233,7 +233,7 @@ func Test_refreshRun(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { aa := authArgs{} - tt.opts.AuthFlow = func(_ config.Config, _ *iostreams.IOStreams, hostname string, scopes []string) error { + tt.opts.AuthFlow = func(_ config.Config, _ *iostreams.IOStreams, hostname string, scopes []string, interactive bool) error { aa.hostname = hostname aa.scopes = scopes return nil diff --git a/pkg/cmd/auth/shared/login_flow.go b/pkg/cmd/auth/shared/login_flow.go index 0bac49b35..427336356 100644 --- a/pkg/cmd/auth/shared/login_flow.go +++ b/pkg/cmd/auth/shared/login_flow.go @@ -117,7 +117,7 @@ func Login(opts *LoginOptions) error { if authMode == 0 { var err error - authToken, err = authflow.AuthFlowWithConfig(cfg, opts.IO, hostname, "", append(opts.Scopes, additionalScopes...)) + authToken, err = authflow.AuthFlowWithConfig(cfg, opts.IO, hostname, "", append(opts.Scopes, additionalScopes...), opts.Interactive) if err != nil { return fmt.Errorf("failed to authenticate via web browser: %w", err) } From be9f01101ae6c5d9957eb0690fbe1893d396bb9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 11 Jan 2022 19:18:11 +0100 Subject: [PATCH 02/19] Tweak auth flow re: interactivity Fixes non-interactive login flow and make sure "prompt" configuration is respected by never prompting if it was explicitly disabled. No longer asks to press Enter again after "Authentication complete" message, since that didn't provide any value to the user. --- internal/authflow/flow.go | 49 ++++++++++++------------------- pkg/cmd/auth/login/login.go | 35 +++++++++------------- pkg/cmd/auth/login/login_test.go | 15 +++++++--- pkg/cmd/auth/refresh/refresh.go | 3 ++ pkg/cmd/auth/shared/login_flow.go | 3 +- 5 files changed, 48 insertions(+), 57 deletions(-) diff --git a/internal/authflow/flow.go b/internal/authflow/flow.go index 1c6bb951f..fbf0a9e34 100644 --- a/internal/authflow/flow.go +++ b/internal/authflow/flow.go @@ -27,13 +27,11 @@ type iconfig interface { Write() error } -func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice string, additionalScopes []string, interactive bool) (string, error) { +func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice string, additionalScopes []string, isInteractive bool) (string, error) { // TODO this probably shouldn't live in this package. It should probably be in a new package that // depends on both iostreams and config. - stderr := IO.ErrOut - cs := IO.ColorScheme() - token, userLogin, err := authFlow(hostname, IO, notice, additionalScopes, interactive) + token, userLogin, err := authFlow(hostname, IO, notice, additionalScopes, isInteractive) if err != nil { return "", err } @@ -47,22 +45,10 @@ func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice s return "", err } - err = cfg.Write() - if err != nil { - return "", err - } - - if interactive { - fmt.Fprintf(stderr, "%s Authentication complete. %s to continue...\n", - cs.SuccessIcon(), cs.Bold("Press Enter")) - _ = waitForEnter(IO.In) - } else { - fmt.Fprintf(stderr, "%s Authentication complete.\n", cs.SuccessIcon()) - } - return token, nil + return token, cfg.Write() } -func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string, interactive bool) (string, string, error) { +func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string, isInteractive bool) (string, string, error) { w := IO.ErrOut cs := IO.ColorScheme() @@ -93,24 +79,25 @@ func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, addition return nil }, BrowseURL: func(url string) error { - if interactive { - fmt.Fprintf(w, "- %s to open %s in your browser... ", cs.Bold("Press Enter"), oauthHost) - _ = waitForEnter(IO.In) + if !isInteractive { + fmt.Fprintf(w, "%s to continue in your web browser: %s\n", cs.Bold("Open this URL"), url) + return nil + } - // FIXME: read the browser from cmd Factory rather than recreating it - browser := cmdutil.NewBrowser(os.Getenv("BROWSER"), IO.Out, IO.ErrOut) - if err := browser.Browse(url); err != nil { - fmt.Fprintf(w, "%s Failed opening a web browser at %s\n", cs.Red("!"), url) - fmt.Fprintf(w, " %s\n", err) - fmt.Fprint(w, " Please try entering the URL in your browser manually\n") - } - } else { - fmt.Fprintf(w, "- Open this URL in your browser: %s", cs.Bold(url)) + fmt.Fprintf(w, "%s to open %s in your browser... ", cs.Bold("Press Enter"), oauthHost) + _ = waitForEnter(IO.In) + + // FIXME: read the browser from cmd Factory rather than recreating it + browser := cmdutil.NewBrowser(os.Getenv("BROWSER"), IO.Out, IO.ErrOut) + if err := browser.Browse(url); err != nil { + fmt.Fprintf(w, "%s Failed opening a web browser at %s\n", cs.Red("!"), url) + fmt.Fprintf(w, " %s\n", err) + fmt.Fprint(w, " Please try entering the URL in your browser manually\n") } return nil }, WriteSuccessHTML: func(w io.Writer) { - fmt.Fprintln(w, oauthSuccessPage) + fmt.Fprint(w, oauthSuccessPage) }, HTTPClient: httpClient, Stdin: IO.In, diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index f591fcbc6..51e74861d 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -68,37 +68,34 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm $ gh auth login --hostname enterprise.internal `), RunE: func(cmd *cobra.Command, args []string) error { - if !opts.IO.CanPrompt() && !(tokenStdin || opts.Web) { - return cmdutil.FlagErrorf("--web or --with-token required when not running interactively") - } - if tokenStdin && opts.Web { - return cmdutil.FlagErrorf("specify only one of --web or --with-token") + return cmdutil.FlagErrorf("specify only one of `--web` or `--with-token`") + } + if tokenStdin && len(opts.Scopes) > 0 { + return cmdutil.FlagErrorf("specify only one of `--scopes` or `--with-token`") } if tokenStdin { defer opts.IO.In.Close() token, err := ioutil.ReadAll(opts.IO.In) if err != nil { - return fmt.Errorf("failed to read token from STDIN: %w", err) + return fmt.Errorf("failed to read token from standard input: %w", err) } opts.Token = strings.TrimSpace(string(token)) } - if opts.IO.CanPrompt() && opts.Token == "" && !opts.Web { + if opts.IO.CanPrompt() && opts.Token == "" { opts.Interactive = true } if cmd.Flags().Changed("hostname") { if err := ghinstance.HostnameValidator(opts.Hostname); err != nil { - return cmdutil.FlagErrorf("error parsing --hostname: %w", err) + return cmdutil.FlagErrorf("error parsing hostname: %w", err) } } - if !opts.Interactive { - if opts.Hostname == "" { - opts.Hostname = ghinstance.Default() - } + if opts.Hostname == "" && (!opts.Interactive || opts.Web) { + opts.Hostname = ghinstance.Default() } opts.MainExecutable = f.Executable() @@ -125,15 +122,11 @@ func loginRun(opts *LoginOptions) error { } hostname := opts.Hostname - if hostname == "" { - if opts.Interactive { - var err error - hostname, err = promptForHostname() - if err != nil { - return err - } - } else { - return errors.New("must specify --hostname") + if opts.Interactive && hostname == "" { + var err error + hostname, err = promptForHostname() + if err != nil { + return err } } diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index f6fbcabd5..1bc283327 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -50,13 +50,19 @@ func Test_NewCmdLogin(t *testing.T) { name: "nontty, hostname", stdinTTY: false, cli: "--hostname claire.redfield", - wantsErr: true, + wants: LoginOptions{ + Hostname: "claire.redfield", + Token: "", + }, }, { name: "nontty", stdinTTY: false, cli: "", - wantsErr: true, + wants: LoginOptions{ + Hostname: "github.com", + Token: "", + }, }, { name: "nontty, with-token, hostname", @@ -102,8 +108,9 @@ func Test_NewCmdLogin(t *testing.T) { stdinTTY: true, cli: "--web", wants: LoginOptions{ - Hostname: "github.com", - Web: true, + Hostname: "github.com", + Web: true, + Interactive: true, }, }, { diff --git a/pkg/cmd/auth/refresh/refresh.go b/pkg/cmd/auth/refresh/refresh.go index 02cb224c3..cd470132c 100644 --- a/pkg/cmd/auth/refresh/refresh.go +++ b/pkg/cmd/auth/refresh/refresh.go @@ -158,6 +158,9 @@ func refreshRun(opts *RefreshOptions) error { return err } + cs := opts.IO.ColorScheme() + fmt.Fprintf(opts.IO.ErrOut, "%s Authentication complete.\n", cs.SuccessIcon()) + if credentialFlow.ShouldSetup() { username, _ := cfg.Get(hostname, "user") password, _ := cfg.Get(hostname, "oauth_token") diff --git a/pkg/cmd/auth/shared/login_flow.go b/pkg/cmd/auth/shared/login_flow.go index 427336356..3570106da 100644 --- a/pkg/cmd/auth/shared/login_flow.go +++ b/pkg/cmd/auth/shared/login_flow.go @@ -99,7 +99,7 @@ func Login(opts *LoginOptions) error { var authMode int if opts.Web { authMode = 0 - } else { + } else if opts.Interactive { err := prompt.SurveyAskOne(&survey.Select{ Message: "How would you like to authenticate GitHub CLI?", Options: []string{ @@ -121,6 +121,7 @@ func Login(opts *LoginOptions) error { if err != nil { return fmt.Errorf("failed to authenticate via web browser: %w", err) } + fmt.Fprintf(opts.IO.ErrOut, "%s Authentication complete.\n", cs.SuccessIcon()) userValidated = true } else { minimumScopes := append([]string{"repo", "read:org"}, additionalScopes...) From 3c443efbed4735ac331b3354ff164ae77b7d0495 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 12 Jan 2022 21:37:24 +0100 Subject: [PATCH 03/19] Improve Survey prompt stubber for tests Both SurveyAsk and SurveyAskOne methods now share the same sets of stubs, making it possible to change which of these methods is used in the implementation without breaking tests. A new method `AskStubber.StubPrompt("")` is added as test helper to supersede old Stub and StubOne methods. The new helper matches on prompt messages rather than on field names, enabling tests to be written based on what the user would see rather than coupling to implementation details. The new stubber also allows verifying whether a Select or MultiSelect was rendered with the expected set of options. Furthermore, if a stubbed value is not present among those options, the stubber will panic instead of continuing normally. Stubbed Selects with an int instead of a string target receiver are now transparently handled. The values for Select stubs are always strings in tests, but the stubber will write an int answer if the receiver expects one as a selected index instead of a selected string value. Lastly, this set of changes improves test resiliency since the stubs are now matched based on prompt message (or field name for legacy stubs created with Stub) instead of sequentially, enabling the implementation to reorder the prompts without breaking existing tests. --- pkg/cmd/pr/shared/survey_test.go | 2 +- pkg/prompt/stubber.go | 196 +++++++++++++++++++++---------- 2 files changed, 137 insertions(+), 61 deletions(-) diff --git a/pkg/cmd/pr/shared/survey_test.go b/pkg/cmd/pr/shared/survey_test.go index a28d96198..b0daa59f0 100644 --- a/pkg/cmd/pr/shared/survey_test.go +++ b/pkg/cmd/pr/shared/survey_test.go @@ -71,7 +71,7 @@ func TestMetadataSurvey_selectAll(t *testing.T) { }, { Name: "milestone", - Value: []string{"(none)"}, + Value: "(none)", }, }) diff --git a/pkg/prompt/stubber.go b/pkg/prompt/stubber.go index be920cd25..6ad992d02 100644 --- a/pkg/prompt/stubber.go +++ b/pkg/prompt/stubber.go @@ -2,19 +2,15 @@ package prompt import ( "fmt" - "reflect" + "strings" "github.com/AlecAivazis/survey/v2" "github.com/AlecAivazis/survey/v2/core" + "github.com/cli/cli/v2/pkg/surveyext" ) type AskStubber struct { - Asks [][]*survey.Question - AskOnes []*survey.Prompt - Count int - OneCount int - Stubs [][]*QuestionStub - StubOnes []*PromptStub + stubs []*QuestionStub } func InitAskStubber() (*AskStubber, func()) { @@ -22,53 +18,104 @@ func InitAskStubber() (*AskStubber, func()) { origSurveyAskOne := SurveyAskOne as := AskStubber{} - SurveyAskOne = func(p survey.Prompt, response interface{}, opts ...survey.AskOpt) error { - as.AskOnes = append(as.AskOnes, &p) - count := as.OneCount - as.OneCount += 1 - if count >= len(as.StubOnes) { - panic(fmt.Sprintf("more asks than stubs. most recent call: %v", p)) - } - stubbedPrompt := as.StubOnes[count] - if stubbedPrompt.Default { - // TODO this is failing for basic AskOne invocations with a string result. - defaultValue := reflect.ValueOf(p).Elem().FieldByName("Default") - _ = core.WriteAnswer(response, "", defaultValue) - } else { - _ = core.WriteAnswer(response, "", stubbedPrompt.Value) + answerFromStub := func(p survey.Prompt, fieldName string, response interface{}) error { + var message string + var defaultValue interface{} + var options []string + switch pt := p.(type) { + case *survey.Confirm: + message = pt.Message + defaultValue = pt.Default + case *survey.Input: + message = pt.Message + defaultValue = pt.Default + case *survey.Select: + message = pt.Message + options = pt.Options + case *survey.MultiSelect: + message = pt.Message + options = pt.Options + case *survey.Password: + message = pt.Message + case *surveyext.GhEditor: + message = pt.Message + defaultValue = pt.Default + default: + panic(fmt.Sprintf("prompt type %T is not supported by the stubber", pt)) } + var stub *QuestionStub + for _, s := range as.stubs { + if !s.matched && (s.message == "" && strings.EqualFold(s.Name, fieldName) || s.message == message) { + stub = s + stub.matched = true + break + } + } + if stub == nil { + panic(fmt.Sprintf("no prompt stub for %q", message)) + } + + if len(stub.options) > 0 { + if err := compareOptions(stub.options, options); err != nil { + panic(fmt.Sprintf("options mismatch for %q: %v", message, err)) + } + } + + userValue := stub.Value + + if stringValue, ok := stub.Value.(string); ok && len(options) > 0 { + foundIndex := -1 + for i, o := range options { + if o == stringValue { + foundIndex = i + break + } + } + if foundIndex < 0 { + panic(fmt.Sprintf("answer %q not found in options for %q: %v", stringValue, message, options)) + } + userValue = core.OptionAnswer{ + Value: stringValue, + Index: foundIndex, + } + } + + if stub.Default { + if defaultIndex, ok := defaultValue.(int); ok && len(options) > 0 { + userValue = core.OptionAnswer{ + Value: options[defaultIndex], + Index: defaultIndex, + } + } else if defaultValue == nil && len(options) > 0 { + userValue = core.OptionAnswer{ + Value: options[0], + Index: 0, + } + } else { + userValue = defaultValue + } + } + + if err := core.WriteAnswer(response, fieldName, userValue); err != nil { + return fmt.Errorf("AskStubber failed writing the answer for field %q: %w", fieldName, err) + } return nil } + SurveyAskOne = func(p survey.Prompt, response interface{}, opts ...survey.AskOpt) error { + return answerFromStub(p, "", response) + } + SurveyAsk = func(qs []*survey.Question, response interface{}, opts ...survey.AskOpt) error { - as.Asks = append(as.Asks, qs) - count := as.Count - as.Count += 1 - if count >= len(as.Stubs) { - panic(fmt.Sprintf("more asks than stubs. most recent call: %#v", qs)) - } - - // actually set response - stubbedQuestions := as.Stubs[count] - if len(stubbedQuestions) != len(qs) { - panic(fmt.Sprintf("asked questions: %d; stubbed questions: %d", len(qs), len(stubbedQuestions))) - } - for i, sq := range stubbedQuestions { - q := qs[i] - if q.Name != sq.Name { - panic(fmt.Sprintf("stubbed question mismatch: %s != %s", q.Name, sq.Name)) - } - if sq.Default { - defaultValue := reflect.ValueOf(q.Prompt).Elem().FieldByName("Default") - _ = core.WriteAnswer(response, q.Name, defaultValue) - } else { - _ = core.WriteAnswer(response, q.Name, sq.Value) + for _, q := range qs { + if err := answerFromStub(q.Prompt, q.Name, response); err != nil { + return err } } - return nil } + teardown := func() { SurveyAsk = origSurveyAsk SurveyAskOne = origSurveyAskOne @@ -76,30 +123,59 @@ func InitAskStubber() (*AskStubber, func()) { return &as, teardown } -type PromptStub struct { - Value interface{} - Default bool -} - type QuestionStub struct { Name string Value interface{} Default bool + + matched bool + message string + options []string } +// AssertOptions asserts the options presented to the user in Selects and MultiSelects. +func (s *QuestionStub) AssertOptions(opts []string) *QuestionStub { + s.options = opts + return s +} + +// AnswerWith defines an answer for the given stub. +func (s *QuestionStub) AnswerWith(v interface{}) *QuestionStub { + s.Value = v + return s +} + +// AnswerDefault marks the current stub to be answered with the default value for the prompt question. +func (s *QuestionStub) AnswerDefault() *QuestionStub { + s.Default = true + return s +} + +// Deprecated: use StubPrompt func (as *AskStubber) StubOne(value interface{}) { - as.StubOnes = append(as.StubOnes, &PromptStub{ - Value: value, - }) -} - -func (as *AskStubber) StubOneDefault() { - as.StubOnes = append(as.StubOnes, &PromptStub{ - Default: true, - }) + as.Stub([]*QuestionStub{{Value: value}}) } +// Deprecated: use StubPrompt func (as *AskStubber) Stub(stubbedQuestions []*QuestionStub) { - // A call to .Ask takes a list of questions; a stub is then a list of questions in the same order. - as.Stubs = append(as.Stubs, stubbedQuestions) + as.stubs = append(as.stubs, stubbedQuestions...) +} + +// StubPrompt records a stub for an interactive prompt matched by its message. +func (as *AskStubber) StubPrompt(msg string) *QuestionStub { + stub := &QuestionStub{message: msg} + as.stubs = append(as.stubs, stub) + return stub +} + +func compareOptions(expected, got []string) error { + if len(expected) != len(got) { + return fmt.Errorf("expected %v, got %v (length mismatch)", expected, got) + } + for i, v := range expected { + if v != got[i] { + return fmt.Errorf("expected %v, got %v", expected, got) + } + } + return nil } From ff072574f942f7eae87306de85f6af3ce4fb36ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 12 Jan 2022 23:11:02 +0100 Subject: [PATCH 04/19] Port `release create` tests to the new ask stubber --- pkg/cmd/release/create/create_test.go | 85 +++++++-------------------- 1 file changed, 21 insertions(+), 64 deletions(-) diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index f13150002..66dea87c0 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -515,27 +515,16 @@ func Test_createRun_interactive(t *testing.T) { name: "create a release from existing tag", opts: &CreateOptions{}, askStubs: func(as *prompt.AskStubber) { - as.StubOne("v1.2.3") // Tag prompt - as.Stub([]*prompt.QuestionStub{ - { - Name: "name", - Value: "title", - }, - { - Name: "releaseNotesAction", - Value: "Leave blank", - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "prerelease", - Value: false, - }, - { - Name: "submitAction", - Value: "Publish release", - }, - }) + as.StubPrompt("Choose a tag"). + AssertOptions([]string{"v1.2.3", "v1.2.2", "v1.0.0", "v0.1.2", "Create a new tag"}). + AnswerWith("v1.2.3") + as.StubPrompt("Title (optional)").AnswerWith("") + as.StubPrompt("Release notes"). + AssertOptions([]string{"Write my own", "Write using generated notes as template", "Leave blank"}). + AnswerWith("Leave blank") + as.StubPrompt("Is this a prerelease?").AnswerWith(false) + as.StubPrompt("Submit?"). + AssertOptions([]string{"Publish release", "Save as draft", "Cancel"}).AnswerWith("Publish release") }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/tags"), httpmock.StatusStringResponse(200, `[ @@ -558,28 +547,12 @@ func Test_createRun_interactive(t *testing.T) { name: "create a release from new tag", opts: &CreateOptions{}, askStubs: func(as *prompt.AskStubber) { - as.StubOne("Create a new tag") // Tag prompt - as.StubOne("v1.2.3") // New tag prompt - as.Stub([]*prompt.QuestionStub{ - { - Name: "name", - Value: "title", - }, - { - Name: "releaseNotesAction", - Value: "Leave blank", - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "prerelease", - Value: false, - }, - { - Name: "submitAction", - Value: "Publish release", - }, - }) + as.StubPrompt("Choose a tag").AnswerWith("Create a new tag") + as.StubPrompt("Tag name").AnswerWith("v1.2.3") + as.StubPrompt("Title (optional)").AnswerWith("") + as.StubPrompt("Release notes").AnswerWith("Leave blank") + as.StubPrompt("Is this a prerelease?").AnswerWith(false) + as.StubPrompt("Submit?").AnswerWith("Publish release") }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/tags"), httpmock.StatusStringResponse(200, `[ @@ -604,26 +577,10 @@ func Test_createRun_interactive(t *testing.T) { TagName: "v1.2.3", }, askStubs: func(as *prompt.AskStubber) { - as.Stub([]*prompt.QuestionStub{ - { - Name: "name", - Value: "title", - }, - { - Name: "releaseNotesAction", - Value: "Write using generated notes as template", - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "prerelease", - Value: false, - }, - { - Name: "submitAction", - Value: "Publish release", - }, - }) + as.StubPrompt("Title (optional)").AnswerDefault() + as.StubPrompt("Release notes").AnswerWith("Write using generated notes as template") + as.StubPrompt("Is this a prerelease?").AnswerWith(false) + as.StubPrompt("Submit?").AnswerWith("Publish release") }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases/generate-notes"), @@ -641,7 +598,7 @@ func Test_createRun_interactive(t *testing.T) { wantParams: map[string]interface{}{ "body": "generated body", "draft": false, - "name": "title", + "name": "generated name", "prerelease": false, "tag_name": "v1.2.3", }, From 90ec1089efdd68dc3e4e877ed27d6987e248b1c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 12 Jan 2022 23:48:22 +0100 Subject: [PATCH 05/19] Port `pr create` tests to the new ask stubber --- pkg/cmd/pr/create/create_test.go | 62 ++++++++++++++------------------ 1 file changed, 26 insertions(+), 36 deletions(-) diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 2b2d19e46..395f194a6 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -285,24 +285,10 @@ func TestPRCreate_recover(t *testing.T) { as, teardown := prompt.InitAskStubber() defer teardown() - as.Stub([]*prompt.QuestionStub{ - { - Name: "Title", - Default: true, - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "Body", - Default: true, - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "confirmation", - Value: 0, - }, - }) + + as.StubPrompt("Title").AnswerDefault() + as.StubPrompt("Body").AnswerDefault() + as.StubPrompt("What's next?").AnswerDefault() tmpfile, err := ioutil.TempFile(t.TempDir(), "testrecover*") assert.NoError(t, err) @@ -395,7 +381,8 @@ func TestPRCreate(t *testing.T) { ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() - ask.StubOne(0) + + ask.StubPrompt("Where should we push the 'feature' branch?").AnswerDefault() output, err := runCommand(http, nil, "feature", true, `-t "my title" -b "my body"`) require.NoError(t, err) @@ -440,7 +427,8 @@ func TestPRCreate_NoMaintainerModify(t *testing.T) { ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() - ask.StubOne(0) + + ask.StubPrompt("Where should we push the 'feature' branch?").AnswerDefault() output, err := runCommand(http, nil, "feature", true, `-t "my title" -b "my body" --no-maintainer-edit`) require.NoError(t, err) @@ -490,7 +478,10 @@ func TestPRCreate_createFork(t *testing.T) { ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() - ask.StubOne(1) + + ask.StubPrompt("Where should we push the 'feature' branch?"). + AssertOptions([]string{"OWNER/REPO", "Create a fork of OWNER/REPO", "Skip pushing the branch", "Cancel"}). + AnswerWith("Create a fork of OWNER/REPO") output, err := runCommand(http, nil, "feature", true, `-t title -b body`) require.NoError(t, err) @@ -620,19 +611,14 @@ func TestPRCreate_nonLegacyTemplate(t *testing.T) { as, teardown := prompt.InitAskStubber() defer teardown() - as.StubOne(0) // template - as.Stub([]*prompt.QuestionStub{ - { - Name: "Body", - Default: true, - }, - }) // body - as.Stub([]*prompt.QuestionStub{ - { - Name: "confirmation", - Value: 0, - }, - }) // confirm + + as.StubPrompt("Choose a template"). + AssertOptions([]string{"Bug fix", "Open a blank pull request"}). + AnswerWith("Bug fix") + as.StubPrompt("Body").AnswerDefault() + as.StubPrompt("What's next?"). + AssertOptions([]string{"Submit", "Continue in browser", "Add metadata", "Cancel"}). + AnswerDefault() output, err := runCommandWithRootDirOverridden(http, nil, "feature", true, `-t "my title" -H feature`, "./fixtures/repoWithNonLegacyPRTemplates") require.NoError(t, err) @@ -773,7 +759,10 @@ func TestPRCreate_web(t *testing.T) { ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() - ask.StubOne(0) + + ask.StubPrompt("Where should we push the 'feature' branch?"). + AssertOptions([]string{"OWNER/REPO", "Skip pushing the branch", "Cancel"}). + AnswerDefault() output, err := runCommand(http, nil, "feature", true, `--web`) require.NoError(t, err) @@ -844,7 +833,8 @@ func TestPRCreate_webProject(t *testing.T) { ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() - ask.StubOne(0) + + ask.StubPrompt("Where should we push the 'feature' branch?").AnswerDefault() output, err := runCommand(http, nil, "feature", true, `--web -p Triage`) require.NoError(t, err) From 0cb4b7aaefb94ef621002fd87c67b7b7206cdf50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 13 Jan 2022 11:16:58 +0100 Subject: [PATCH 06/19] Improve SurveyAskOne stub error messages --- pkg/prompt/stubber.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/prompt/stubber.go b/pkg/prompt/stubber.go index 6ad992d02..482381161 100644 --- a/pkg/prompt/stubber.go +++ b/pkg/prompt/stubber.go @@ -98,7 +98,11 @@ func InitAskStubber() (*AskStubber, func()) { } if err := core.WriteAnswer(response, fieldName, userValue); err != nil { - return fmt.Errorf("AskStubber failed writing the answer for field %q: %w", fieldName, err) + topic := fmt.Sprintf("field %q", fieldName) + if fieldName == "" { + topic = fmt.Sprintf("%q", message) + } + return fmt.Errorf("AskStubber failed writing the answer for %s: %w", topic, err) } return nil } From 649e30ce2a7a1d92b1843ea542b0dae23cc69144 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 13 Jan 2022 11:17:42 +0100 Subject: [PATCH 07/19] Fix ask stubs in `repo create` tests --- pkg/cmd/repo/create/create_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index 9048845fd..d27da66b5 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -175,7 +175,7 @@ func Test_createRun(t *testing.T) { as.Stub([]*prompt.QuestionStub{ {Name: "repoName", Value: "REPO"}, {Name: "repoDescription", Value: "my new repo"}, - {Name: "repoVisibility", Value: "PRIVATE"}, + {Name: "repoVisibility", Value: "Private"}, }) as.Stub([]*prompt.QuestionStub{ {Name: "addGitIgnore", Value: true}}) @@ -214,7 +214,7 @@ func Test_createRun(t *testing.T) { as.Stub([]*prompt.QuestionStub{ {Name: "repoName", Value: "REPO"}, {Name: "repoDescription", Value: "my new repo"}, - {Name: "repoVisibility", Value: "PRIVATE"}, + {Name: "repoVisibility", Value: "Private"}, }) as.Stub([]*prompt.QuestionStub{ {Name: "addGitIgnore", Value: false}}) @@ -237,7 +237,7 @@ func Test_createRun(t *testing.T) { as.Stub([]*prompt.QuestionStub{ {Name: "repoName", Value: "REPO"}, {Name: "repoDescription", Value: "my new repo"}, - {Name: "repoVisibility", Value: "PRIVATE"}, + {Name: "repoVisibility", Value: "Private"}, }) as.StubOne(false) }, @@ -274,7 +274,7 @@ func Test_createRun(t *testing.T) { as.Stub([]*prompt.QuestionStub{ {Name: "repoName", Value: "REPO"}, {Name: "repoDescription", Value: "my new repo"}, - {Name: "repoVisibility", Value: "PRIVATE"}, + {Name: "repoVisibility", Value: "Private"}, }) as.StubOne(true) //ask for adding a remote as.StubOne("origin") //ask for remote name @@ -314,7 +314,7 @@ func Test_createRun(t *testing.T) { as.Stub([]*prompt.QuestionStub{ {Name: "repoName", Value: "REPO"}, {Name: "repoDescription", Value: "my new repo"}, - {Name: "repoVisibility", Value: "PRIVATE"}, + {Name: "repoVisibility", Value: "Private"}, }) as.StubOne(true) //ask for adding a remote as.StubOne("origin") //ask for remote name From 62bd82809dccc37f37fc5ccc835dacaf32996aaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 13 Jan 2022 11:56:45 +0100 Subject: [PATCH 08/19] Port `auth login` tests to the new ask stubber --- pkg/cmd/auth/login/login_test.go | 62 ++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 22 deletions(-) diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index 4c20e2ed5..dc4bab91e 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -5,6 +5,7 @@ import ( "net/http" "os" "regexp" + "runtime" "testing" "github.com/MakeNowJust/heredoc" @@ -18,6 +19,21 @@ import ( "github.com/stretchr/testify/assert" ) +func stubHomeDir(t *testing.T, dir string) { + homeEnv := "HOME" + switch runtime.GOOS { + case "windows": + homeEnv = "USERPROFILE" + case "plan9": + homeEnv = "home" + } + oldHomeDir := os.Getenv(homeEnv) + os.Setenv(homeEnv, dir) + t.Cleanup(func() { + os.Setenv(homeEnv, oldHomeDir) + }) +} + func Test_NewCmdLogin(t *testing.T) { tests := []struct { name string @@ -352,6 +368,8 @@ func Test_loginRun_nontty(t *testing.T) { } func Test_loginRun_Survey(t *testing.T) { + stubHomeDir(t, t.TempDir()) + tests := []struct { name string opts *LoginOptions @@ -377,8 +395,8 @@ func Test_loginRun_Survey(t *testing.T) { // httpmock.StringResponse(`{"data":{"viewer":{"login":"jillv"}}}`)) }, askStubs: func(as *prompt.AskStubber) { - as.StubOne(0) // host type github.com - as.StubOne(false) // do not continue + as.StubPrompt("What account do you want to log into?").AnswerWith("GitHub.com") + as.StubPrompt("You're already logged into github.com. Do you want to re-authenticate?").AnswerWith(false) }, wantHosts: "", // nothing should have been written to hosts wantErrOut: nil, @@ -396,10 +414,10 @@ func Test_loginRun_Survey(t *testing.T) { git_protocol: https `), askStubs: func(as *prompt.AskStubber) { - as.StubOne("HTTPS") // git_protocol - as.StubOne(false) // cache credentials - as.StubOne(1) // auth mode: token - as.StubOne("def456") // auth token + as.StubPrompt("What is your preferred protocol for Git operations?").AnswerWith("HTTPS") + as.StubPrompt("Authenticate Git with your GitHub credentials?").AnswerWith(false) + as.StubPrompt("How would you like to authenticate GitHub CLI?").AnswerWith("Paste an authentication token") + as.StubPrompt("Paste your authentication token:").AnswerWith("def456") }, runStubs: func(rs *run.CommandStubber) { rs.Register(`git config credential\.https:/`, 1, "") @@ -425,12 +443,12 @@ func Test_loginRun_Survey(t *testing.T) { Interactive: true, }, askStubs: func(as *prompt.AskStubber) { - as.StubOne(1) // host type enterprise - as.StubOne("brad.vickers") // hostname - as.StubOne("HTTPS") // git_protocol - as.StubOne(false) // cache credentials - as.StubOne(1) // auth mode: token - as.StubOne("def456") // auth token + as.StubPrompt("What account do you want to log into?").AnswerWith("GitHub Enterprise Server") + as.StubPrompt("GHE hostname:").AnswerWith("brad.vickers") + as.StubPrompt("What is your preferred protocol for Git operations?").AnswerWith("HTTPS") + as.StubPrompt("Authenticate Git with your GitHub credentials?").AnswerWith(false) + as.StubPrompt("How would you like to authenticate GitHub CLI?").AnswerWith("Paste an authentication token") + as.StubPrompt("Paste your authentication token:").AnswerWith("def456") }, runStubs: func(rs *run.CommandStubber) { rs.Register(`git config credential\.https:/`, 1, "") @@ -456,11 +474,11 @@ func Test_loginRun_Survey(t *testing.T) { Interactive: true, }, askStubs: func(as *prompt.AskStubber) { - as.StubOne(0) // host type github.com - as.StubOne("HTTPS") // git_protocol - as.StubOne(false) // cache credentials - as.StubOne(1) // auth mode: token - as.StubOne("def456") // auth token + as.StubPrompt("What account do you want to log into?").AnswerWith("GitHub.com") + as.StubPrompt("What is your preferred protocol for Git operations?").AnswerWith("HTTPS") + as.StubPrompt("Authenticate Git with your GitHub credentials?").AnswerWith(false) + as.StubPrompt("How would you like to authenticate GitHub CLI?").AnswerWith("Paste an authentication token") + as.StubPrompt("Paste your authentication token:").AnswerWith("def456") }, runStubs: func(rs *run.CommandStubber) { rs.Register(`git config credential\.https:/`, 1, "") @@ -480,11 +498,11 @@ func Test_loginRun_Survey(t *testing.T) { Interactive: true, }, askStubs: func(as *prompt.AskStubber) { - as.StubOne(0) // host type github.com - as.StubOne("SSH") // git_protocol - as.StubOne(10) // TODO: SSH key selection - as.StubOne(1) // auth mode: token - as.StubOne("def456") // auth token + as.StubPrompt("What account do you want to log into?").AnswerWith("GitHub.com") + as.StubPrompt("What is your preferred protocol for Git operations?").AnswerWith("SSH") + as.StubPrompt("Generate a new SSH key to add to your GitHub account?").AnswerWith(false) + as.StubPrompt("How would you like to authenticate GitHub CLI?").AnswerWith("Paste an authentication token") + as.StubPrompt("Paste your authentication token:").AnswerWith("def456") }, wantErrOut: regexp.MustCompile("Tip: you can generate a Personal Access Token here https://github.com/settings/tokens"), }, From 8e64c149e155c1b7d51524e22b27e2399d2f5496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 13 Jan 2022 11:58:58 +0100 Subject: [PATCH 09/19] Return errors in ask stubber instead of panicking --- pkg/prompt/stubber.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/prompt/stubber.go b/pkg/prompt/stubber.go index 482381161..3159540f3 100644 --- a/pkg/prompt/stubber.go +++ b/pkg/prompt/stubber.go @@ -41,7 +41,7 @@ func InitAskStubber() (*AskStubber, func()) { message = pt.Message defaultValue = pt.Default default: - panic(fmt.Sprintf("prompt type %T is not supported by the stubber", pt)) + return fmt.Errorf("prompt type %T is not supported by the stubber", pt) } var stub *QuestionStub @@ -53,12 +53,12 @@ func InitAskStubber() (*AskStubber, func()) { } } if stub == nil { - panic(fmt.Sprintf("no prompt stub for %q", message)) + return fmt.Errorf("no prompt stub for %q", message) } if len(stub.options) > 0 { if err := compareOptions(stub.options, options); err != nil { - panic(fmt.Sprintf("options mismatch for %q: %v", message, err)) + return fmt.Errorf("stubbed options mismatch for %q: %v", message, err) } } @@ -73,7 +73,7 @@ func InitAskStubber() (*AskStubber, func()) { } } if foundIndex < 0 { - panic(fmt.Sprintf("answer %q not found in options for %q: %v", stringValue, message, options)) + return fmt.Errorf("answer %q not found in options for %q: %v", stringValue, message, options) } userValue = core.OptionAnswer{ Value: stringValue, From c839d3ba1def7cf396052f2f0cd672514673e426 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 13 Jan 2022 12:23:42 +0100 Subject: [PATCH 10/19] Check for unused ask stubs at the end of the test --- pkg/cmd/gist/view/view_test.go | 40 +++++++++++++++----------- pkg/cmd/pr/review/review_test.go | 24 +++------------- pkg/cmd/workflow/enable/enable_test.go | 19 ++++++------ pkg/prompt/stubber.go | 24 ++++++++++++++-- 4 files changed, 57 insertions(+), 50 deletions(-) diff --git a/pkg/cmd/gist/view/view_test.go b/pkg/cmd/gist/view/view_test.go index cc58867cf..dbacd9101 100644 --- a/pkg/cmd/gist/view/view_test.go +++ b/pkg/cmd/gist/view/view_test.go @@ -355,9 +355,9 @@ func Test_viewRun(t *testing.T) { )), ) - as, surveyteardown := prompt.InitAskStubber() - defer surveyteardown() - as.StubOne(0) + as, surveyteardown := prompt.NewAskStubber() + defer surveyteardown(t) + as.StubPrompt("Select a gist").AnswerDefault() } if tt.opts == nil { @@ -392,16 +392,18 @@ func Test_viewRun(t *testing.T) { func Test_promptGists(t *testing.T) { tests := []struct { - name string - gistIndex int - response string - wantOut string - gist *shared.Gist - wantErr bool + name string + askStubs func(as *prompt.AskStubber) + response string + wantOut string + gist *shared.Gist + wantErr bool }{ { - name: "multiple files, select first gist", - gistIndex: 0, + name: "multiple files, select first gist", + askStubs: func(as *prompt.AskStubber) { + as.StubPrompt("Select a gist").AnswerWith("cool.txt about 6 hours ago") + }, response: `{ "data": { "viewer": { "gists": { "nodes": [ { "name": "gistid1", @@ -421,8 +423,10 @@ func Test_promptGists(t *testing.T) { wantOut: "gistid1", }, { - name: "multiple files, select second gist", - gistIndex: 1, + name: "multiple files, select second gist", + askStubs: func(as *prompt.AskStubber) { + as.StubPrompt("Select a gist").AnswerWith("gistfile0.txt about 6 hours ago") + }, response: `{ "data": { "viewer": { "gists": { "nodes": [ { "name": "gistid1", @@ -465,11 +469,13 @@ func Test_promptGists(t *testing.T) { ) client := &http.Client{Transport: reg} - as, surveyteardown := prompt.InitAskStubber() - defer surveyteardown() - as.StubOne(tt.gistIndex) - t.Run(tt.name, func(t *testing.T) { + as, surveyteardown := prompt.NewAskStubber() + defer surveyteardown(t) + if tt.askStubs != nil { + tt.askStubs(as) + } + gistID, err := promptGists(client, "github.com", io.ColorScheme()) assert.NoError(t, err) assert.Equal(t, tt.wantOut, gistID) diff --git a/pkg/cmd/pr/review/review_test.go b/pkg/cmd/pr/review/review_test.go index 75ae8c200..87ec8e984 100644 --- a/pkg/cmd/pr/review/review_test.go +++ b/pkg/cmd/pr/review/review_test.go @@ -309,27 +309,11 @@ func TestPRReview_interactive_no_body(t *testing.T) { shared.RunCommandFinder("", &api.PullRequest{ID: "THE-ID", Number: 123}, ghrepo.New("OWNER", "REPO")) - as, teardown := prompt.InitAskStubber() - defer teardown() + as, teardown := prompt.NewAskStubber() + defer teardown(t) - as.Stub([]*prompt.QuestionStub{ - { - Name: "reviewType", - Value: "Request changes", - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "body", - Default: true, - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "confirm", - Value: true, - }, - }) + as.StubPrompt("What kind of review do you want to give?").AnswerWith("Request changes") + as.StubPrompt("Review body").AnswerWith("") _, err := runCommand(http, nil, true, "") assert.EqualError(t, err, "this type of review cannot be blank") diff --git a/pkg/cmd/workflow/enable/enable_test.go b/pkg/cmd/workflow/enable/enable_test.go index fb0e703bb..7cc3fbd47 100644 --- a/pkg/cmd/workflow/enable/enable_test.go +++ b/pkg/cmd/workflow/enable/enable_test.go @@ -121,7 +121,7 @@ func TestEnableRun(t *testing.T) { httpmock.StatusStringResponse(204, "{}")) }, askStubs: func(as *prompt.AskStubber) { - as.StubOne(0) + as.StubPrompt("Select a workflow").AnswerWith("a disabled workflow (disabled.yml)") }, wantOut: "✓ Enabled a disabled workflow\n", }, @@ -176,7 +176,7 @@ func TestEnableRun(t *testing.T) { httpmock.StatusStringResponse(204, "{}")) }, askStubs: func(as *prompt.AskStubber) { - as.StubOne(1) + as.StubPrompt("Which workflow do you mean?").AnswerWith("a disabled workflow (anotherDisabled.yml)") }, wantOut: "✓ Enabled a disabled workflow\n", }, @@ -194,9 +194,6 @@ func TestEnableRun(t *testing.T) { httpmock.REST("PUT", "repos/OWNER/REPO/actions/workflows/456/enable"), httpmock.StatusStringResponse(204, "{}")) }, - askStubs: func(as *prompt.AskStubber) { - as.StubOne(0) - }, wantOut: "✓ Enabled a disabled workflow\n", }, { @@ -279,13 +276,13 @@ func TestEnableRun(t *testing.T) { return ghrepo.FromFullName("OWNER/REPO") } - as, teardown := prompt.InitAskStubber() - defer teardown() - if tt.askStubs != nil { - tt.askStubs(as) - } - t.Run(tt.name, func(t *testing.T) { + as, teardown := prompt.NewAskStubber() + defer teardown(t) + if tt.askStubs != nil { + tt.askStubs(as) + } + err := runEnable(tt.opts) if tt.wantErr { assert.Error(t, err) diff --git a/pkg/prompt/stubber.go b/pkg/prompt/stubber.go index 3159540f3..f01a28dcf 100644 --- a/pkg/prompt/stubber.go +++ b/pkg/prompt/stubber.go @@ -13,7 +13,11 @@ type AskStubber struct { stubs []*QuestionStub } -func InitAskStubber() (*AskStubber, func()) { +type testing interface { + Errorf(format string, args ...interface{}) +} + +func NewAskStubber() (*AskStubber, func(t testing)) { origSurveyAsk := SurveyAsk origSurveyAskOne := SurveyAskOne as := AskStubber{} @@ -120,13 +124,29 @@ func InitAskStubber() (*AskStubber, func()) { return nil } - teardown := func() { + teardown := func(t testing) { SurveyAsk = origSurveyAsk SurveyAskOne = origSurveyAskOne + for _, s := range as.stubs { + if !s.matched { + if t == nil { + panic(fmt.Sprintf("unmatched prompt stub: %+v", s)) + } else { + t.Errorf("unmatched prompt stub: %+v", s) + } + } + } } return &as, teardown } +func InitAskStubber() (*AskStubber, func()) { + as, teardown := NewAskStubber() + return as, func() { + teardown(nil) + } +} + type QuestionStub struct { Name string Value interface{} From 171482970f5025cd7bc820f341832e704a7167dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 13 Jan 2022 13:16:05 +0100 Subject: [PATCH 11/19] Cache Go modules between CI runs --- .github/workflows/go.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 226050ea9..f12d8f0d0 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -17,6 +17,15 @@ jobs: - name: Check out code uses: actions/checkout@v2 + - name: Cache Go modules + uses: actions/cache@v2 + with: + path: ~/go + key: ${{ runner.os }}-build-${{ hashFiles('go.mod') }} + restore-keys: | + ${{ runner.os }}-build- + ${{ runner.os }}- + - name: Download dependencies run: go mod download From 456d55ead9e0410989701e2f42bd1407ec1f1cce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 13 Jan 2022 19:39:43 +0100 Subject: [PATCH 12/19] Have a single Render function handle all Markdown rendering --- pkg/cmd/gist/view/view.go | 5 +- pkg/cmd/issue/view/view.go | 3 +- pkg/cmd/pr/review/review.go | 3 +- pkg/cmd/pr/shared/comments.go | 3 +- pkg/cmd/pr/view/view.go | 3 +- pkg/cmd/release/view/view.go | 3 +- pkg/cmd/repo/view/view.go | 3 +- pkg/cmd/root/help_reference.go | 5 +- pkg/cmd/workflow/view/view.go | 9 +-- pkg/markdown/markdown.go | 87 +++++++----------------- pkg/markdown/markdown_test.go | 121 +++++---------------------------- 11 files changed, 56 insertions(+), 189 deletions(-) diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index 40134412d..121ca2fac 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -128,8 +128,7 @@ func viewRun(opts *ViewOptions) error { return err } - theme := opts.IO.DetectTerminalTheme() - markdownStyle := markdown.GetStyle(theme) + opts.IO.DetectTerminalTheme() if err := opts.IO.StartPager(); err != nil { fmt.Fprintf(opts.IO.ErrOut, "starting pager failed: %v\n", err) } @@ -145,7 +144,7 @@ func viewRun(opts *ViewOptions) error { } if strings.Contains(gf.Type, "markdown") && !opts.Raw { - rendered, err := markdown.Render(gf.Content, markdownStyle) + rendered, err := markdown.Render(gf.Content, markdown.WithIO(opts.IO)) if err != nil { return err } diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index a839f488e..1eab5342c 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -231,8 +231,7 @@ func printHumanIssuePreview(opts *ViewOptions, issue *api.Issue) error { if issue.Body == "" { md = fmt.Sprintf("\n %s\n\n", cs.Gray("No description provided")) } else { - style := markdown.GetStyle(opts.IO.TerminalTheme()) - md, err = markdown.Render(issue.Body, style) + md, err = markdown.Render(issue.Body, markdown.WithIO(opts.IO)) if err != nil { return err } diff --git a/pkg/cmd/pr/review/review.go b/pkg/cmd/pr/review/review.go index d3b15a4ec..07593d747 100644 --- a/pkg/cmd/pr/review/review.go +++ b/pkg/cmd/pr/review/review.go @@ -273,8 +273,7 @@ func reviewSurvey(io *iostreams.IOStreams, editorCommand string) (*api.PullReque } if len(bodyAnswers.Body) > 0 { - style := markdown.GetStyle(io.DetectTerminalTheme()) - renderedBody, err := markdown.Render(bodyAnswers.Body, style) + renderedBody, err := markdown.Render(bodyAnswers.Body, markdown.WithIO(io)) if err != nil { return nil, err } diff --git a/pkg/cmd/pr/shared/comments.go b/pkg/cmd/pr/shared/comments.go index 86b52b68c..0d1d6c476 100644 --- a/pkg/cmd/pr/shared/comments.go +++ b/pkg/cmd/pr/shared/comments.go @@ -123,8 +123,7 @@ func formatComment(io *iostreams.IOStreams, comment Comment, newest bool) (strin if comment.Content() == "" { md = fmt.Sprintf("\n %s\n\n", cs.Gray("No body provided")) } else { - style := markdown.GetStyle(io.TerminalTheme()) - md, err = markdown.Render(comment.Content(), style) + md, err = markdown.Render(comment.Content(), markdown.WithIO(io)) if err != nil { return "", err } diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index bc3f517e4..f0746772d 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -215,8 +215,7 @@ func printHumanPrPreview(opts *ViewOptions, pr *api.PullRequest) error { if pr.Body == "" { md = fmt.Sprintf("\n %s\n\n", cs.Gray("No description provided")) } else { - style := markdown.GetStyle(opts.IO.TerminalTheme()) - md, err = markdown.Render(pr.Body, style) + md, err = markdown.Render(pr.Body, markdown.WithIO(opts.IO)) if err != nil { return err } diff --git a/pkg/cmd/release/view/view.go b/pkg/cmd/release/view/view.go index ae7d10e97..dde802f8d 100644 --- a/pkg/cmd/release/view/view.go +++ b/pkg/cmd/release/view/view.go @@ -141,8 +141,7 @@ func renderReleaseTTY(io *iostreams.IOStreams, release *shared.Release) error { fmt.Fprintf(w, "%s\n", iofmt.Gray(fmt.Sprintf("%s released this %s", release.Author.Login, utils.FuzzyAgo(time.Since(*release.PublishedAt))))) } - style := markdown.GetStyle(io.TerminalTheme()) - renderedDescription, err := markdown.Render(release.Body, style) + renderedDescription, err := markdown.Render(release.Body, markdown.WithIO(io)) if err != nil { return err } diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index 3d2879dc0..2a1c0fe30 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -187,8 +187,7 @@ func viewRun(opts *ViewOptions) error { readmeContent = cs.Gray("This repository does not have a README") } else if isMarkdownFile(readme.Filename) { var err error - style := markdown.GetStyle(opts.IO.TerminalTheme()) - readmeContent, err = markdown.RenderWithBaseURL(readme.Content, style, readme.BaseURL) + readmeContent, err = markdown.Render(readme.Content, markdown.WithIO(opts.IO), markdown.WithBaseURL(readme.BaseURL)) if err != nil { return fmt.Errorf("error rendering markdown: %w", err) } diff --git a/pkg/cmd/root/help_reference.go b/pkg/cmd/root/help_reference.go index 73070e0e9..b12f33b99 100644 --- a/pkg/cmd/root/help_reference.go +++ b/pkg/cmd/root/help_reference.go @@ -14,13 +14,12 @@ import ( func referenceHelpFn(io *iostreams.IOStreams) func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { wrapWidth := 0 - style := "notty" if io.IsStdoutTTY() { + io.DetectTerminalTheme() wrapWidth = io.TerminalWidth() - style = markdown.GetStyle(io.DetectTerminalTheme()) } - md, err := markdown.RenderWithWrap(cmd.Long, style, wrapWidth) + md, err := markdown.Render(cmd.Long, markdown.WithIO(io), markdown.WithWrap(wrapWidth)) if err != nil { fmt.Fprintln(io.ErrOut, err) return diff --git a/pkg/cmd/workflow/view/view.go b/pkg/cmd/workflow/view/view.go index d6b71e6c1..210835759 100644 --- a/pkg/cmd/workflow/view/view.go +++ b/pkg/cmd/workflow/view/view.go @@ -156,8 +156,7 @@ func viewWorkflowContent(opts *ViewOptions, client *api.Client, workflow *shared yaml := string(yamlBytes) - theme := opts.IO.DetectTerminalTheme() - markdownStyle := markdown.GetStyle(theme) + opts.IO.DetectTerminalTheme() if err := opts.IO.StartPager(); err != nil { fmt.Fprintf(opts.IO.ErrOut, "starting pager failed: %v\n", err) } @@ -172,11 +171,7 @@ func viewWorkflowContent(opts *ViewOptions, client *api.Client, workflow *shared fmt.Fprintf(out, "ID: %s", cs.Cyanf("%d", workflow.ID)) codeBlock := fmt.Sprintf("```yaml\n%s\n```", yaml) - rendered, err := markdown.RenderWithOpts(codeBlock, markdownStyle, - markdown.RenderOpts{ - markdown.WithoutIndentation(), - markdown.WithoutWrap(), - }) + rendered, err := markdown.Render(codeBlock, markdown.WithIO(opts.IO), markdown.WithoutIndentation(), markdown.WithWrap(0)) if err != nil { return err } diff --git a/pkg/markdown/markdown.go b/pkg/markdown/markdown.go index 9da1a2586..cd578bf06 100644 --- a/pkg/markdown/markdown.go +++ b/pkg/markdown/markdown.go @@ -7,8 +7,6 @@ import ( "github.com/charmbracelet/glamour" ) -type RenderOpts []glamour.TermRendererOption - func WithoutIndentation() glamour.TermRendererOption { overrides := []byte(` { @@ -23,15 +21,38 @@ func WithoutIndentation() glamour.TermRendererOption { return glamour.WithStylesFromJSONBytes(overrides) } -func WithoutWrap() glamour.TermRendererOption { - return glamour.WithWordWrap(0) +func WithWrap(w int) glamour.TermRendererOption { + return glamour.WithWordWrap(w) } -func render(text string, opts RenderOpts) (string, error) { +type IOStreams interface { + TerminalTheme() string +} + +func WithIO(io IOStreams) glamour.TermRendererOption { + style := os.Getenv("GLAMOUR_STYLE") + if style == "" || style == "auto" { + theme := io.TerminalTheme() + switch theme { + case "light", "dark": + style = theme + default: + style = "notty" + } + } + return glamour.WithStylePath(style) +} + +func WithBaseURL(u string) glamour.TermRendererOption { + return glamour.WithBaseURL(u) +} + +func Render(text string, opts ...glamour.TermRendererOption) (string, error) { // Glamour rendering preserves carriage return characters in code blocks, but // we need to ensure that no such characters are present in the output. text = strings.ReplaceAll(text, "\r\n", "\n") + opts = append(opts, glamour.WithEmoji()) tr, err := glamour.NewTermRenderer(opts...) if err != nil { return "", err @@ -39,59 +60,3 @@ func render(text string, opts RenderOpts) (string, error) { return tr.Render(text) } - -func Render(text, style string) (string, error) { - opts := RenderOpts{ - glamour.WithStylePath(style), - glamour.WithEmoji(), - } - - return render(text, opts) -} - -func RenderWithOpts(text, style string, opts RenderOpts) (string, error) { - defaultOpts := RenderOpts{ - glamour.WithStylePath(style), - glamour.WithEmoji(), - } - opts = append(defaultOpts, opts...) - - return render(text, opts) -} - -func RenderWithBaseURL(text, style, baseURL string) (string, error) { - opts := RenderOpts{ - glamour.WithStylePath(style), - glamour.WithEmoji(), - glamour.WithBaseURL(baseURL), - } - - return render(text, opts) -} - -func RenderWithWrap(text, style string, wrap int) (string, error) { - opts := RenderOpts{ - glamour.WithStylePath(style), - glamour.WithEmoji(), - glamour.WithWordWrap(wrap), - } - - return render(text, opts) -} - -func GetStyle(defaultStyle string) string { - style := fromEnv() - if style != "" && style != "auto" { - return style - } - - if defaultStyle == "light" || defaultStyle == "dark" { - return defaultStyle - } - - return "notty" -} - -var fromEnv = func() string { - return os.Getenv("GLAMOUR_STYLE") -} diff --git a/pkg/markdown/markdown_test.go b/pkg/markdown/markdown_test.go index 079b5ff7a..77b56d6e9 100644 --- a/pkg/markdown/markdown_test.go +++ b/pkg/markdown/markdown_test.go @@ -1,50 +1,46 @@ package markdown import ( + "os" "testing" "github.com/stretchr/testify/assert" ) func Test_Render(t *testing.T) { + os.Unsetenv("GLAMOUR_STYLE") + type input struct { text string - style string - } - type output struct { - wantsErr bool + theme string } tests := []struct { - name string - input input - output output + name string + input input + wantsErr bool }{ { - name: "invalid glamour style", + name: "light theme", input: input{ text: "some text", - style: "invalid", - }, - output: output{ - wantsErr: true, + theme: "light", }, + wantsErr: false, }, { - name: "valid glamour style", + name: "dark theme", input: input{ text: "some text", - style: "dark", - }, - output: output{ - wantsErr: false, + theme: "dark", }, + wantsErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := Render(tt.input.text, tt.input.style) - if tt.output.wantsErr { + _, err := Render(tt.input.text, WithIO(terminalThemer(tt.input.theme))) + if tt.wantsErr { assert.Error(t, err) return } @@ -53,89 +49,8 @@ func Test_Render(t *testing.T) { } } -func Test_GetStyle(t *testing.T) { - type input struct { - glamourStyle string - terminalTheme string - } - type output struct { - style string - } - tests := []struct { - name string - input input - output output - }{ - { - name: "no glamour style and no terminal theme", - input: input{ - glamourStyle: "", - terminalTheme: "none", - }, - output: output{ - style: "notty", - }, - }, - { - name: "auto glamour style and no terminal theme", - input: input{ - glamourStyle: "auto", - terminalTheme: "none", - }, - output: output{ - style: "notty", - }, - }, - { - name: "user glamour style and no terminal theme", - input: input{ - glamourStyle: "somestyle", - terminalTheme: "none", - }, - output: output{ - style: "somestyle", - }, - }, - { - name: "no glamour style and light terminal theme", - input: input{ - glamourStyle: "", - terminalTheme: "light", - }, - output: output{ - style: "light", - }, - }, - { - name: "no glamour style and dark terminal theme", - input: input{ - glamourStyle: "", - terminalTheme: "dark", - }, - output: output{ - style: "dark", - }, - }, - { - name: "no glamour style and unknown terminal theme", - input: input{ - glamourStyle: "", - terminalTheme: "unknown", - }, - output: output{ - style: "notty", - }, - }, - } +type terminalThemer string - for _, tt := range tests { - fromEnv = func() string { - return tt.input.glamourStyle - } - - t.Run(tt.name, func(t *testing.T) { - style := GetStyle(tt.input.terminalTheme) - assert.Equal(t, tt.output.style, style) - }) - } +func (tt terminalThemer) TerminalTheme() string { + return string(tt) } From 4e1bca736fec3dc4b1b985f88ebb41f7b0e21cf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 13 Jan 2022 19:41:01 +0100 Subject: [PATCH 13/19] Preserve newlines in GitHub-flavored markdown --- go.mod | 5 ++--- go.sum | 43 ++++++++++++---------------------------- pkg/markdown/markdown.go | 2 +- 3 files changed, 16 insertions(+), 34 deletions(-) diff --git a/go.mod b/go.mod index ca455277d..91f708819 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/AlecAivazis/survey/v2 v2.3.2 github.com/MakeNowJust/heredoc v1.0.0 github.com/briandowns/spinner v1.18.0 - github.com/charmbracelet/glamour v0.3.0 + github.com/charmbracelet/glamour v0.4.0 github.com/cli/browser v1.1.0 github.com/cli/oauth v0.9.0 github.com/cli/safeexec v1.0.0 @@ -26,8 +26,7 @@ require ( github.com/mattn/go-colorable v0.1.12 github.com/mattn/go-isatty v0.0.14 github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d - github.com/microcosm-cc/bluemonday v1.0.16 // indirect - github.com/muesli/reflow v0.2.1-0.20210502190812-c80126ec2ad5 + github.com/muesli/reflow v0.3.0 github.com/muesli/termenv v0.9.0 github.com/muhammadmuzzammil1998/jsonc v0.0.0-20201229145248-615b0916ca38 github.com/opentracing/opentracing-go v1.1.0 diff --git a/go.sum b/go.sum index f2c325538..44e602489 100644 --- a/go.sum +++ b/go.sum @@ -56,15 +56,8 @@ github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6 github.com/Netflix/go-expect v0.0.0-20180615182759-c93bf25de8e8 h1:xzYJEypr/85nBpB11F9br+3HUrpgb+fcm5iADzXXYEw= github.com/Netflix/go-expect v0.0.0-20180615182759-c93bf25de8e8/go.mod h1:oX5x61PbNXchhh0oikYAH+4Pcfw5LKv21+Jnpr6r6Pc= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= -github.com/alecthomas/assert v0.0.0-20170929043011-405dbfeb8e38 h1:smF2tmSOzy2Mm+0dGI2AIUHY+w0BUc+4tn40djz7+6U= -github.com/alecthomas/assert v0.0.0-20170929043011-405dbfeb8e38/go.mod h1:r7bzyVFMNntcxPZXK3/+KdruV1H5KSlyVY0gc+NgInI= -github.com/alecthomas/chroma v0.8.2 h1:x3zkuE2lUk/RIekyAJ3XRqSCP4zwWDfcw/YJCuCAACg= -github.com/alecthomas/chroma v0.8.2/go.mod h1:sko8vR34/90zvl5QdcUdvzL3J8NKjAUx9va9jPuFNoM= -github.com/alecthomas/colour v0.0.0-20160524082231-60882d9e2721 h1:JHZL0hZKJ1VENNfmXvHbgYlbUOvpzYzvy2aZU5gXVeo= -github.com/alecthomas/colour v0.0.0-20160524082231-60882d9e2721/go.mod h1:QO9JBoKquHd+jz9nshCh40fOfO+JzsoXy8qTHF68zU0= -github.com/alecthomas/kong v0.2.4/go.mod h1:kQOmtJgV+Lb4aj+I2LEn40cbtawdWJ9Y8QLq+lElKxE= -github.com/alecthomas/repr v0.0.0-20180818092828-117648cd9897 h1:p9Sln00KOTlrYkxI1zYWl1QLnEqAqEARBEYa8FQnQcY= -github.com/alecthomas/repr v0.0.0-20180818092828-117648cd9897/go.mod h1:xTS7Pm1pD1mvyM075QCDSRqH6qRLXylzS24ZTpRiSzQ= +github.com/alecthomas/chroma v0.10.0 h1:7XDcGkCQopCNKjZHfYrNLraA+M7e0fMiJ/Mfikbfjek= +github.com/alecthomas/chroma v0.10.0/go.mod h1:jtJATyUxlIORhUOFNA9NZDWGAQ8wpxQQqNSB4rjA/1s= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= @@ -88,8 +81,8 @@ github.com/census-instrumentation/opencensus-proto v0.3.0/go.mod h1:f6KPmirojxKA github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/cespare/xxhash/v2 v2.1.2/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/charmbracelet/glamour v0.3.0 h1:3H+ZrKlSg8s+WU6V7eF2eRVYt8lCueffbi7r2+ffGkc= -github.com/charmbracelet/glamour v0.3.0/go.mod h1:TzF0koPZhqq0YVBNL100cPHznAAjVj7fksX2RInwjGw= +github.com/charmbracelet/glamour v0.4.0 h1:scR+smyB7WdmrlIaff6IVlm48P48JaNM7JypM/VGl4k= +github.com/charmbracelet/glamour v0.4.0/go.mod h1:9ZRtG19AUIzcTm7FGLGbq3D5WKQ5UyZBbQsMQN0XIqc= github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI= github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI= github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU= @@ -123,13 +116,11 @@ github.com/cpuguy83/go-md2man/v2 v2.0.1 h1:r/myEWzV9lfsM1tFLgDyu0atFtJ1fXn261LKY github.com/cpuguy83/go-md2man/v2 v2.0.1/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.17 h1:QeVUsEDNrLBW4tMgZHvxy18sKtr6VI492kBhUfhDJNI= github.com/creack/pty v1.1.17/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= -github.com/danwakefield/fnmatch v0.0.0-20160403171240-cbb64ac3d964 h1:y5HC9v93H5EPKqaS1UYVg1uYah5Xf51mBfIoWehClUQ= -github.com/danwakefield/fnmatch v0.0.0-20160403171240-cbb64ac3d964/go.mod h1:Xd9hchkHSWYkEqJwUGisez3G1QY8Ryz0sdWrLPMGjLk= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/dlclark/regexp2 v1.2.0 h1:8sAhBGEM0dRWogWqWyQeIJnxjWO6oIjl8FKqREDsGfk= -github.com/dlclark/regexp2 v1.2.0/go.mod h1:2pZnwuY/m+8K6iRw6wQdMtk+rH5tNGR1i55kozfMjCc= +github.com/dlclark/regexp2 v1.4.0 h1:F1rxgk7p4uKjwIQxBs9oAXe5CqrXlCduYEJvrF4u93E= +github.com/dlclark/regexp2 v1.4.0/go.mod h1:2pZnwuY/m+8K6iRw6wQdMtk+rH5tNGR1i55kozfMjCc= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= @@ -333,16 +324,15 @@ github.com/mattn/go-isatty v0.0.13/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Ky github.com/mattn/go-isatty v0.0.14 h1:yVuAays6BHfxijgZPzw+3Zlu5yQgKGP2/hcQbHb7S9Y= github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI= -github.com/mattn/go-runewidth v0.0.10/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk= +github.com/mattn/go-runewidth v0.0.12/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk= github.com/mattn/go-runewidth v0.0.13 h1:lTGmDsbAYt5DmK6OnoV7EuIF1wEIFAcxld6ypU4OSgU= github.com/mattn/go-runewidth v0.0.13/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE= github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d h1:5PJl274Y63IEHC+7izoQE9x6ikvDFZS2mDVS3drnohI= github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE= -github.com/microcosm-cc/bluemonday v1.0.6/go.mod h1:HOT/6NaBlR0f9XlxD3zolN6Z3N8Lp4pvhp+jLS5ihnI= -github.com/microcosm-cc/bluemonday v1.0.16 h1:kHmAq2t7WPWLjiGvzKa5o3HzSfahUKiOq7fAPUiMNIc= -github.com/microcosm-cc/bluemonday v1.0.16/go.mod h1:Z0r70sCuXHig8YpBzCc5eGHAap2K7e/u082ZUpDRRqM= +github.com/microcosm-cc/bluemonday v1.0.17 h1:Z1a//hgsQ4yjC+8zEkV8IWySkXnsxmdSY642CTFQb5Y= +github.com/microcosm-cc/bluemonday v1.0.17/go.mod h1:Z0r70sCuXHig8YpBzCc5eGHAap2K7e/u082ZUpDRRqM= github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg= github.com/miekg/dns v1.1.26/go.mod h1:bPDLeHnStXmXAq1m/Ch/hvfNHr14JKNPMBo3VZKjuso= github.com/miekg/dns v1.1.41/go.mod h1:p6aan82bvRIyn+zDIv9xYNUpwa73JcSh9BKwknJysuI= @@ -357,10 +347,8 @@ github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJ github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0= github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0= github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= -github.com/muesli/reflow v0.2.0/go.mod h1:qT22vjVmM9MIUeLgsVYe/Ye7eZlbv9dZjL3dVhUqLX8= -github.com/muesli/reflow v0.2.1-0.20210502190812-c80126ec2ad5 h1:T+Fc6qGlSfM+z0JPlp+n5rijvlg6C6JYFSNaqnCifDU= -github.com/muesli/reflow v0.2.1-0.20210502190812-c80126ec2ad5/go.mod h1:Xk+z4oIWdQqJzsxyjgl3P22oYZnHdZ8FFTHAQQt5BMQ= -github.com/muesli/termenv v0.8.1/go.mod h1:kzt/D/4a88RoheZmwfqorY3A+tnsSMA9HJC/fQSFKo0= +github.com/muesli/reflow v0.3.0 h1:IFsN6K9NfGtjeggFP+68I4chLZV2yIKsXJFNZ+eWh6s= +github.com/muesli/reflow v0.3.0/go.mod h1:pbwTDkVPibjO2kyvBQRBxTWEEGDGq0FlB1BIKtnHY/8= github.com/muesli/termenv v0.9.0 h1:wnbOaGz+LUR3jNT0zOzinPnyDaCZUQRZj9GxK8eRVl8= github.com/muesli/termenv v0.9.0/go.mod h1:R/LzAKf+suGs4IsO95y7+7DpFHO0KABgnZqtlyx2mBw= github.com/muhammadmuzzammil1998/jsonc v0.0.0-20201229145248-615b0916ca38 h1:0FrBxrkJ0hVembTb/e4EU5Ml6vLcOusAqymmYISg5Uo= @@ -375,7 +363,6 @@ github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144T github.com/pelletier/go-toml v1.9.4/go.mod h1:u1nR/EPcESfeI/szUZKdtJ0xRNbUoANCkoOuaOx1Y+c= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= -github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/sftp v1.10.1/go.mod h1:lYOWFsE0bwd1+KfKJaKeuokY15vzFx25BLbzYYoAxZI= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -403,8 +390,6 @@ github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQD github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= github.com/sagikazarmark/crypt v0.3.0/go.mod h1:uD/D+6UF4SrIR1uGEv7bBNkNqLGqUr43MRiaGWX1Nig= github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc= -github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ= -github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo= github.com/shurcooL/githubv4 v0.0.0-20200928013246-d292edc3691b h1:0/ecDXh/HTHRtSDSFnD2/Ta1yQ5J76ZspVY4u0/jGFk= github.com/shurcooL/githubv4 v0.0.0-20200928013246-d292edc3691b/go.mod h1:hAF0iLZy4td2EX+/8Tw+4nodhlMrwN3HupfaXj3zkGo= github.com/shurcooL/graphql v0.0.0-20200928012149-18c5c3165e3a h1:KikTa6HtAK8cS1qjvUvvq4QO21QnwC+EfvB+OAuZ/ZU= @@ -440,9 +425,9 @@ github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= -github.com/yuin/goldmark v1.3.3/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= -github.com/yuin/goldmark v1.3.5 h1:dPmz1Snjq0kmkz159iL7S6WzdahUTHnHB5M56WFVifs= github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= +github.com/yuin/goldmark v1.4.4 h1:zNWRjYUW32G9KirMXYHQHVNFkXvMI7LpgNW2AgYAoIs= +github.com/yuin/goldmark v1.4.4/go.mod h1:rmuwmfZ0+bvzB24eSC//bk1R1Zp3hM0OXYv/G2LIilg= github.com/yuin/goldmark-emoji v1.0.1 h1:ctuWEyzGBwiucEqxzwe0SOYDXPAucOrE9NQC18Wa1os= github.com/yuin/goldmark-emoji v1.0.1/go.mod h1:2w1E6FEWLcDQkoTE+7HU6QF1F6SLlNGjRIBbIZQFqkQ= go.etcd.io/etcd/api/v3 v3.5.1/go.mod h1:cbVKeC6lCfl7j/8jBhAK6aIYO9XOjdptoxU/nLQcPvs= @@ -531,7 +516,6 @@ golang.org/x/net v0.0.0-20201209123823-ac852fbbde11/go.mod h1:m0MpNAwzfU5UDzcl9v golang.org/x/net v0.0.0-20210119194325-5f4716e94777/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4/go.mod h1:RBQZq4jEuRlivfhVLdyRGr576XBO4/greRjx4P4O3yc= -golang.org/x/net v0.0.0-20210331212208-0fccb6fa2b5c/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20210410081132-afb366fc7cd1/go.mod h1:9tjilg8BloeKEkVJvy7fQ90B1CfIiPueXVOjqfkSzI8= golang.org/x/net v0.0.0-20210503060351-7fd8e65b6420/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= @@ -601,7 +585,6 @@ golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200331124033-c3d80250170d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200413165638-669c56c373c4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200501052902-10377860bb8e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200511232937-7e40ca221e25/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200515095857-1151b9dac4a9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= diff --git a/pkg/markdown/markdown.go b/pkg/markdown/markdown.go index cd578bf06..32313a15c 100644 --- a/pkg/markdown/markdown.go +++ b/pkg/markdown/markdown.go @@ -52,7 +52,7 @@ func Render(text string, opts ...glamour.TermRendererOption) (string, error) { // we need to ensure that no such characters are present in the output. text = strings.ReplaceAll(text, "\r\n", "\n") - opts = append(opts, glamour.WithEmoji()) + opts = append(opts, glamour.WithEmoji(), glamour.WithPreservedNewLines()) tr, err := glamour.NewTermRenderer(opts...) if err != nil { return "", err From 659eed777da037fb4cd9327d39eb6750aaf65075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 14 Jan 2022 15:42:02 +0100 Subject: [PATCH 14/19] Have TerminalTheme invoke DetectTerminalTheme if necessary --- pkg/iostreams/iostreams.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index e07111450..cf7a0b04d 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -69,35 +69,37 @@ func (s *IOStreams) HasTrueColor() bool { return s.hasTrueColor } -func (s *IOStreams) DetectTerminalTheme() string { +// DetectTerminalTheme is a utility to call before starting the output pager so that the terminal background +// can be reliably detected. +func (s *IOStreams) DetectTerminalTheme() { if !s.ColorEnabled() { s.terminalTheme = "none" - return "none" + return } if s.pagerProcess != nil { s.terminalTheme = "none" - return "none" + return } style := os.Getenv("GLAMOUR_STYLE") if style != "" && style != "auto" { s.terminalTheme = "none" - return "none" + return } if termenv.HasDarkBackground() { s.terminalTheme = "dark" - return "dark" + return } s.terminalTheme = "light" - return "light" } +// TerminalTheme returns "light", "dark", or "none" depending on the background color of the terminal. func (s *IOStreams) TerminalTheme() string { if s.terminalTheme == "" { - return "none" + s.DetectTerminalTheme() } return s.terminalTheme From a33b5a55c437bfb6ddfba39b53507392ac832f52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 14 Jan 2022 18:52:00 +0100 Subject: [PATCH 15/19] Have `NewAskStubber` perform auto-cleanup --- pkg/cmd/gist/view/view_test.go | 6 ++--- pkg/cmd/pr/review/review_test.go | 3 +-- pkg/cmd/workflow/enable/enable_test.go | 3 +-- pkg/prompt/stubber.go | 35 +++++++++++++------------- 4 files changed, 21 insertions(+), 26 deletions(-) diff --git a/pkg/cmd/gist/view/view_test.go b/pkg/cmd/gist/view/view_test.go index dbacd9101..952490d7d 100644 --- a/pkg/cmd/gist/view/view_test.go +++ b/pkg/cmd/gist/view/view_test.go @@ -355,8 +355,7 @@ func Test_viewRun(t *testing.T) { )), ) - as, surveyteardown := prompt.NewAskStubber() - defer surveyteardown(t) + as := prompt.NewAskStubber(t) as.StubPrompt("Select a gist").AnswerDefault() } @@ -470,8 +469,7 @@ func Test_promptGists(t *testing.T) { client := &http.Client{Transport: reg} t.Run(tt.name, func(t *testing.T) { - as, surveyteardown := prompt.NewAskStubber() - defer surveyteardown(t) + as := prompt.NewAskStubber(t) if tt.askStubs != nil { tt.askStubs(as) } diff --git a/pkg/cmd/pr/review/review_test.go b/pkg/cmd/pr/review/review_test.go index 87ec8e984..ced1736d9 100644 --- a/pkg/cmd/pr/review/review_test.go +++ b/pkg/cmd/pr/review/review_test.go @@ -309,8 +309,7 @@ func TestPRReview_interactive_no_body(t *testing.T) { shared.RunCommandFinder("", &api.PullRequest{ID: "THE-ID", Number: 123}, ghrepo.New("OWNER", "REPO")) - as, teardown := prompt.NewAskStubber() - defer teardown(t) + as := prompt.NewAskStubber(t) as.StubPrompt("What kind of review do you want to give?").AnswerWith("Request changes") as.StubPrompt("Review body").AnswerWith("") diff --git a/pkg/cmd/workflow/enable/enable_test.go b/pkg/cmd/workflow/enable/enable_test.go index 7cc3fbd47..cfc9ea79c 100644 --- a/pkg/cmd/workflow/enable/enable_test.go +++ b/pkg/cmd/workflow/enable/enable_test.go @@ -277,8 +277,7 @@ func TestEnableRun(t *testing.T) { } t.Run(tt.name, func(t *testing.T) { - as, teardown := prompt.NewAskStubber() - defer teardown(t) + as := prompt.NewAskStubber(t) if tt.askStubs != nil { tt.askStubs(as) } diff --git a/pkg/prompt/stubber.go b/pkg/prompt/stubber.go index f01a28dcf..8d9e22520 100644 --- a/pkg/prompt/stubber.go +++ b/pkg/prompt/stubber.go @@ -15,9 +15,24 @@ type AskStubber struct { type testing interface { Errorf(format string, args ...interface{}) + Cleanup(func()) } -func NewAskStubber() (*AskStubber, func(t testing)) { +func NewAskStubber(t testing) *AskStubber { + as, teardown := InitAskStubber() + t.Cleanup(func() { + teardown() + for _, s := range as.stubs { + if !s.matched { + t.Errorf("unmatched prompt stub: %+v", s) + } + } + }) + return as +} + +// Deprecated: use NewAskStubber +func InitAskStubber() (*AskStubber, func()) { origSurveyAsk := SurveyAsk origSurveyAskOne := SurveyAskOne as := AskStubber{} @@ -124,29 +139,13 @@ func NewAskStubber() (*AskStubber, func(t testing)) { return nil } - teardown := func(t testing) { + teardown := func() { SurveyAsk = origSurveyAsk SurveyAskOne = origSurveyAskOne - for _, s := range as.stubs { - if !s.matched { - if t == nil { - panic(fmt.Sprintf("unmatched prompt stub: %+v", s)) - } else { - t.Errorf("unmatched prompt stub: %+v", s) - } - } - } } return &as, teardown } -func InitAskStubber() (*AskStubber, func()) { - as, teardown := NewAskStubber() - return as, func() { - teardown(nil) - } -} - type QuestionStub struct { Name string Value interface{} From e43cb2b880df8c8d0419ce2dd232e04efcab5dbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 14 Jan 2022 19:34:15 +0100 Subject: [PATCH 16/19] Port more legacy stubs to the new ask stubber --- pkg/cmd/auth/login/login_test.go | 3 +- pkg/cmd/auth/logout/logout_test.go | 11 ++-- pkg/cmd/auth/refresh/refresh_test.go | 5 +- pkg/cmd/auth/shared/login_flow_test.go | 13 +++-- pkg/cmd/extension/command_test.go | 9 ++-- pkg/cmd/gist/delete/delete_test.go | 8 --- pkg/cmd/gist/edit/edit_test.go | 19 ++++--- pkg/cmd/issue/create/create_test.go | 65 ++++-------------------- pkg/cmd/issue/delete/delete_test.go | 12 ++--- pkg/cmd/run/watch/watch_test.go | 27 ++++++---- pkg/cmd/secret/set/set_test.go | 6 +-- pkg/cmd/workflow/disable/disable_test.go | 15 +++--- pkg/cmd/workflow/run/run_test.go | 26 ++++------ pkg/cmd/workflow/view/view_test.go | 8 --- 14 files changed, 80 insertions(+), 147 deletions(-) diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index dc4bab91e..b7c8438cb 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -548,8 +548,7 @@ func Test_loginRun_Survey(t *testing.T) { hostsBuf := bytes.Buffer{} defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - as, teardown := prompt.InitAskStubber() - defer teardown() + as := prompt.NewAskStubber(t) if tt.askStubs != nil { tt.askStubs(as) } diff --git a/pkg/cmd/auth/logout/logout_test.go b/pkg/cmd/auth/logout/logout_test.go index a5d46ef09..4d74caaf5 100644 --- a/pkg/cmd/auth/logout/logout_test.go +++ b/pkg/cmd/auth/logout/logout_test.go @@ -106,8 +106,8 @@ func Test_logoutRun_tty(t *testing.T) { cfgHosts: []string{"cheryl.mason", "github.com"}, wantHosts: "cheryl.mason:\n oauth_token: abc123\n", askStubs: func(as *prompt.AskStubber) { - as.StubOne("github.com") - as.StubOne(true) + as.StubPrompt("What account do you want to log out of?").AnswerWith("github.com") + as.StubPrompt("Are you sure you want to log out of github.com account 'cybilb'?").AnswerWith(true) }, wantErrOut: regexp.MustCompile(`Logged out of github.com account 'cybilb'`), }, @@ -116,7 +116,7 @@ func Test_logoutRun_tty(t *testing.T) { opts: &LogoutOptions{}, cfgHosts: []string{"github.com"}, askStubs: func(as *prompt.AskStubber) { - as.StubOne(true) + as.StubPrompt("Are you sure you want to log out of github.com account 'cybilb'?").AnswerWith(true) }, wantErrOut: regexp.MustCompile(`Logged out of github.com account 'cybilb'`), }, @@ -133,7 +133,7 @@ func Test_logoutRun_tty(t *testing.T) { cfgHosts: []string{"cheryl.mason", "github.com"}, wantHosts: "github.com:\n oauth_token: abc123\n", askStubs: func(as *prompt.AskStubber) { - as.StubOne(true) + as.StubPrompt("Are you sure you want to log out of cheryl.mason account 'cybilb'?").AnswerWith(true) }, wantErrOut: regexp.MustCompile(`Logged out of cheryl.mason account 'cybilb'`), }, @@ -169,8 +169,7 @@ func Test_logoutRun_tty(t *testing.T) { hostsBuf := bytes.Buffer{} defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - as, teardown := prompt.InitAskStubber() - defer teardown() + as := prompt.NewAskStubber(t) if tt.askStubs != nil { tt.askStubs(as) } diff --git a/pkg/cmd/auth/refresh/refresh_test.go b/pkg/cmd/auth/refresh/refresh_test.go index 800913b1a..1bee8435d 100644 --- a/pkg/cmd/auth/refresh/refresh_test.go +++ b/pkg/cmd/auth/refresh/refresh_test.go @@ -194,7 +194,7 @@ func Test_refreshRun(t *testing.T) { Hostname: "", }, askStubs: func(as *prompt.AskStubber) { - as.StubOne("github.com") + as.StubPrompt("What account do you want to refresh auth for?").AnswerWith("github.com") }, wantAuthArgs: authArgs{ hostname: "github.com", @@ -276,8 +276,7 @@ func Test_refreshRun(t *testing.T) { hostsBuf := bytes.Buffer{} defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - as, teardown := prompt.InitAskStubber() - defer teardown() + as := prompt.NewAskStubber(t) if tt.askStubs != nil { tt.askStubs(as) } diff --git a/pkg/cmd/auth/shared/login_flow_test.go b/pkg/cmd/auth/shared/login_flow_test.go index 530e34045..6f1b35ebe 100644 --- a/pkg/cmd/auth/shared/login_flow_test.go +++ b/pkg/cmd/auth/shared/login_flow_test.go @@ -47,14 +47,13 @@ func TestLogin_ssh(t *testing.T) { httpmock.REST("POST", "api/v3/user/keys"), httpmock.StringResponse(`{}`)) - ask, askRestore := prompt.InitAskStubber() - defer askRestore() + ask := prompt.NewAskStubber(t) - ask.StubOne("SSH") // preferred protocol - ask.StubOne(true) // generate a new key - ask.StubOne("monkey") // enter a passphrase - ask.StubOne(1) // paste a token - ask.StubOne("ATOKEN") // token + ask.StubPrompt("What is your preferred protocol for Git operations?").AnswerWith("SSH") + ask.StubPrompt("Generate a new SSH key to add to your GitHub account?").AnswerWith(true) + ask.StubPrompt("Enter a passphrase for your new SSH key (Optional)").AnswerWith("monkey") + ask.StubPrompt("How would you like to authenticate GitHub CLI?").AnswerWith("Paste an authentication token") + ask.StubPrompt("Paste your authentication token:").AnswerWith("ATOKEN") rs, runRestore := run.Stub() defer runRestore(t) diff --git a/pkg/cmd/extension/command_test.go b/pkg/cmd/extension/command_test.go index 5a46592f9..8f896eab0 100644 --- a/pkg/cmd/extension/command_test.go +++ b/pkg/cmd/extension/command_test.go @@ -316,8 +316,10 @@ func TestNewCmdExtension(t *testing.T) { }, isTTY: true, askStubs: func(as *prompt.AskStubber) { - as.StubOne("test") - as.StubOne(0) + as.StubPrompt("Extension name:").AnswerWith("test") + as.StubPrompt("What kind of extension?"). + AssertOptions([]string{"Script (Bash, Ruby, Python, etc)", "Go", "Other Precompiled (C++, Rust, etc)"}). + AnswerDefault() }, wantStdout: heredoc.Doc(` ✓ Created directory gh-test @@ -456,8 +458,7 @@ func TestNewCmdExtension(t *testing.T) { assertFunc = tt.managerStubs(em) } - as, teardown := prompt.InitAskStubber() - defer teardown() + as := prompt.NewAskStubber(t) if tt.askStubs != nil { tt.askStubs(as) } diff --git a/pkg/cmd/gist/delete/delete_test.go b/pkg/cmd/gist/delete/delete_test.go index 0d98a44c2..9d6574911 100644 --- a/pkg/cmd/gist/delete/delete_test.go +++ b/pkg/cmd/gist/delete/delete_test.go @@ -10,7 +10,6 @@ import ( "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -61,7 +60,6 @@ func Test_deleteRun(t *testing.T) { opts *DeleteOptions gist *shared.Gist httpStubs func(*httpmock.Registry) - askStubs func(*prompt.AskStubber) nontty bool wantErr bool wantStderr string @@ -122,12 +120,6 @@ func Test_deleteRun(t *testing.T) { tt.httpStubs(reg) } - as, teardown := prompt.InitAskStubber() - defer teardown() - if tt.askStubs != nil { - tt.askStubs(as) - } - if tt.opts == nil { tt.opts = &DeleteOptions{} } diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index cac99b923..74c4f3502 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -146,8 +146,8 @@ func Test_editRun(t *testing.T) { { name: "multiple files, submit", askStubs: func(as *prompt.AskStubber) { - as.StubOne("unix.md") - as.StubOne("Submit") + as.StubPrompt("Edit which file?").AnswerWith("unix.md") + as.StubPrompt("What next?").AnswerWith("Submit") }, gist: &shared.Gist{ ID: "1234", @@ -191,8 +191,8 @@ func Test_editRun(t *testing.T) { { name: "multiple files, cancel", askStubs: func(as *prompt.AskStubber) { - as.StubOne("unix.md") - as.StubOne("Cancel") + as.StubPrompt("Edit which file?").AnswerWith("unix.md") + as.StubPrompt("What next?").AnswerWith("Cancel") }, wantErr: "CancelError", gist: &shared.Gist{ @@ -280,12 +280,6 @@ func Test_editRun(t *testing.T) { tt.httpStubs(reg) } - as, teardown := prompt.InitAskStubber() - defer teardown() - if tt.askStubs != nil { - tt.askStubs(as) - } - if tt.opts == nil { tt.opts = &EditOptions{} } @@ -308,6 +302,11 @@ func Test_editRun(t *testing.T) { } t.Run(tt.name, func(t *testing.T) { + as := prompt.NewAskStubber(t) + if tt.askStubs != nil { + tt.askStubs(as) + } + err := editRun(tt.opts) reg.Verify(t) if tt.wantErr != "" { diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index f8d948999..4eaea333a 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -401,27 +401,11 @@ func TestIssueCreate_recover(t *testing.T) { assert.Equal(t, []interface{}{"BUGID", "TODOID"}, inputs["labelIds"]) })) - as, teardown := prompt.InitAskStubber() - defer teardown() + as := prompt.NewAskStubber(t) - as.Stub([]*prompt.QuestionStub{ - { - Name: "Title", - Default: true, - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "Body", - Default: true, - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "confirmation", - Value: 0, - }, - }) + as.StubPrompt("Title").AnswerDefault() + as.StubPrompt("Body").AnswerDefault() + as.StubPrompt("What's next?").AnswerWith("Submit") tmpfile, err := ioutil.TempFile(t.TempDir(), "testrecover*") assert.NoError(t, err) @@ -484,25 +468,11 @@ func TestIssueCreate_nonLegacyTemplate(t *testing.T) { }), ) - as, teardown := prompt.InitAskStubber() - defer teardown() + as := prompt.NewAskStubber(t) - // template - as.StubOne(1) - // body - as.Stub([]*prompt.QuestionStub{ - { - Name: "Body", - Default: true, - }, - }) // body - // confirm - as.Stub([]*prompt.QuestionStub{ - { - Name: "confirmation", - Value: 0, - }, - }) + as.StubPrompt("Choose a template").AnswerWith("Submit a request") + as.StubPrompt("Body").AnswerDefault() + as.StubPrompt("What's next?").AnswerWith("Submit") output, err := runCommandWithRootDirOverridden(http, true, `-t hello`, "./fixtures/repoWithNonLegacyIssueTemplates") if err != nil { @@ -526,23 +496,10 @@ func TestIssueCreate_continueInBrowser(t *testing.T) { } } }`), ) - as, teardown := prompt.InitAskStubber() - defer teardown() + as := prompt.NewAskStubber(t) - // title - as.Stub([]*prompt.QuestionStub{ - { - Name: "Title", - Value: "hello", - }, - }) - // confirm - as.Stub([]*prompt.QuestionStub{ - { - Name: "confirmation", - Value: 1, - }, - }) + as.StubPrompt("Title").AnswerWith("hello") + as.StubPrompt("What's next?").AnswerWith("Continue in browser") _, cmdTeardown := run.Stub() defer cmdTeardown(t) diff --git a/pkg/cmd/issue/delete/delete_test.go b/pkg/cmd/issue/delete/delete_test.go index 3d5fd1761..d9cf089c3 100644 --- a/pkg/cmd/issue/delete/delete_test.go +++ b/pkg/cmd/issue/delete/delete_test.go @@ -75,9 +75,9 @@ func TestIssueDelete(t *testing.T) { assert.Equal(t, inputs["issueId"], "THE-ID") }), ) - as, teardown := prompt.InitAskStubber() - defer teardown() - as.StubOne("13") + + as := prompt.NewAskStubber(t) + as.StubPrompt("You're going to delete issue #13. This action cannot be reversed. To confirm, type the issue number:").AnswerWith("13") output, err := runCommand(httpRegistry, true, "13") if err != nil { @@ -103,9 +103,9 @@ func TestIssueDelete_cancel(t *testing.T) { "issue": { "id": "THE-ID", "number": 13, "title": "The title of the issue"} } } }`), ) - as, teardown := prompt.InitAskStubber() - defer teardown() - as.StubOne("14") + + as := prompt.NewAskStubber(t) + as.StubPrompt("You're going to delete issue #13. This action cannot be reversed. To confirm, type the issue number:").AnswerWith("14") output, err := runCommand(httpRegistry, true, "13") if err != nil { diff --git a/pkg/cmd/run/watch/watch_test.go b/pkg/cmd/run/watch/watch_test.go index a96f826f4..a21df2217 100644 --- a/pkg/cmd/run/watch/watch_test.go +++ b/pkg/cmd/run/watch/watch_test.go @@ -234,7 +234,9 @@ func TestWatchRun(t *testing.T) { }, httpStubs: successfulRunStubs, askStubs: func(as *prompt.AskStubber) { - as.StubOne(1) + as.StubPrompt("Select a workflow run"). + AssertOptions([]string{"* cool commit, run (trunk) Feb 23, 2021", "* cool commit, more runs (trunk) Feb 23, 2021"}). + AnswerWith("* cool commit, more runs (trunk) Feb 23, 2021") }, wantOut: "\x1b[2J\x1b[0;0H\x1b[JRefreshing run status every 0 seconds. Press Ctrl+C to quit.\n\n* trunk more runs · 2\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\x1b[0;0H\x1b[JRefreshing run status every 0 seconds. Press Ctrl+C to quit.\n\n✓ trunk more runs · 2\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\n✓ Run more runs (2) completed with 'success'\n", }, @@ -247,7 +249,9 @@ func TestWatchRun(t *testing.T) { }, httpStubs: successfulRunStubs, askStubs: func(as *prompt.AskStubber) { - as.StubOne(1) + as.StubPrompt("Select a workflow run"). + AssertOptions([]string{"* cool commit, run (trunk) Feb 23, 2021", "* cool commit, more runs (trunk) Feb 23, 2021"}). + AnswerWith("* cool commit, more runs (trunk) Feb 23, 2021") }, wantOut: "\x1b[2J\x1b[2JRefreshing run status every 0 seconds. Press Ctrl+C to quit.\n\n* trunk more runs · 2\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\x1b[2JRefreshing run status every 0 seconds. Press Ctrl+C to quit.\n\n✓ trunk more runs · 2\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n", }, @@ -262,7 +266,9 @@ func TestWatchRun(t *testing.T) { }, httpStubs: failedRunStubs, askStubs: func(as *prompt.AskStubber) { - as.StubOne(1) + as.StubPrompt("Select a workflow run"). + AssertOptions([]string{"* cool commit, run (trunk) Feb 23, 2021", "* cool commit, more runs (trunk) Feb 23, 2021"}). + AnswerWith("* cool commit, more runs (trunk) Feb 23, 2021") }, wantOut: "\x1b[2J\x1b[0;0H\x1b[JRefreshing run status every 0 seconds. Press Ctrl+C to quit.\n\n* trunk more runs · 2\nTriggered via push about 59 minutes ago\n\n\x1b[0;0H\x1b[JRefreshing run status every 0 seconds. Press Ctrl+C to quit.\n\nX trunk more runs · 2\nTriggered via push about 59 minutes ago\n\nJOBS\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nX Run more runs (2) completed with 'failure'\n", wantErr: true, @@ -278,7 +284,9 @@ func TestWatchRun(t *testing.T) { }, httpStubs: failedRunStubs, askStubs: func(as *prompt.AskStubber) { - as.StubOne(1) + as.StubPrompt("Select a workflow run"). + AssertOptions([]string{"* cool commit, run (trunk) Feb 23, 2021", "* cool commit, more runs (trunk) Feb 23, 2021"}). + AnswerWith("* cool commit, more runs (trunk) Feb 23, 2021") }, wantOut: "\x1b[2J\x1b[2JRefreshing run status every 0 seconds. Press Ctrl+C to quit.\n\n* trunk more runs · 2\nTriggered via push about 59 minutes ago\n\n\x1b[2JRefreshing run status every 0 seconds. Press Ctrl+C to quit.\n\nX trunk more runs · 2\nTriggered via push about 59 minutes ago\n\nJOBS\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n", wantErr: true, @@ -313,13 +321,12 @@ func TestWatchRun(t *testing.T) { return ghrepo.FromFullName("OWNER/REPO") } - as, teardown := prompt.InitAskStubber() - defer teardown() - if tt.askStubs != nil { - tt.askStubs(as) - } - t.Run(tt.name, func(t *testing.T) { + as := prompt.NewAskStubber(t) + if tt.askStubs != nil { + tt.askStubs(as) + } + err := watchRun(tt.opts) if tt.wantErr { assert.EqualError(t, err, tt.errMsg) diff --git a/pkg/cmd/secret/set/set_test.go b/pkg/cmd/secret/set/set_test.go index d65ed1cae..29a2c4758 100644 --- a/pkg/cmd/secret/set/set_test.go +++ b/pkg/cmd/secret/set/set_test.go @@ -487,10 +487,8 @@ func Test_getBodyPrompt(t *testing.T) { io.SetStdinTTY(true) io.SetStdoutTTY(true) - as, teardown := prompt.InitAskStubber() - defer teardown() - - as.StubOne("cool secret") + as := prompt.NewAskStubber(t) + as.StubPrompt("Paste your secret").AnswerWith("cool secret") body, err := getBody(&SetOptions{ IO: io, diff --git a/pkg/cmd/workflow/disable/disable_test.go b/pkg/cmd/workflow/disable/disable_test.go index 1057da6a9..a1adbdd45 100644 --- a/pkg/cmd/workflow/disable/disable_test.go +++ b/pkg/cmd/workflow/disable/disable_test.go @@ -121,7 +121,7 @@ func TestDisableRun(t *testing.T) { httpmock.StatusStringResponse(204, "{}")) }, askStubs: func(as *prompt.AskStubber) { - as.StubOne(1) + as.StubPrompt("Select a workflow").AnswerWith("another workflow (another.yml)") }, wantOut: "✓ Disabled another workflow\n", }, @@ -176,7 +176,7 @@ func TestDisableRun(t *testing.T) { httpmock.StatusStringResponse(204, "{}")) }, askStubs: func(as *prompt.AskStubber) { - as.StubOne(1) + as.StubPrompt("Which workflow do you mean?").AnswerWith("another workflow (yetanother.yml)") }, wantOut: "✓ Disabled another workflow\n", }, @@ -277,13 +277,12 @@ func TestDisableRun(t *testing.T) { return ghrepo.FromFullName("OWNER/REPO") } - as, teardown := prompt.InitAskStubber() - defer teardown() - if tt.askStubs != nil { - tt.askStubs(as) - } - t.Run(tt.name, func(t *testing.T) { + as := prompt.NewAskStubber(t) + if tt.askStubs != nil { + tt.askStubs(as) + } + err := runDisable(tt.opts) if tt.wantErr { assert.Error(t, err) diff --git a/pkg/cmd/workflow/run/run_test.go b/pkg/cmd/workflow/run/run_test.go index 196f1850d..1f42164ff 100644 --- a/pkg/cmd/workflow/run/run_test.go +++ b/pkg/cmd/workflow/run/run_test.go @@ -557,7 +557,7 @@ jobs: httpmock.StatusStringResponse(204, "cool")) }, askStubs: func(as *prompt.AskStubber) { - as.StubOne(0) + as.StubPrompt("Select a workflow").AnswerDefault() }, wantBody: map[string]interface{}{ "inputs": map[string]interface{}{}, @@ -594,17 +594,9 @@ jobs: httpmock.StatusStringResponse(204, "cool")) }, askStubs: func(as *prompt.AskStubber) { - as.StubOne(0) - as.Stub([]*prompt.QuestionStub{ - { - Name: "greeting", - Default: true, - }, - { - Name: "name", - Value: "scully", - }, - }) + as.StubPrompt("Select a workflow").AnswerDefault() + as.StubPrompt("greeting").AnswerWith("hi") + as.StubPrompt("name").AnswerWith("scully") }, wantBody: map[string]interface{}{ "inputs": map[string]interface{}{ @@ -638,12 +630,12 @@ jobs: }, "github.com"), nil } - as, teardown := prompt.InitAskStubber() - defer teardown() - if tt.askStubs != nil { - tt.askStubs(as) - } t.Run(tt.name, func(t *testing.T) { + as := prompt.NewAskStubber(t) + if tt.askStubs != nil { + tt.askStubs(as) + } + err := runRun(tt.opts) if tt.wantErr { assert.Error(t, err) diff --git a/pkg/cmd/workflow/view/view_test.go b/pkg/cmd/workflow/view/view_test.go index b035070bf..dd89cf83e 100644 --- a/pkg/cmd/workflow/view/view_test.go +++ b/pkg/cmd/workflow/view/view_test.go @@ -13,7 +13,6 @@ import ( "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -189,7 +188,6 @@ func TestViewRun(t *testing.T) { name string opts *ViewOptions httpStubs func(*httpmock.Registry) - askStubs func(*prompt.AskStubber) tty bool wantOut string wantErrOut string @@ -417,12 +415,6 @@ func TestViewRun(t *testing.T) { browser := &cmdutil.TestBrowser{} tt.opts.Browser = browser - as, teardown := prompt.InitAskStubber() - defer teardown() - if tt.askStubs != nil { - tt.askStubs(as) - } - t.Run(tt.name, func(t *testing.T) { err := runView(tt.opts) if tt.wantErr { From 44775f87c8475bef0c2b3b0042600effffa014ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 14 Jan 2022 19:52:52 +0100 Subject: [PATCH 17/19] Add `nolint` directives to allow-list current lint violations --- pkg/cmd/pr/create/create_test.go | 9 +++++++ pkg/cmd/pr/merge/merge_test.go | 37 ++++++++++++++++++++------ pkg/cmd/pr/review/review_test.go | 8 ++++++ pkg/cmd/pr/shared/survey_test.go | 6 +++++ pkg/cmd/pr/shared/templates_test.go | 2 ++ pkg/cmd/release/create/create_test.go | 1 + pkg/cmd/repo/archive/archive_test.go | 3 +++ pkg/cmd/repo/create/create_test.go | 38 ++++++++++++++++++++++++--- pkg/cmd/repo/delete/delete_test.go | 4 +++ pkg/cmd/repo/fork/fork_test.go | 6 +++++ pkg/cmd/repo/rename/rename_test.go | 5 ++++ pkg/cmd/run/cancel/cancel_test.go | 2 ++ pkg/cmd/run/rerun/rerun_test.go | 2 ++ pkg/cmd/run/view/view_test.go | 14 ++++++++++ 14 files changed, 125 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 395f194a6..a6c785fdc 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -283,6 +283,7 @@ func TestPRCreate_recover(t *testing.T) { cs.Register(`git status --porcelain`, 0, "") cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "") + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, teardown := prompt.InitAskStubber() defer teardown() @@ -379,6 +380,7 @@ func TestPRCreate(t *testing.T) { cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") cs.Register(`git push --set-upstream origin HEAD:feature`, 0, "") + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() @@ -425,6 +427,7 @@ func TestPRCreate_NoMaintainerModify(t *testing.T) { cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature`, 0, "") cs.Register(`git push --set-upstream origin HEAD:feature`, 0, "") + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() @@ -476,6 +479,7 @@ func TestPRCreate_createFork(t *testing.T) { cs.Register(`git remote add -f fork https://github.com/monalisa/REPO.git`, 0, "") cs.Register(`git push --set-upstream fork HEAD:feature`, 0, "") + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() @@ -535,6 +539,7 @@ func TestPRCreate_pushedToNonBaseRepo(t *testing.T) { deadbeef refs/remotes/origin/feature `)) // determineTrackingBranch + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber _, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() @@ -576,6 +581,7 @@ func TestPRCreate_pushedToDifferentBranchName(t *testing.T) { deadbeef refs/remotes/origin/my-feat2 `)) // determineTrackingBranch + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber _, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() @@ -609,6 +615,7 @@ func TestPRCreate_nonLegacyTemplate(t *testing.T) { cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "1234567890,commit 0\n2345678901,commit 1") cs.Register(`git status --porcelain`, 0, "") + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, teardown := prompt.InitAskStubber() defer teardown() @@ -757,6 +764,7 @@ func TestPRCreate_web(t *testing.T) { cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "") cs.Register(`git push --set-upstream origin HEAD:feature`, 0, "") + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() @@ -831,6 +839,7 @@ func TestPRCreate_webProject(t *testing.T) { cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "") cs.Register(`git push --set-upstream origin HEAD:feature`, 0, "") + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber ask, cleanupAsk := prompt.InitAskStubber() defer cleanupAsk() diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index 679142b82..d692b0a08 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -769,8 +769,10 @@ func TestPrMerge_alreadyMerged(t *testing.T) { cs.Register(`git branch -D blueberries`, 0, "") cs.Register(`git pull --ff-only`, 0, "") + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, surveyTeardown := prompt.InitAskStubber() defer surveyTeardown() + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(true) output, err := runCommand(http, "blueberries", true, "pr merge 4") @@ -846,11 +848,15 @@ func TestPRMerge_interactive(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "") + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, surveyTeardown := prompt.InitAskStubber() defer surveyTeardown() - as.StubOne(0) // Merge method survey - as.StubOne(false) // Delete branch survey + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(0) // Merge method survey + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(false) // Delete branch survey + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne("Submit") // Confirm submit survey output, err := runCommand(http, "blueberries", true, "") @@ -905,10 +911,13 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { cs.Register(`git branch -D blueberries`, 0, "") cs.Register(`git pull --ff-only`, 0, "") + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, surveyTeardown := prompt.InitAskStubber() defer surveyTeardown() - as.StubOne(0) // Merge method survey + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(0) // Merge method survey + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne("Submit") // Confirm submit survey output, err := runCommand(http, "blueberries", true, "-d") @@ -964,14 +973,20 @@ func TestPRMerge_interactiveSquashEditCommitMsgAndSubject(t *testing.T) { _, cmdTeardown := run.Stub() defer cmdTeardown(t) + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, surveyTeardown := prompt.InitAskStubber() defer surveyTeardown() - as.StubOne(2) // Merge method survey - as.StubOne(false) // Delete branch survey + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(2) // Merge method survey + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(false) // Delete branch survey + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne("Edit commit subject") // Confirm submit survey + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne("Edit commit message") // Confirm submit survey - as.StubOne("Submit") // Confirm submit survey + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne("Submit") // Confirm submit survey err := mergeRun(&MergeOptions{ IO: io, @@ -1017,11 +1032,15 @@ func TestPRMerge_interactiveCancelled(t *testing.T) { cs.Register(`git rev-parse --verify refs/heads/`, 0, "") + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, surveyTeardown := prompt.InitAskStubber() defer surveyTeardown() - as.StubOne(0) // Merge method survey - as.StubOne(true) // Delete branch survey + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(0) // Merge method survey + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(true) // Delete branch survey + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne("Cancel") // Confirm submit survey output, err := runCommand(http, "blueberries", true, "") @@ -1038,8 +1057,10 @@ func Test_mergeMethodSurvey(t *testing.T) { RebaseMergeAllowed: true, SquashMergeAllowed: true, } + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, surveyTeardown := prompt.InitAskStubber() defer surveyTeardown() + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(0) // Select first option which is rebase merge method, err := mergeMethodSurvey(repo) assert.Nil(t, err) diff --git a/pkg/cmd/pr/review/review_test.go b/pkg/cmd/pr/review/review_test.go index ced1736d9..e0ee7b071 100644 --- a/pkg/cmd/pr/review/review_test.go +++ b/pkg/cmd/pr/review/review_test.go @@ -270,21 +270,25 @@ func TestPRReview_interactive(t *testing.T) { }), ) + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, teardown := prompt.InitAskStubber() defer teardown() + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ { Name: "reviewType", Value: "Approve", }, }) + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ { Name: "body", Value: "cool story", }, }) + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ { Name: "confirm", @@ -333,21 +337,25 @@ func TestPRReview_interactive_blank_approve(t *testing.T) { }), ) + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, teardown := prompt.InitAskStubber() defer teardown() + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ { Name: "reviewType", Value: "Approve", }, }) + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ { Name: "body", Default: true, }, }) + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ { Name: "confirm", diff --git a/pkg/cmd/pr/shared/survey_test.go b/pkg/cmd/pr/shared/survey_test.go index b0daa59f0..52d20d505 100644 --- a/pkg/cmd/pr/shared/survey_test.go +++ b/pkg/cmd/pr/shared/survey_test.go @@ -43,15 +43,18 @@ func TestMetadataSurvey_selectAll(t *testing.T) { }, } + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, restoreAsk := prompt.InitAskStubber() defer restoreAsk() + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ { Name: "metadata", Value: []string{"Labels", "Projects", "Assignees", "Reviewers", "Milestone"}, }, }) + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ { Name: "reviewers", @@ -109,15 +112,18 @@ func TestMetadataSurvey_keepExisting(t *testing.T) { }, } + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, restoreAsk := prompt.InitAskStubber() defer restoreAsk() + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ { Name: "metadata", Value: []string{"Labels", "Projects"}, }, }) + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ { Name: "labels", diff --git a/pkg/cmd/pr/shared/templates_test.go b/pkg/cmd/pr/shared/templates_test.go index 5ef0d7f72..03d522be6 100644 --- a/pkg/cmd/pr/shared/templates_test.go +++ b/pkg/cmd/pr/shared/templates_test.go @@ -63,9 +63,11 @@ func TestTemplateManager_hasAPI(t *testing.T) { assert.Equal(t, "LEGACY", string(m.LegacyBody())) + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, askRestore := prompt.InitAskStubber() defer askRestore() + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(1) // choose "Feature Request" tpl, err := m.Choose() assert.NoError(t, err) diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index 66dea87c0..a6f6a2d7a 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -631,6 +631,7 @@ func Test_createRun_interactive(t *testing.T) { return val, nil } + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, teardown := prompt.InitAskStubber() defer teardown() if tt.askStubs != nil { diff --git a/pkg/cmd/repo/archive/archive_test.go b/pkg/cmd/repo/archive/archive_test.go index b3feff532..a60f15765 100644 --- a/pkg/cmd/repo/archive/archive_test.go +++ b/pkg/cmd/repo/archive/archive_test.go @@ -80,6 +80,7 @@ func Test_ArchiveRun(t *testing.T) { name: "unarchived repo tty", wantStdout: "✓ Archived repository OWNER/REPO\n", askStubs: func(q *prompt.AskStubber) { + //nolint:staticcheck // SA1019: q.StubOne is deprecated: use StubPrompt q.StubOne(true) }, isTTY: true, @@ -98,6 +99,7 @@ func Test_ArchiveRun(t *testing.T) { wantStdout: "✓ Archived repository OWNER/REPO\n", opts: ArchiveOptions{}, askStubs: func(q *prompt.AskStubber) { + //nolint:staticcheck // SA1019: q.StubOne is deprecated: use StubPrompt q.StubOne(true) }, isTTY: true, @@ -138,6 +140,7 @@ func Test_ArchiveRun(t *testing.T) { io, _, stdout, stderr := iostreams.Test() tt.opts.IO = io + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber q, teardown := prompt.InitAskStubber() defer teardown() if tt.askStubs != nil { diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index d27da66b5..7c50dba46 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -171,22 +171,30 @@ func Test_createRun(t *testing.T) { tty: true, wantStdout: "✓ Created repository OWNER/REPO on GitHub\n", askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne("Create a new repository on GitHub from scratch") + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ {Name: "repoName", Value: "REPO"}, {Name: "repoDescription", Value: "my new repo"}, {Name: "repoVisibility", Value: "Private"}, }) + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ {Name: "addGitIgnore", Value: true}}) + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ {Name: "chooseGitIgnore", Value: "Go"}}) + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ {Name: "addLicense", Value: true}}) + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ {Name: "chooseLicense", Value: "GNU Lesser General Public License v3.0"}}) + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ {Name: "confirmSubmit", Value: true}}) + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(true) //clone locally? }, httpStubs: func(reg *httpmock.Registry) { @@ -210,16 +218,21 @@ func Test_createRun(t *testing.T) { opts: &CreateOptions{Interactive: true}, tty: true, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne("Create a new repository on GitHub from scratch") + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ {Name: "repoName", Value: "REPO"}, {Name: "repoDescription", Value: "my new repo"}, {Name: "repoVisibility", Value: "Private"}, }) + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ {Name: "addGitIgnore", Value: false}}) + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ {Name: "addLicense", Value: false}}) + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ {Name: "confirmSubmit", Value: false}}) }, @@ -232,13 +245,17 @@ func Test_createRun(t *testing.T) { opts: &CreateOptions{Interactive: true}, tty: true, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne("Push an existing local repository to GitHub") + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(".") + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ {Name: "repoName", Value: "REPO"}, {Name: "repoDescription", Value: "my new repo"}, {Name: "repoVisibility", Value: "Private"}, }) + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(false) }, httpStubs: func(reg *httpmock.Registry) { @@ -269,16 +286,22 @@ func Test_createRun(t *testing.T) { opts: &CreateOptions{Interactive: true}, tty: true, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne("Push an existing local repository to GitHub") + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(".") + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ {Name: "repoName", Value: "REPO"}, {Name: "repoDescription", Value: "my new repo"}, {Name: "repoVisibility", Value: "Private"}, }) - as.StubOne(true) //ask for adding a remote + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(true) //ask for adding a remote + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne("origin") //ask for remote name - as.StubOne(false) //ask to push to remote + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(false) //ask to push to remote }, httpStubs: func(reg *httpmock.Registry) { reg.Register( @@ -309,16 +332,22 @@ func Test_createRun(t *testing.T) { opts: &CreateOptions{Interactive: true}, tty: true, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne("Push an existing local repository to GitHub") + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(".") + //nolint:staticcheck // SA1019: as.Stub is deprecated: use StubPrompt as.Stub([]*prompt.QuestionStub{ {Name: "repoName", Value: "REPO"}, {Name: "repoDescription", Value: "my new repo"}, {Name: "repoVisibility", Value: "Private"}, }) - as.StubOne(true) //ask for adding a remote + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(true) //ask for adding a remote + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne("origin") //ask for remote name - as.StubOne(true) //ask to push to remote + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt + as.StubOne(true) //ask to push to remote }, httpStubs: func(reg *httpmock.Registry) { reg.Register( @@ -407,6 +436,7 @@ func Test_createRun(t *testing.T) { }, } for _, tt := range tests { + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber q, teardown := prompt.InitAskStubber() defer teardown() if tt.askStubs != nil { diff --git a/pkg/cmd/repo/delete/delete_test.go b/pkg/cmd/repo/delete/delete_test.go index da9c04223..3bd3e5d22 100644 --- a/pkg/cmd/repo/delete/delete_test.go +++ b/pkg/cmd/repo/delete/delete_test.go @@ -94,6 +94,7 @@ func Test_deleteRun(t *testing.T) { askStubs: func(q *prompt.AskStubber) { // TODO: survey stubber doesn't have WithValidator support // so this always passes regardless of prompt input + //nolint:staticcheck // SA1019: q.StubOne is deprecated: use StubPrompt q.StubOne("OWNER/REPO") }, httpStubs: func(reg *httpmock.Registry) { @@ -108,6 +109,7 @@ func Test_deleteRun(t *testing.T) { opts: &DeleteOptions{}, wantStdout: "✓ Deleted repository OWNER/REPO\n", askStubs: func(q *prompt.AskStubber) { + //nolint:staticcheck // SA1019: q.StubOne is deprecated: use StubPrompt q.StubOne("OWNER/REPO") }, httpStubs: func(reg *httpmock.Registry) { @@ -134,6 +136,7 @@ func Test_deleteRun(t *testing.T) { wantStdout: "✓ Deleted repository OWNER/REPO\n", tty: true, askStubs: func(q *prompt.AskStubber) { + //nolint:staticcheck // SA1019: q.StubOne is deprecated: use StubPrompt q.StubOne("OWNER/REPO") }, httpStubs: func(reg *httpmock.Registry) { @@ -147,6 +150,7 @@ func Test_deleteRun(t *testing.T) { }, } for _, tt := range tests { + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber q, teardown := prompt.InitAskStubber() defer teardown() if tt.askStubs != nil { diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 775152440..5fb458b93 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -236,6 +236,7 @@ func TestRepoFork(t *testing.T) { }, httpStubs: forkPost, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(false) }, wantErrOut: "✓ Created fork someone/REPO\n", @@ -254,6 +255,7 @@ func TestRepoFork(t *testing.T) { cs.Register(`git remote add -f origin https://github.com/someone/REPO.git`, 0, "") }, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(true) }, wantErrOut: "✓ Created fork someone/REPO\n✓ Added remote origin\n", @@ -442,6 +444,7 @@ func TestRepoFork(t *testing.T) { }, httpStubs: forkPost, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(false) }, wantErrOut: "✓ Created fork someone/REPO\n", @@ -455,6 +458,7 @@ func TestRepoFork(t *testing.T) { }, httpStubs: forkPost, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(true) }, execStubs: func(cs *run.CommandStubber) { @@ -475,6 +479,7 @@ func TestRepoFork(t *testing.T) { }, httpStubs: forkPost, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(true) }, execStubs: func(cs *run.CommandStubber) { @@ -570,6 +575,7 @@ func TestRepoFork(t *testing.T) { return tt.remotes, nil } + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, teardown := prompt.InitAskStubber() defer teardown() if tt.askStubs != nil { diff --git a/pkg/cmd/repo/rename/rename_test.go b/pkg/cmd/repo/rename/rename_test.go index 042579613..a886befaa 100644 --- a/pkg/cmd/repo/rename/rename_test.go +++ b/pkg/cmd/repo/rename/rename_test.go @@ -117,6 +117,7 @@ func TestRenameRun(t *testing.T) { name: "none argument", wantOut: "✓ Renamed repository OWNER/NEW_REPO\n✓ Updated the \"origin\" remote\n", askStubs: func(q *prompt.AskStubber) { + //nolint:staticcheck // SA1019: q.StubOne is deprecated: use StubPrompt q.StubOne("NEW_REPO") }, httpStubs: func(reg *httpmock.Registry) { @@ -136,6 +137,7 @@ func TestRenameRun(t *testing.T) { }, wantOut: "✓ Renamed repository OWNER/NEW_REPO\n", askStubs: func(q *prompt.AskStubber) { + //nolint:staticcheck // SA1019: q.StubOne is deprecated: use StubPrompt q.StubOne("NEW_REPO") }, httpStubs: func(reg *httpmock.Registry) { @@ -184,6 +186,7 @@ func TestRenameRun(t *testing.T) { }, wantOut: "✓ Renamed repository OWNER/NEW_REPO\n✓ Updated the \"origin\" remote\n", askStubs: func(q *prompt.AskStubber) { + //nolint:staticcheck // SA1019: q.StubOne is deprecated: use StubPrompt q.StubOne(true) }, httpStubs: func(reg *httpmock.Registry) { @@ -204,6 +207,7 @@ func TestRenameRun(t *testing.T) { DoConfirm: true, }, askStubs: func(q *prompt.AskStubber) { + //nolint:staticcheck // SA1019: q.StubOne is deprecated: use StubPrompt q.StubOne(false) }, wantOut: "", @@ -211,6 +215,7 @@ func TestRenameRun(t *testing.T) { } for _, tt := range testCases { + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber q, teardown := prompt.InitAskStubber() defer teardown() if tt.askStubs != nil { diff --git a/pkg/cmd/run/cancel/cancel_test.go b/pkg/cmd/run/cancel/cancel_test.go index 9304ee8fb..99a286137 100644 --- a/pkg/cmd/run/cancel/cancel_test.go +++ b/pkg/cmd/run/cancel/cancel_test.go @@ -174,6 +174,7 @@ func TestRunCancel(t *testing.T) { httpmock.StatusStringResponse(202, "{}")) }, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(0) }, wantOut: "✓ Request to cancel workflow submitted.\n", @@ -195,6 +196,7 @@ func TestRunCancel(t *testing.T) { return ghrepo.FromFullName("OWNER/REPO") } + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, teardown := prompt.InitAskStubber() defer teardown() if tt.askStubs != nil { diff --git a/pkg/cmd/run/rerun/rerun_test.go b/pkg/cmd/run/rerun/rerun_test.go index 11c5ff983..11362ce4b 100644 --- a/pkg/cmd/run/rerun/rerun_test.go +++ b/pkg/cmd/run/rerun/rerun_test.go @@ -137,6 +137,7 @@ func TestRerun(t *testing.T) { httpmock.StringResponse("{}")) }, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(2) }, wantOut: "✓ Requested rerun of run 1234\n", @@ -193,6 +194,7 @@ func TestRerun(t *testing.T) { return ghrepo.FromFullName("OWNER/REPO") } + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, teardown := prompt.InitAskStubber() defer teardown() if tt.askStubs != nil { diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index 7b61d3b2f..2a0540985 100644 --- a/pkg/cmd/run/view/view_test.go +++ b/pkg/cmd/run/view/view_test.go @@ -375,6 +375,7 @@ func TestViewRun(t *testing.T) { httpmock.JSONResponse([]shared.Annotation{})) }, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(2) }, opts: &ViewOptions{ @@ -411,7 +412,9 @@ func TestViewRun(t *testing.T) { httpmock.FileResponse("./fixtures/run_log.zip")) }, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(2) + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(1) }, wantOut: coolJobRunLogOutput, @@ -464,7 +467,9 @@ func TestViewRun(t *testing.T) { httpmock.FileResponse("./fixtures/run_log.zip")) }, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(2) + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(0) }, wantOut: expectedRunLogOutput, @@ -523,7 +528,9 @@ func TestViewRun(t *testing.T) { httpmock.FileResponse("./fixtures/run_log.zip")) }, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(4) + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(2) }, wantOut: quuxTheBarfLogOutput, @@ -576,7 +583,9 @@ func TestViewRun(t *testing.T) { httpmock.FileResponse("./fixtures/run_log.zip")) }, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(4) + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(0) }, wantOut: quuxTheBarfLogOutput, @@ -700,7 +709,9 @@ func TestViewRun(t *testing.T) { httpmock.JSONResponse(shared.FailedJobAnnotations)) }, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(2) + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(0) }, wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\nJOBS\n✓ cool job in 4m34s (ID 10)\nX sad job in 4m34s (ID 20)\n ✓ barf the quux\n X quux the barf\n\nANNOTATIONS\nX the job is sad\nsad job: blaze.py#420\n\n\nFor more information about a job, try: gh run view --job=\nView this run on GitHub: https://github.com/runs/3\n", @@ -733,7 +744,9 @@ func TestViewRun(t *testing.T) { httpmock.JSONResponse([]shared.Annotation{})) }, askStubs: func(as *prompt.AskStubber) { + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(2) + //nolint:staticcheck // SA1019: as.StubOne is deprecated: use StubPrompt as.StubOne(1) }, wantOut: "\n✓ trunk successful · 3\nTriggered via push about 59 minutes ago\n\n✓ cool job in 4m34s (ID 10)\n ✓ fob the barz\n ✓ barz the fob\n\nTo see the full job log, try: gh run view --log --job=10\nView this run on GitHub: https://github.com/runs/3\n", @@ -830,6 +843,7 @@ func TestViewRun(t *testing.T) { return ghrepo.FromFullName("OWNER/REPO") } + //nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber as, teardown := prompt.InitAskStubber() defer teardown() if tt.askStubs != nil { From 9f46def1a89f48713bf5b3c2f58f37cf63c34a3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 14 Jan 2022 19:54:09 +0100 Subject: [PATCH 18/19] Add `nolint-insert` script to auto-comment lint violations Step 1: mark a function as deprecated Step 2: run `script/nolint-insert` Step 3: all callers of that function now have a `//nolint` directive --- script/nolint-insert | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100755 script/nolint-insert diff --git a/script/nolint-insert b/script/nolint-insert new file mode 100755 index 000000000..b9e3f9298 --- /dev/null +++ b/script/nolint-insert @@ -0,0 +1,24 @@ +#!/bin/bash +# Usage: script/nolint-insert +# script/nolint-insert 'nolint:staticcheck // ' +set -e + +insert-line() { + local n=$'\n' + sed -i.bak "${2}i\\${n}${3}${n}" "$1" + rm "$1.bak" +} + +reverse() { + awk '{a[i++]=$0} END {for (j=i-1; j>=0;) print a[j--] }' +} + +comment="${1}" + +golangci-lint run --out-format json | jq -r '.Issues[] | [.Pos.Filename, .Pos.Line, .FromLinter, .Text] | @tsv' | reverse | while IFS=$'\t' read -r filename line linter text; do + directive="nolint:${linter} // $text" + [ -z "$comment" ] || directive="$comment" + insert-line "$filename" "$line" "//${directive}" +done + +go fmt ./... From 583af3e54c506fa1c0e6bc673c2c62a7e6303660 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 14 Jan 2022 23:18:07 +0100 Subject: [PATCH 19/19] Allow `gh auth git-credential` to authenticate GitHub Gist requests (#3064) * Allow `gh auth git-credential` to authenticate GitHub Gist requests When there are stored credentials for `example.com`, allow using them to authenticate requests to `gist.example.com` as well. * Fix writing out of credential config * remove unneccessary function * actually delete Co-authored-by: nate smith --- internal/ghinstance/host.go | 16 +++++-- pkg/cmd/auth/gitcredential/helper.go | 10 +++- pkg/cmd/auth/gitcredential/helper_test.go | 26 ++++++++++ pkg/cmd/auth/shared/git_credential.go | 56 +++++++++++++++------- pkg/cmd/auth/shared/git_credential_test.go | 49 ++++++++++++++++++- 5 files changed, 134 insertions(+), 23 deletions(-) diff --git a/internal/ghinstance/host.go b/internal/ghinstance/host.go index 146a94e77..b96852a4d 100644 --- a/internal/ghinstance/host.go +++ b/internal/ghinstance/host.go @@ -72,13 +72,23 @@ func RESTPrefix(hostname string) string { } func GistPrefix(hostname string) string { + prefix := "https://" + + if strings.EqualFold(hostname, localhost) { + prefix = "http://" + } + + return prefix + GistHost(hostname) +} + +func GistHost(hostname string) string { if IsEnterprise(hostname) { - return fmt.Sprintf("https://%s/gist/", hostname) + return fmt.Sprintf("%s/gist/", hostname) } if strings.EqualFold(hostname, localhost) { - return fmt.Sprintf("http://%s/gist/", hostname) + return fmt.Sprintf("%s/gist/", hostname) } - return fmt.Sprintf("https://gist.%s/", hostname) + return fmt.Sprintf("gist.%s/", hostname) } func HostPrefix(hostname string) string { diff --git a/pkg/cmd/auth/gitcredential/helper.go b/pkg/cmd/auth/gitcredential/helper.go index 8d1ab7ff3..117584b34 100644 --- a/pkg/cmd/auth/gitcredential/helper.go +++ b/pkg/cmd/auth/gitcredential/helper.go @@ -100,12 +100,18 @@ func helperRun(opts *CredentialOptions) error { return err } + lookupHost := wants["host"] var gotUser string - gotToken, source, _ := cfg.GetWithSource(wants["host"], "oauth_token") + gotToken, source, _ := cfg.GetWithSource(lookupHost, "oauth_token") + if gotToken == "" && strings.HasPrefix(lookupHost, "gist.") { + lookupHost = strings.TrimPrefix(lookupHost, "gist.") + gotToken, source, _ = cfg.GetWithSource(lookupHost, "oauth_token") + } + if strings.HasSuffix(source, "_TOKEN") { gotUser = tokenUser } else { - gotUser, _, _ = cfg.GetWithSource(wants["host"], "user") + gotUser, _, _ = cfg.GetWithSource(lookupHost, "user") } if gotUser == "" || gotToken == "" { diff --git a/pkg/cmd/auth/gitcredential/helper_test.go b/pkg/cmd/auth/gitcredential/helper_test.go index 7e30ec495..c06d6877f 100644 --- a/pkg/cmd/auth/gitcredential/helper_test.go +++ b/pkg/cmd/auth/gitcredential/helper_test.go @@ -74,6 +74,32 @@ func Test_helperRun(t *testing.T) { `), wantStderr: "", }, + { + name: "gist host", + opts: CredentialOptions{ + Operation: "get", + Config: func() (config, error) { + return tinyConfig{ + "_source": "/Users/monalisa/.config/gh/hosts.yml", + "github.com:user": "monalisa", + "github.com:oauth_token": "OTOKEN", + }, nil + }, + }, + input: heredoc.Doc(` + protocol=https + host=gist.github.com + username=monalisa + `), + wantErr: false, + wantStdout: heredoc.Doc(` + protocol=https + host=gist.github.com + username=monalisa + password=OTOKEN + `), + wantStderr: "", + }, { name: "url input", opts: CredentialOptions{ diff --git a/pkg/cmd/auth/shared/git_credential.go b/pkg/cmd/auth/shared/git_credential.go index 9e47bcc7e..fb8ba31c2 100644 --- a/pkg/cmd/auth/shared/git_credential.go +++ b/pkg/cmd/auth/shared/git_credential.go @@ -63,25 +63,46 @@ func (flow *GitCredentialFlow) Setup(hostname, username, authToken string) error func (flow *GitCredentialFlow) gitCredentialSetup(hostname, username, password string) error { if flow.helper == "" { - // first use a blank value to indicate to git we want to sever the chain of credential helpers - preConfigureCmd, err := git.GitCommand("config", "--global", "--replace-all", gitCredentialHelperKey(hostname), "") - if err != nil { - return err - } - if err = run.PrepareCmd(preConfigureCmd).Run(); err != nil { - return err + credHelperKeys := []string{ + gitCredentialHelperKey(hostname), } - // use GitHub CLI as a credential helper (for this host only) - configureCmd, err := git.GitCommand( - "config", "--global", "--add", - gitCredentialHelperKey(hostname), - fmt.Sprintf("!%s auth git-credential", shellQuote(flow.Executable)), - ) - if err != nil { - return err + gistHost := strings.TrimSuffix(ghinstance.GistHost(hostname), "/") + if strings.HasPrefix(gistHost, "gist.") { + credHelperKeys = append(credHelperKeys, gitCredentialHelperKey(gistHost)) } - return run.PrepareCmd(configureCmd).Run() + + var configErr error + + for _, credHelperKey := range credHelperKeys { + if configErr != nil { + break + } + // first use a blank value to indicate to git we want to sever the chain of credential helpers + preConfigureCmd, err := git.GitCommand("config", "--global", "--replace-all", credHelperKey, "") + if err != nil { + configErr = err + break + } + if err = run.PrepareCmd(preConfigureCmd).Run(); err != nil { + configErr = err + break + } + + // second configure the actual helper for this host + configureCmd, err := git.GitCommand( + "config", "--global", "--add", + credHelperKey, + fmt.Sprintf("!%s auth git-credential", shellQuote(flow.Executable)), + ) + if err != nil { + configErr = err + } else { + configErr = run.PrepareCmd(configureCmd).Run() + } + } + + return configErr } // clear previous cached credentials @@ -121,7 +142,8 @@ func (flow *GitCredentialFlow) gitCredentialSetup(hostname, username, password s } func gitCredentialHelperKey(hostname string) string { - return fmt.Sprintf("credential.%s.helper", strings.TrimSuffix(ghinstance.HostPrefix(hostname), "/")) + host := strings.TrimSuffix(ghinstance.HostPrefix(hostname), "/") + return fmt.Sprintf("credential.%s.helper", host) } func gitCredentialHelper(hostname string) (helper string, err error) { diff --git a/pkg/cmd/auth/shared/git_credential_test.go b/pkg/cmd/auth/shared/git_credential_test.go index dfdb72db9..fe674e1d7 100644 --- a/pkg/cmd/auth/shared/git_credential_test.go +++ b/pkg/cmd/auth/shared/git_credential_test.go @@ -22,7 +22,54 @@ func TestGitCredentialSetup_configureExisting(t *testing.T) { } } -func TestGitCredentialSetup_setOurs(t *testing.T) { +func TestGitCredentialsSetup_setOurs_GH(t *testing.T) { + cs, restoreRun := run.Stub() + defer restoreRun(t) + cs.Register(`git config --global --replace-all credential\.`, 0, "", func(args []string) { + if key := args[len(args)-2]; key != "credential.https://github.com.helper" { + t.Errorf("git config key was %q", key) + } + if val := args[len(args)-1]; val != "" { + t.Errorf("global credential helper configured to %q", val) + } + }) + cs.Register(`git config --global --add credential\.`, 0, "", func(args []string) { + if key := args[len(args)-2]; key != "credential.https://github.com.helper" { + t.Errorf("git config key was %q", key) + } + if val := args[len(args)-1]; val != "!/path/to/gh auth git-credential" { + t.Errorf("global credential helper configured to %q", val) + } + }) + cs.Register(`git config --global --replace-all credential\.`, 0, "", func(args []string) { + if key := args[len(args)-2]; key != "credential.https://gist.github.com.helper" { + t.Errorf("git config key was %q", key) + } + if val := args[len(args)-1]; val != "" { + t.Errorf("global credential helper configured to %q", val) + } + }) + cs.Register(`git config --global --add credential\.`, 0, "", func(args []string) { + if key := args[len(args)-2]; key != "credential.https://gist.github.com.helper" { + t.Errorf("git config key was %q", key) + } + if val := args[len(args)-1]; val != "!/path/to/gh auth git-credential" { + t.Errorf("global credential helper configured to %q", val) + } + }) + + f := GitCredentialFlow{ + Executable: "/path/to/gh", + helper: "", + } + + if err := f.gitCredentialSetup("github.com", "monalisa", "PASSWD"); err != nil { + t.Errorf("GitCredentialSetup() error = %v", err) + } + +} + +func TestGitCredentialSetup_setOurs_nonGH(t *testing.T) { cs, restoreRun := run.Stub() defer restoreRun(t) cs.Register(`git config --global --replace-all credential\.`, 0, "", func(args []string) {