From 3f7f847c220a36b26e71ed73320e2ce521cf8ff8 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Tue, 24 Mar 2026 14:30:11 -0700 Subject: [PATCH 1/3] refactor: remove listFunc stub from cmd/auth/list Remove the package-level `listFunc` variable that was used to stub `auth.List` in tests. The command now calls `auth.List` directly, and tests mock at the dependency layer (`Auth().Auths()`) instead, exercising the full command code path. --- cmd/auth/auth_test.go | 25 +-------------- cmd/auth/list.go | 8 ++--- cmd/auth/list_test.go | 72 ++++++++++++++++++++++++++----------------- 3 files changed, 47 insertions(+), 58 deletions(-) diff --git a/cmd/auth/auth_test.go b/cmd/auth/auth_test.go index 55569b66..b42a0993 100644 --- a/cmd/auth/auth_test.go +++ b/cmd/auth/auth_test.go @@ -15,29 +15,16 @@ package auth import ( - "context" "testing" "github.com/slackapi/slack-cli/internal/hooks" "github.com/slackapi/slack-cli/internal/shared" - "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/test/testutil" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" ) -type listMockObject struct { - mock.Mock -} - -func (m *listMockObject) MockListFunction(ctx context.Context, clients *shared.ClientFactory) ([]types.SlackAuth, error) { - args := m.Called() - return args.Get(0).([]types.SlackAuth), args.Error(1) -} - func TestAuthCommand(t *testing.T) { - // Create mocks ctx := slackcontext.MockContext(t.Context()) clientsMock := shared.NewClientsMock() clientsMock.AddDefaultMocks() @@ -49,16 +36,6 @@ func TestAuthCommand(t *testing.T) { cmd := NewCommand(clients) testutil.MockCmdIO(clients.IO, cmd) - mock := new(listMockObject) - listFunc = mock.MockListFunction - mock.On("MockListFunction").Return([]types.SlackAuth{}, nil) - - // Execute test err := cmd.ExecuteContext(ctx) - if err != nil { - assert.Fail(t, "cmd.Execute had unexpected error") - } - - // Check result - mock.AssertCalled(t, "MockListFunction") + assert.NoError(t, err) } diff --git a/cmd/auth/list.go b/cmd/auth/list.go index 17f3e0b6..cce149a5 100644 --- a/cmd/auth/list.go +++ b/cmd/auth/list.go @@ -18,7 +18,7 @@ import ( "fmt" "github.com/slackapi/slack-cli/internal/iostreams" - "github.com/slackapi/slack-cli/internal/pkg/auth" + authpkg "github.com/slackapi/slack-cli/internal/pkg/auth" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slacktrace" @@ -28,10 +28,6 @@ import ( "golang.org/x/text/language" ) -// Create handle to the function for testing -// TODO - Stopgap until we learn the correct way to structure our code for testing. -var listFunc = auth.List - // NewListCommand creates the Cobra command for listing authorized accounts func NewListCommand(clients *shared.ClientFactory) *cobra.Command { return &cobra.Command{ @@ -51,7 +47,7 @@ func NewListCommand(clients *shared.ClientFactory) *cobra.Command { // runListCommand will execute the list command func runListCommand(cmd *cobra.Command, clients *shared.ClientFactory) error { ctx := cmd.Context() - userAuthList, err := listFunc(ctx, clients) + userAuthList, err := authpkg.List(ctx, clients) if err != nil { return err } diff --git a/cmd/auth/list_test.go b/cmd/auth/list_test.go index 35581a21..fb73be91 100644 --- a/cmd/auth/list_test.go +++ b/cmd/auth/list_test.go @@ -15,8 +15,8 @@ package auth import ( - "context" "testing" + "time" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" @@ -26,37 +26,53 @@ import ( "github.com/stretchr/testify/mock" ) -// Setup a mock for the List package -type ListPkgMock struct { - mock.Mock -} - -func (m *ListPkgMock) List(ctx context.Context, clients *shared.ClientFactory) ([]types.SlackAuth, error) { - m.Called() - return []types.SlackAuth{}, nil -} - func TestListCommand(t *testing.T) { - // Create mocks - ctx := slackcontext.MockContext(t.Context()) - clientsMock := shared.NewClientsMock() - clientsMock.AddDefaultMocks() + tests := map[string]struct { + auths []types.SlackAuth + expected []string + }{ + "no authorized accounts": { + auths: []types.SlackAuth{}, + expected: []string{ + "You are not logged in to any Slack accounts", + "login", + }, + }, + "a single authorized account": { + auths: []types.SlackAuth{ + { + TeamDomain: "test-workspace", + TeamID: "T12345", + UserID: "U67890", + LastUpdated: time.Date(2025, 1, 15, 10, 30, 0, 0, time.UTC), + }, + }, + expected: []string{ + "test-workspace", + "T12345", + "U67890", + }, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + clientsMock := shared.NewClientsMock() + clientsMock.Auth.On("Auths", mock.Anything).Return(tc.auths, nil) + clientsMock.AddDefaultMocks() - // Create clients that is mocked for testing - clients := shared.NewClientFactory(clientsMock.MockClientFactory()) + clients := shared.NewClientFactory(clientsMock.MockClientFactory()) - // Create the command - cmd := NewListCommand(clients) - testutil.MockCmdIO(clients.IO, cmd) + cmd := NewListCommand(clients) + testutil.MockCmdIO(clients.IO, cmd) - listPkgMock := new(ListPkgMock) - listFunc = listPkgMock.List + err := cmd.ExecuteContext(ctx) + assert.NoError(t, err) - listPkgMock.On("List").Return([]types.SlackAuth{}, nil) - err := cmd.ExecuteContext(ctx) - if err != nil { - assert.Fail(t, "cmd.Execute had unexpected error") + output := clientsMock.GetCombinedOutput() + for _, expected := range tc.expected { + assert.Contains(t, output, expected) + } + }) } - - listPkgMock.AssertCalled(t, "List") } From a85074e20512f3e1e3d0c1cc2e4d01dc074fd1b6 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Tue, 24 Mar 2026 14:37:28 -0700 Subject: [PATCH 2/3] test: add multiple accounts test case for auth list --- cmd/auth/list_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/cmd/auth/list_test.go b/cmd/auth/list_test.go index fb73be91..87e4a2ff 100644 --- a/cmd/auth/list_test.go +++ b/cmd/auth/list_test.go @@ -53,6 +53,30 @@ func TestListCommand(t *testing.T) { "U67890", }, }, + "multiple authorized accounts": { + auths: []types.SlackAuth{ + { + TeamDomain: "alpha-workspace", + TeamID: "T11111", + UserID: "U11111", + LastUpdated: time.Date(2025, 1, 10, 8, 0, 0, 0, time.UTC), + }, + { + TeamDomain: "beta-workspace", + TeamID: "T22222", + UserID: "U22222", + LastUpdated: time.Date(2025, 2, 20, 16, 0, 0, 0, time.UTC), + }, + }, + expected: []string{ + "alpha-workspace", + "T11111", + "U11111", + "beta-workspace", + "T22222", + "U22222", + }, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { From 5d4a9028ecb2db69493d3353549cec6d769623f5 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Tue, 24 Mar 2026 14:44:14 -0700 Subject: [PATCH 3/3] test: add minimal output assertion to auth command test --- cmd/auth/auth_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/auth/auth_test.go b/cmd/auth/auth_test.go index b42a0993..0d392cc6 100644 --- a/cmd/auth/auth_test.go +++ b/cmd/auth/auth_test.go @@ -38,4 +38,6 @@ func TestAuthCommand(t *testing.T) { err := cmd.ExecuteContext(ctx) assert.NoError(t, err) + // Verify the auth command delegates to the list command; detailed list output testing is in TestListCommand + assert.Contains(t, clientsMock.GetCombinedOutput(), "logged in") }