Revert "test(auth): isolate OAuth coverage fixtures"

internal/auth/oauth_provider_coverage_test.go

@@ -18,8 +18,6 @@ import (
 	"sync/atomic"
 	"testing"
 	"time"
-
-	"github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/keychain"
 )
 
 type oauthLoginFixture struct {
@@ -35,79 +33,8 @@ type oauthLoginFixture struct {
 	exchangeError   atomic.Bool
 }
 
-type oauthLoginResult struct {
-	token *TokenData
-	err   error
-}
-
-type oauthHTTPResult struct {
-	status int
-	body   string
-	err    error
-}
-
-const oauthTestWaitTimeout = 5 * time.Second
-
-func isolateOAuthPersistence(t *testing.T) {
-	t.Helper()
-	t.Setenv(keychain.DisableKeychainEnv, "1")
-	cleanupKeychain(t)
-}
-
-func startOAuthLogin(t *testing.T, parent context.Context, f *oauthLoginFixture) <-chan oauthLoginResult {
-	t.Helper()
-	ctx, cancel := context.WithCancel(parent)
-	done := make(chan oauthLoginResult, 1)
-	finished := make(chan struct{})
-	go func() {
-		defer close(finished)
-		token, err := f.provider.Login(ctx, true)
-		done <- oauthLoginResult{token: token, err: err}
-	}()
-	t.Cleanup(func() {
-		cancel()
-		select {
-		case <-finished:
-		case <-time.After(oauthTestWaitTimeout):
-			t.Errorf("OAuth Login goroutine did not stop after cancellation")
-		}
-	})
-	return done
-}
-
-func awaitOAuthLogin(t *testing.T, done <-chan oauthLoginResult) oauthLoginResult {
-	t.Helper()
-	select {
-	case result := <-done:
-		return result
-	case <-time.After(oauthTestWaitTimeout):
-		t.Fatal("timed out waiting for OAuth Login")
-		return oauthLoginResult{}
-	}
-}
-
-func waitOAuthSignal(t *testing.T, signal <-chan struct{}, done <-chan oauthLoginResult, name string) {
-	t.Helper()
-	select {
-	case <-signal:
-	case result := <-done:
-		t.Fatalf("OAuth Login returned before %s: token=%#v err=%v", name, result.token, result.err)
-	case <-time.After(oauthTestWaitTimeout):
-		t.Fatalf("timed out waiting for OAuth %s", name)
-	}
-}
-
-func closeOAuthRelease(ch chan struct{}) {
-	select {
-	case <-ch:
-	default:
-		close(ch)
-	}
-}
-
 func newOAuthLoginFixture(t *testing.T, status func(int32) CLIAuthStatus) *oauthLoginFixture {
 	t.Helper()
-	isolateOAuthPersistence(t)
 	SetClientID("")
 	SetClientSecret("")
 	resetClientIDFromMCP()
@@ -164,10 +91,6 @@ func newOAuthLoginFixture(t *testing.T, status func(int32) CLIAuthStatus) *oauth
 		}
 	}))
 	t.Cleanup(f.server.Close)
-	t.Cleanup(func() {
-		closeOAuthRelease(f.exchangeRelease)
-		closeOAuthRelease(f.statusRelease)
-	})
 	f.configDir = setupMCPConfigDir(t, f.server.URL)
 
 	oldClient := oauthHTTPClient
@@ -195,25 +118,17 @@ func newOAuthLoginFixture(t *testing.T, status func(int32) CLIAuthStatus) *oauth
 
 func httpGetBody(t *testing.T, rawURL string) (int, string) {
 	t.Helper()
-	result := getHTTPBody(rawURL)
-	if result.err != nil {
-		t.Fatalf("GET %s: %v", rawURL, result.err)
-	}
-	return result.status, result.body
-}
-
-func getHTTPBody(rawURL string) oauthHTTPResult {
 	client := &http.Client{Timeout: 2 * time.Second}
 	resp, err := client.Get(rawURL)
 	if err != nil {
-		return oauthHTTPResult{err: err}
+		t.Fatalf("GET %s: %v", rawURL, err)
 	}
 	defer resp.Body.Close()
 	data, err := io.ReadAll(resp.Body)
 	if err != nil {
-		return oauthHTTPResult{err: err}
+		t.Fatal(err)
 	}
-	return oauthHTTPResult{status: resp.StatusCode, body: string(data)}
+	return resp.StatusCode, string(data)
 }
 
 func TestCrossPlatformCoverageOAuthLoginCallbackAndAPIs(t *testing.T) {
@@ -221,7 +136,17 @@ func TestCrossPlatformCoverageOAuthLoginCallbackAndAPIs(t *testing.T) {
 		return CLIAuthStatus{Success: true, Result: &CLIAuthResult{CLIAuthEnabled: true}}
 	})
 
