From aaf4c4e4e3a3c91f5d7a5f87e197bf0de4b59476 Mon Sep 17 00:00:00 2001 From: Tyler McGoffin Date: Mon, 21 Oct 2024 09:22:27 -0700 Subject: [PATCH] Clean up auth-login-logout acceptance test with native functionality The previous commit introduced two new functions, setEnvVar and deleteEnvVar that are duplicative of functionality native to testscripts. This commit switches to the native functionality and removes the duplicative functions introduced in the previous commit. Additionally, it removes the `--token` flag that was added to `gh auth login` --- acceptance/acceptance_test.go | 24 ------------------- .../testdata/auth/auth-login-logout.txtar | 10 ++++---- pkg/cmd/auth/login/login.go | 15 ++++-------- 3 files changed, 11 insertions(+), 38 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 3c2b3358b..dd5b8b0d1 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -240,30 +240,6 @@ func sharedCmds(tsEnv testScriptEnv) map[string]func(ts *testscript.TestScript, ts.Check(os.WriteFile(src, data, mode)) }, - // perhaps these need to be extracted to different files, soon? - // I'm prompted to ask because "deleteEnvVar" could just be - // syntactic sugar for setEnvVar with an empty value. Can't do that - // with the current implementation - "deleteEnvVar": func(ts *testscript.TestScript, neg bool, args []string) { - if neg { - ts.Fatalf("unsupported: ! deleteEnvVar") - } - if len(args) != 1 { - ts.Fatalf("usage: deleteEnvVar name") - } - - ts.Setenv(args[0], "") - }, - "setEnvVar": func(ts *testscript.TestScript, neg bool, args []string) { - if neg { - ts.Fatalf("unsupported: ! setEnvVar") - } - if len(args) != 2 { - ts.Fatalf("usage: setEnvVar name value") - } - - ts.Setenv(args[0], args[1]) - }, "stdout2env": func(ts *testscript.TestScript, neg bool, args []string) { if neg { ts.Fatalf("unsupported: ! stdout2env") diff --git a/acceptance/testdata/auth/auth-login-logout.txtar b/acceptance/testdata/auth/auth-login-logout.txtar index 3e46d4a0f..3933ce1da 100644 --- a/acceptance/testdata/auth/auth-login-logout.txtar +++ b/acceptance/testdata/auth/auth-login-logout.txtar @@ -3,19 +3,21 @@ # overrides are happening # Copy $GH_TOKEN to a new env var -setEnvVar LOGIN_TOKEN $GH_TOKEN +env LOGIN_TOKEN=$GH_TOKEN exec echo $LOGIN_TOKEN stdout $GH_TOKEN # Remove GH_TOKEN env var so we don't fall back to it -deleteEnvVar GH_TOKEN +env GH_TOKEN='' # Ensure the token was deleted exec echo $GH_TOKEN ! stdout '.' -# Login to the host -exec gh auth login --hostname=$GH_HOST --with-token --token=$LOGIN_TOKEN --insecure-storage +# Login to the host by feeding the token to stdin +exec echo $LOGIN_TOKEN +stdin stdout +exec gh auth login --hostname=$GH_HOST --with-token --insecure-storage # Check that we are logged in exec gh auth status --hostname $GH_HOST diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index eb0b2e1c4..653c9e6c0 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -104,18 +104,14 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm } if tokenStdin { - if opts.Token == "" { - defer opts.IO.In.Close() - token, err := io.ReadAll(opts.IO.In) - if err != nil { - return fmt.Errorf("failed to read token from standard input: %w", err) - } - opts.Token = strings.TrimSpace(string(token)) + defer opts.IO.In.Close() + token, err := io.ReadAll(opts.IO.In) + if err != nil { + return fmt.Errorf("failed to read token from standard input: %w", err) } + opts.Token = strings.TrimSpace(string(token)) } - // I think this is dead code because the token will always be set if we make it past - // the conditional above (lines 106-115) if opts.IO.CanPrompt() && opts.Token == "" { opts.Interactive = true } @@ -143,7 +139,6 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm cmd.Flags().StringSliceVarP(&opts.Scopes, "scopes", "s", nil, "Additional authentication scopes to request") cmd.Flags().BoolVar(&tokenStdin, "with-token", false, "Read token from standard input") cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open a browser to authenticate") - cmd.Flags().StringVarP(&opts.Token, "token", "t", "", "Authenticate using a personal access token") cmdutil.StringEnumFlag(cmd, &opts.GitProtocol, "git-protocol", "p", "", []string{"ssh", "https"}, "The protocol to use for git operations on this host") // secure storage became the default on 2023/4/04; this flag is left as a no-op for backwards compatibility