successful logout with a msg when cfg files are absent

coder · AbhineetJain · May 24, 2022 · May 23, 2022 · May 23, 2022 · May 23, 2022
commit 33adc09e6abec3a9dc733be0db5ad0d567d5e5d8
diff --git a/cli/logout.go b/cli/logout.go
@@ -13,32 +13,41 @@ func logout() *cobra.Command {
 		Use:   "logout",
 		Short: "Remove the local authenticated session",
 		RunE: func(cmd *cobra.Command, args []string) error {
+			var isLoggedOut bool
+
 			config := createConfig(cmd)
 
 			err := config.URL().Delete()
 			if err != nil {
-				// If the URL configuration file is absent, the user is logged out
-				if os.IsNotExist(err) {
-					return xerrors.New(notLoggedInMessage)
+				// Only throw error if the URL configuration file is present,
+				// otherwise the user is already logged out, and we proceed
+				if !os.IsNotExist(err) {
+					return xerrors.Errorf("remove URL file: %w", err)
 				}
-				return xerrors.Errorf("remove URL file: %w", err)
+				isLoggedOut = true
 			}
 
 			err = config.Session().Delete()
 			if err != nil {
-				// If the session configuration file is absent, the user is logged out
-				if os.IsNotExist(err) {
-					return xerrors.New(notLoggedInMessage)
+				// Only throw error if the session configuration file is present,
+				// otherwise the user is already logged out, and we proceed
+				if !os.IsNotExist(err) {
+					return xerrors.Errorf("remove session file: %w", err)
 				}
-				return xerrors.Errorf("remove session file: %w", err)
+				isLoggedOut = true
 			}
 
 			err = config.Organization().Delete()
-			// If the organization configuration file is absent, we should still log out
+			// If the organization configuration file is absent, we still proceed
 			if err != nil && !os.IsNotExist(err) {
 				return xerrors.Errorf("remove organization file: %w", err)
 			}
 
+			// If the user was already logged out, we show them a message
+			if isLoggedOut {
+				_, _ = fmt.Fprintf(cmd.OutOrStdout(), notLoggedInMessage+"\n")
+			}
+
 			_, _ = fmt.Fprintf(cmd.OutOrStdout(), caret+"Successfully logged out.\n")
 			return nil
 		},

diff --git a/cli/logout_test.go b/cli/logout_test.go
@@ -17,53 +17,99 @@ func TestLogout(t *testing.T) {
 	t.Run("Logout", func(t *testing.T) {
 		t.Parallel()
 
-		config := login(t)
+		pty := ptytest.New(t)
+		config := login(t, pty)
 
 		// ensure session files exist
 		assert.FileExists(t, string(config.URL()))
 		assert.FileExists(t, string(config.Session()))
 
+		logoutChan := make(chan struct{})
 		logout, _ := clitest.New(t, "logout", "--global-config", string(config))
-		err := logout.Execute()
-		assert.NoError(t, err)
-		assert.NoFileExists(t, string(config.URL()))
-		assert.NoFileExists(t, string(config.Session()))
+		logout.SetIn(pty.Input())
+		logout.SetOut(pty.Output())
+
+		go func() {
+			defer close(logoutChan)
+			err := logout.Execute()
+			assert.NoError(t, err)
+			assert.NoFileExists(t, string(config.URL()))
+			assert.NoFileExists(t, string(config.Session()))
+		}()
+
+		pty.ExpectMatch("Successfully logged out")
+		<-logoutChan
 	})
 	t.Run("NoURLFile", func(t *testing.T) {
 		t.Parallel()
 
-		logout, _ := clitest.New(t, "logout")
+		pty := ptytest.New(t)
+		config := login(t, pty)
+
+		// ensure session files exist
+		assert.FileExists(t, string(config.URL()))
+		assert.FileExists(t, string(config.Session()))
+
+		os.RemoveAll(string(config.URL()))
 
-		err := logout.Execute()
-		assert.EqualError(t, err, "You are not logged in. Try logging in using 'coder login <url>'.")
+		logoutChan := make(chan struct{})
+		logout, _ := clitest.New(t, "logout", "--global-config", string(config))
+
+		logout.SetIn(pty.Input())
+		logout.SetOut(pty.Output())
+
+		go func() {
+			defer close(logoutChan)
+			err := logout.Execute()
+			assert.NoError(t, err)
+			assert.NoFileExists(t, string(config.URL()))
+			assert.NoFileExists(t, string(config.Session()))
+		}()
+
+		pty.ExpectMatch("You are not logged in. Try logging in using 'coder login <url>'.")
+		pty.ExpectMatch("Successfully logged out")
+		<-logoutChan
 	})
 	t.Run("NoSessionFile", func(t *testing.T) {
 		t.Parallel()
 
-		config := login(t)
+		pty := ptytest.New(t)
+		config := login(t, pty)
 
 		// ensure session files exist
 		assert.FileExists(t, string(config.URL()))
 		assert.FileExists(t, string(config.Session()))
 
 		os.RemoveAll(string(config.Session()))
 
+		logoutChan := make(chan struct{})
 		logout, _ := clitest.New(t, "logout", "--global-config", string(config))
 
-		err := logout.Execute()
-		assert.EqualError(t, err, "You are not logged in. Try logging in using 'coder login <url>'.")
+		logout.SetIn(pty.Input())
+		logout.SetOut(pty.Output())
+
+		go func() {
+			defer close(logoutChan)
+			err := logout.Execute()
+			assert.NoError(t, err)
+			assert.NoFileExists(t, string(config.URL()))
+			assert.NoFileExists(t, string(config.Session()))
+		}()
+
+		pty.ExpectMatch("You are not logged in. Try logging in using 'coder login <url>'.")
+		pty.ExpectMatch("Successfully logged out")
+		<-logoutChan
 	})
 }
 
-func login(t *testing.T) config.Root {
+func login(t *testing.T, pty *ptytest.PTY) config.Root {
 	t.Helper()
 
 	client := coderdtest.New(t, nil)
 	coderdtest.CreateFirstUser(t, client)
 
 	doneChan := make(chan struct{})
 	root, cfg := clitest.New(t, "login", "--force-tty", client.URL.String(), "--no-open")
-	pty := ptytest.New(t)
 	root.SetIn(pty.Input())
 	root.SetOut(pty.Output())
 	go func() {

diff --git a/cli/userlist_test.go b/cli/userlist_test.go
@@ -12,6 +12,7 @@ import (
 )
 
 func TestUserList(t *testing.T) {
+	t.Parallel()
 	t.Run("List", func(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)