-	loginDone := startOAuthLogin(t, context.Background(), f)
+	loginDone := make(chan struct {
+		token *TokenData
+		err   error
+	}, 1)
+	go func() {
+		token, err := f.provider.Login(context.Background(), true)
+		loginDone <- struct {
+			token *TokenData
+			err   error
+		}{token, err}
+	}()
 
 	for _, path := range []string{"/api/superAdmin", "/api/sendApply?adminStaffId=admin-1", "/api/cliAuthEnabled"} {
 		_, body := httpGetBody(t, f.callbackBase+path)
@@ -241,17 +166,18 @@ func TestCrossPlatformCoverageOAuthLoginCallbackAndAPIs(t *testing.T) {
 		t.Fatalf("success page = %q", body)
 	}
 
-	callbackDone := make(chan oauthHTTPResult, 1)
+	callbackDone := make(chan string, 1)
 	go func() {
-		callbackDone <- getHTTPBody(f.callbackBase + CallbackPath + "?code=good")
+		_, callbackBody := httpGetBody(t, f.callbackBase+CallbackPath+"?code=good")
+		callbackDone <- callbackBody
 	}()
-	waitOAuthSignal(t, f.exchangeEntered, loginDone, "token exchange")
+	<-f.exchangeEntered
 	_, body = httpGetBody(t, f.callbackBase+CallbackPath+"?authCode=good")
 	if !strings.Contains(body, "正在处理授权") {
 		t.Fatalf("concurrent callback = %q", body)
 	}
-	closeOAuthRelease(f.exchangeRelease)
-	waitOAuthSignal(t, f.statusEntered, loginDone, "CLI auth status check")
+	close(f.exchangeRelease)
+	<-f.statusEntered
 
 	_, body = httpGetBody(t, f.callbackBase+CallbackPath+"?code=good")
 	if !strings.Contains(body, "<html") {
@@ -278,16 +204,11 @@ func TestCrossPlatformCoverageOAuthLoginCallbackAndAPIs(t *testing.T) {
 		t.Fatalf("auth enabled API = %q", body)
 	}
 
-	closeOAuthRelease(f.statusRelease)
-	select {
-	case callback := <-callbackDone:
-		if callback.err != nil || !strings.Contains(callback.body, "<html") {
-			t.Fatalf("callback body = %q, %v", callback.body, callback.err)
-		}
-	case <-time.After(oauthTestWaitTimeout):
-		t.Fatal("timed out waiting for OAuth callback")
+	close(f.statusRelease)
+	if callbackBody := <-callbackDone; !strings.Contains(callbackBody, "<html") {
+		t.Fatalf("callback body = %q", callbackBody)
 	}
-	result := awaitOAuthLogin(t, loginDone)
+	result := <-loginDone
 	if result.err != nil || result.token == nil || result.token.AccessToken != "access" {
 		t.Fatalf("Login = %#v, %v", result.token, result.err)
 	}
@@ -307,20 +228,23 @@ func TestCrossPlatformCoverageOAuthLoginMissingCallbackCode(t *testing.T) {
 	f := newOAuthLoginFixture(t, func(int32) CLIAuthStatus {
 		return CLIAuthStatus{Success: true, Result: &CLIAuthResult{CLIAuthEnabled: true}}
 	})
-	closeOAuthRelease(f.exchangeRelease)
-	closeOAuthRelease(f.statusRelease)
-	done := startOAuthLogin(t, context.Background(), f)
+	close(f.exchangeRelease)
+	close(f.statusRelease)
+	done := make(chan error, 1)
+	go func() {
+		_, err := f.provider.Login(context.Background(), true)
+		done <- err
+	}()
 	status, body := httpGetBody(t, f.callbackBase+CallbackPath)
 	if status != http.StatusBadRequest || strings.TrimSpace(body) == "" {
 		t.Fatalf("missing callback = %d %q", status, body)
 	}
-	if result := awaitOAuthLogin(t, done); result.err == nil {
+	if err := <-done; err == nil {
 		t.Fatal("missing callback code did not fail login")
 	}
 }
 
 func TestCrossPlatformCoverageOAuthLoginEarlyAndListenerEdges(t *testing.T) {
-	isolateOAuthPersistence(t)
 	var buf bytes.Buffer
 	p := &OAuthProvider{configDir: t.TempDir(), Output: &buf}
 	if p.output() != &buf || (*OAuthProvider)(nil).output() != io.Discard {
@@ -387,7 +311,6 @@ func TestCrossPlatformCoverageOAuthLoginTimeoutAndServerError(t *testing.T) {
 }
 
 func TestCrossPlatformCoverageOAuthProviderOtherMethods(t *testing.T) {
-	isolateOAuthPersistence(t)
 	dir := t.TempDir()
 	_ = DeleteTokenDataKeychain()
 	t.Cleanup(func() { _ = DeleteTokenDataKeychain() })
@@ -440,7 +363,6 @@ func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) {
 }
 
 func TestCrossPlatformCoverageOAuthPersistConfigEdges(t *testing.T) {
-	isolateOAuthPersistence(t)
 	p := &OAuthProvider{configDir: t.TempDir(), logger: slog.Default()}
 	SetClientID("")
 	SetClientSecret("")
@@ -503,12 +425,16 @@ func TestCrossPlatformCoverageOAuthLoginDenialAndPolling(t *testing.T) {
 	for _, tt := range terminal {
 		t.Run(tt.name, func(t *testing.T) {
 			f := newOAuthLoginFixture(t, func(int32) CLIAuthStatus { return tt.status })
-			closeOAuthRelease(f.exchangeRelease)
-			closeOAuthRelease(f.statusRelease)
+			close(f.exchangeRelease)
+			close(f.statusRelease)
 			f.provider.NoBrowser = false
-			done := startOAuthLogin(t, context.Background(), f)
+			done := make(chan error, 1)
+			go func() {
+				_, err := f.provider.Login(context.Background(), true)
+				done <- err
+			}()
 			_, _ = httpGetBody(t, f.callbackBase+CallbackPath+"?code=denied")
-			if result := awaitOAuthLogin(t, done); result.err == nil {
+			if err := <-done; err == nil {
 				t.Fatal("denied login succeeded")
 			}
 		})
@@ -519,11 +445,15 @@ func TestCrossPlatformCoverageOAuthLoginDenialAndPolling(t *testing.T) {
 		f := newOAuthLoginFixture(t, func(int32) CLIAuthStatus {
 			return CLIAuthStatus{Success: true, Result: &CLIAuthResult{ChannelScope: "specified", AllowedChannels: []string{"allowed"}}}
 		})
-		closeOAuthRelease(f.exchangeRelease)
-		closeOAuthRelease(f.statusRelease)
-		done := startOAuthLogin(t, context.Background(), f)
+		close(f.exchangeRelease)
+		close(f.statusRelease)
+		done := make(chan error, 1)
+		go func() {
+			_, err := f.provider.Login(context.Background(), true)
+			done <- err
+		}()
 		_, _ = httpGetBody(t, f.callbackBase+CallbackPath+"?code=denied")
-		if result := awaitOAuthLogin(t, done); result.err == nil {
+		if err := <-done; err == nil {
 			t.Fatal("channel-denied login succeeded")
 		}
 	})
@@ -533,11 +463,15 @@ func TestCrossPlatformCoverageOAuthLoginDenialAndPolling(t *testing.T) {
 		f := newOAuthLoginFixture(t, func(int32) CLIAuthStatus {
 			return CLIAuthStatus{Success: true, Result: &CLIAuthResult{}}
 		})
-		closeOAuthRelease(f.exchangeRelease)
-		closeOAuthRelease(f.statusRelease)
-		done := startOAuthLogin(t, context.Background(), f)
+		close(f.exchangeRelease)
+		close(f.statusRelease)
+		done := make(chan error, 1)
+		go func() {
+			_, err := f.provider.Login(context.Background(), true)
+			done <- err
+		}()
 		_, _ = httpGetBody(t, f.callbackBase+CallbackPath+"?code=pending")
-		if result := awaitOAuthLogin(t, done); result.err == nil {
+		if err := <-done; err == nil {
 			t.Fatal("approval timeout login succeeded")
 		}
 		oauthApprovalTimeout = oldApproval
@@ -547,12 +481,16 @@ func TestCrossPlatformCoverageOAuthLoginDenialAndPolling(t *testing.T) {
 		f := newOAuthLoginFixture(t, func(call int32) CLIAuthStatus {
 			return CLIAuthStatus{Success: true, Result: &CLIAuthResult{CLIAuthEnabled: call > 1}}
 		})
-		closeOAuthRelease(f.exchangeRelease)
-		closeOAuthRelease(f.statusRelease)
-		done := startOAuthLogin(t, context.Background(), f)
+		close(f.exchangeRelease)
+		close(f.statusRelease)
+		done := make(chan error, 1)
+		go func() {
+			_, err := f.provider.Login(context.Background(), true)
+			done <- err
+		}()
 		_, _ = httpGetBody(t, f.callbackBase+CallbackPath+"?code=pending")
-		if result := awaitOAuthLogin(t, done); result.err != nil {
-			t.Fatalf("poll-enabled login failed: %v", result.err)
+		if err := <-done; err != nil {
+			t.Fatalf("poll-enabled login failed: %v", err)
 		}
 	})
 
@@ -560,13 +498,17 @@ func TestCrossPlatformCoverageOAuthLoginDenialAndPolling(t *testing.T) {
 		f := newOAuthLoginFixture(t, func(int32) CLIAuthStatus {
 			return CLIAuthStatus{Success: true, Result: &CLIAuthResult{}}
 		})
-		closeOAuthRelease(f.exchangeRelease)
-		closeOAuthRelease(f.statusRelease)
+		close(f.exchangeRelease)
+		close(f.statusRelease)
 		ctx, cancel := context.WithCancel(context.Background())
-		done := startOAuthLogin(t, ctx, f)
+		done := make(chan error, 1)
+		go func() {
+			_, err := f.provider.Login(ctx, true)
+			done <- err
+		}()
 		_, _ = httpGetBody(t, f.callbackBase+CallbackPath+"?code=pending")
 		cancel()
-		if result := awaitOAuthLogin(t, done); result.err == nil {
+		if err := <-done; err == nil {
 			t.Fatal("canceled pending login succeeded")
 		}
 	})
@@ -577,20 +519,23 @@ func TestCrossPlatformCoverageOAuthLoginExchangeFailure(t *testing.T) {
 		return CLIAuthStatus{Success: true, Result: &CLIAuthResult{CLIAuthEnabled: true}}
 	})
 	f.exchangeError.Store(true)
-	closeOAuthRelease(f.exchangeRelease)
-	closeOAuthRelease(f.statusRelease)
-	done := startOAuthLogin(t, context.Background(), f)
+	close(f.exchangeRelease)
+	close(f.statusRelease)
+	done := make(chan error, 1)
+	go func() {
+		_, err := f.provider.Login(context.Background(), true)
+		done <- err
+	}()
 	_, body := httpGetBody(t, f.callbackBase+CallbackPath+"?code=bad")
 	if !strings.Contains(body, "failed") {
 		t.Fatalf("exchange failure page = %q", body)
 	}
-	if result := awaitOAuthLogin(t, done); result.err == nil {
+	if err := <-done; err == nil {
 		t.Fatal("exchange failure login succeeded")
 	}
 }
 
 func TestCrossPlatformCoverageOAuthRefreshAndParsingEdges(t *testing.T) {
-	isolateOAuthPersistence(t)
 	t.Setenv("DWS_CLIENT_ID", "")
 	t.Setenv("DWS_CLIENT_SECRET", "")
 	dir := t.TempDir()
@@ -695,7 +640,6 @@ func TestCrossPlatformCoverageOAuthRefreshAndParsingEdges(t *testing.T) {
 }
 
 func TestCrossPlatformCoverageOAuthProviderHighLevelEdges(t *testing.T) {
-	isolateOAuthPersistence(t)
 	oldLoad := oauthLoadToken
 	oldLoadLocked := oauthLoadTokenLocked
 	oldAcquire := oauthAcquireLock
@@ -856,24 +800,29 @@ func TestCrossPlatformCoverageOAuthCallbackRemainingEdges(t *testing.T) {
 	oauthSuccessPause = 0
 	oauthSleep = func(time.Duration) {}
 
-	finishExchange := func(t *testing.T, f *oauthLoginFixture, done <-chan oauthLoginResult, code string) string {
+	type loginResult struct {
+		token *TokenData
+		err   error
+	}
+	startLogin := func(ctx context.Context, f *oauthLoginFixture) <-chan loginResult {
 		t.Helper()
-		bodyCh := make(chan oauthHTTPResult, 1)
+		done := make(chan loginResult, 1)
 		go func() {
-			bodyCh <- getHTTPBody(f.callbackBase + CallbackPath + "?code=" + url.QueryEscape(code))
+			token, err := f.provider.Login(ctx, true)
+			done <- loginResult{token: token, err: err}
 		}()
-		waitOAuthSignal(t, f.exchangeEntered, done, "token exchange")
-		closeOAuthRelease(f.exchangeRelease)
-		select {
-		case result := <-bodyCh:
-			if result.err != nil {
-				t.Fatalf("OAuth callback failed: %v", result.err)
-			}
-			return result.body
-		case <-time.After(oauthTestWaitTimeout):
-			t.Fatal("timed out waiting for OAuth callback")
-			return ""
-		}
+		return done
+	}
+	finishExchange := func(t *testing.T, f *oauthLoginFixture, code string) string {
+		t.Helper()
+		bodyCh := make(chan string, 1)
+		go func() {
+			_, body := httpGetBody(t, f.callbackBase+CallbackPath+"?code="+url.QueryEscape(code))
+			bodyCh <- body
+		}()
+		<-f.exchangeEntered
+		close(f.exchangeRelease)
+		return <-bodyCh
 	}
 
 	t.Run("switch organization and cached disabled pages", func(t *testing.T) {
@@ -886,8 +835,8 @@ func TestCrossPlatformCoverageOAuthCallbackRemainingEdges(t *testing.T) {
 			return &CLIAuthStatus{Success: true, Result: &CLIAuthResult{CLIAuthEnabled: true}}, nil
 		}
 		f := newOAuthLoginFixture(t, func(int32) CLIAuthStatus { return CLIAuthStatus{} })
-		done := startOAuthLogin(t, context.Background(), f)
-		if body := finishExchange(t, f, done, "first"); !strings.Contains(body, "<html") {
+		done := startLogin(context.Background(), f)
+		if body := finishExchange(t, f, "first"); !strings.Contains(body, "<html") {
 			t.Fatalf("disabled callback body = %q", body)
 		}
 		if _, body := httpGetBody(t, f.callbackBase+CallbackPath+"?code=first"); !strings.Contains(body, "<html") {
@@ -899,7 +848,7 @@ func TestCrossPlatformCoverageOAuthCallbackRemainingEdges(t *testing.T) {
 		if _, body := httpGetBody(t, f.callbackBase+CallbackPath+"?code=second"); !strings.Contains(body, "<html") {
 			t.Fatalf("switched callback = %q", body)
 		}
-		result := awaitOAuthLogin(t, done)
+		result := <-done
 		if result.err != nil || result.token == nil || result.token.AccessToken != "access" {
 			t.Fatalf("switched login = %#v %v", result.token, result.err)
 		}
@@ -913,15 +862,15 @@ func TestCrossPlatformCoverageOAuthCallbackRemainingEdges(t *testing.T) {
 		oauthSendApply = func(context.Context, string, string) (*SendApplyResponse, error) { return nil, fail }
 		ctx, cancel := context.WithCancel(context.Background())
 		f := newOAuthLoginFixture(t, func(int32) CLIAuthStatus { return CLIAuthStatus{} })
-		done := startOAuthLogin(t, ctx, f)
-		finishExchange(t, f, done, "errors")
+		done := startLogin(ctx, f)
+		finishExchange(t, f, "errors")
 		for _, path := range []string{"/api/superAdmin", "/api/sendApply?adminStaffId=admin", "/api/cliAuthEnabled"} {
 			if _, body := httpGetBody(t, f.callbackBase+path); !strings.Contains(body, "hook failure") {
 				t.Fatalf("API error %s = %q", path, body)
 			}
 		}
 		cancel()
-		if result := awaitOAuthLogin(t, done); !errors.Is(result.err, context.Canceled) {
+		if result := <-done; !errors.Is(result.err, context.Canceled) {
 			t.Fatalf("canceled error login = %v", result.err)
 		}
 	})
@@ -938,14 +887,14 @@ func TestCrossPlatformCoverageOAuthCallbackRemainingEdges(t *testing.T) {
 		f := newOAuthLoginFixture(t, func(int32) CLIAuthStatus { return CLIAuthStatus{} })
 		var output bytes.Buffer
 		f.provider.Output = &output
-		done := startOAuthLogin(t, ctx, f)
-		finishExchange(t, f, done, "apply")
+		done := startLogin(ctx, f)
+		finishExchange(t, f, "apply")
 		if _, body := httpGetBody(t, f.callbackBase+"/api/sendApply?adminStaffId=admin"); !strings.Contains(body, "true") {
 			t.Fatalf("apply response = %q", body)
 		}
 		time.Sleep(20 * time.Millisecond)
 		cancel()
-		if result := awaitOAuthLogin(t, done); !errors.Is(result.err, context.Canceled) {
+		if result := <-done; !errors.Is(result.err, context.Canceled) {
 			t.Fatalf("canceled apply login = %v", result.err)
 		}
 		if !strings.Contains(output.String(), "Waiting for admin approval") && !strings.Contains(output.String(), "等待管理员审批中") {
@@ -959,9 +908,9 @@ func TestCrossPlatformCoverageOAuthCallbackRemainingEdges(t *testing.T) {
 			return &CLIAuthStatus{Success: false, ErrorCode: "ENTERPRISE_NOT_AUTHORIZED"}, nil
 		}
 		f := newOAuthLoginFixture(t, func(int32) CLIAuthStatus { return CLIAuthStatus{} })
-		done := startOAuthLogin(t, context.Background(), f)
-		finishExchange(t, f, done, "enterprise")
-		if result := awaitOAuthLogin(t, done); result.err == nil || !strings.Contains(result.err.Error(), "企业安全认证") {
+		done := startLogin(context.Background(), f)
+		finishExchange(t, f, "enterprise")
+		if result := <-done; result.err == nil || !strings.Contains(result.err.Error(), "企业安全认证") {
 			t.Fatalf("enterprise denial = %v", result.err)
 		}
 	})
@@ -984,16 +933,15 @@ func TestCrossPlatformCoverageOAuthCallbackRemainingEdges(t *testing.T) {
 		fail := errors.New("save failure")
 		oauthSaveToken = func(string, *TokenData) error { return fail }
 		f := newOAuthLoginFixture(t, func(int32) CLIAuthStatus { return CLIAuthStatus{} })
-		done := startOAuthLogin(t, context.Background(), f)
-		finishExchange(t, f, done, "save")
-		if result := awaitOAuthLogin(t, done); !errors.Is(result.err, fail) {
+		done := startLogin(context.Background(), f)
+		finishExchange(t, f, "save")
+		if result := <-done; !errors.Is(result.err, fail) {
 			t.Fatalf("save failure login = %v", result.err)
 		}
 	})
 }
 
 func TestCrossPlatformCoverageOAuthHelperRemainingEdges(t *testing.T) {
-	isolateOAuthPersistence(t)
 	oldClient := oauthHTTPClient
 	oldRequest := oauthNewRequest
 	oldRetry := oauthRetryAfter