-
Notifications
You must be signed in to change notification settings - Fork 33
refactor: remove listFunc stub from cmd/auth/list #435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3f7f847
a85074e
5d4a902
5018288
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: This is the important line that removes the |
||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,77 @@ 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", | ||
| }, | ||
| }, | ||
| "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) { | ||
| ctx := slackcontext.MockContext(t.Context()) | ||
| clientsMock := shared.NewClientsMock() | ||
| clientsMock.Auth.On("Auths", mock.Anything).Return(tc.auths, nil) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🌟 praise: Love to see this! I understand this has significant increase to test coverage: |
||
| 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") | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🪓
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🪵 praise: These
TODOhave been unsolved for some time I recall! Thanks for finding good pattern.