From a155ba5c4ab4b980129b0e6f4852f71468decee7 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 11 Mar 2026 20:56:19 -0700 Subject: [PATCH 1/5] test: increase test coverage in cmd/ and internal/api|config|deputil --- cmd/datastore/query_test.go | 52 +++++++++ internal/api/app_test.go | 48 +++++++++ internal/api/datastore_test.go | 189 +++++++++++++++++++++++++++++++++ internal/config/flags_test.go | 38 +++++++ internal/deputil/url_test.go | 24 +++++ 5 files changed, 351 insertions(+) diff --git a/cmd/datastore/query_test.go b/cmd/datastore/query_test.go index 0f704327..37b3d2b1 100644 --- a/cmd/datastore/query_test.go +++ b/cmd/datastore/query_test.go @@ -465,3 +465,55 @@ func prepareExportMockData(cm *shared.ClientsMock, numberOfItems int, maxItemsTo } return data, nil } + +func Test_getExpressionPatterns(t *testing.T) { + tests := map[string]struct { + expression string + wantAttrs int + wantVals int + }{ + "expression with attributes and values": { + expression: "#name = :name AND #status = :status", + wantAttrs: 2, + wantVals: 2, + }, + "empty expression": { + expression: "", + wantAttrs: 0, + wantVals: 0, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + attrs, vals := getExpressionPatterns(tc.expression) + assert.Len(t, attrs, tc.wantAttrs) + assert.Len(t, vals, tc.wantVals) + }) + } +} + +func Test_mapAttributeFlag(t *testing.T) { + tests := map[string]struct { + flag string + wantErr bool + }{ + "valid JSON": { + flag: `{"#name":"name"}`, + }, + "invalid JSON": { + flag: `not json`, + wantErr: true, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + result, err := mapAttributeFlag(tc.flag) + if tc.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.NotNil(t, result) + } + }) + } +} diff --git a/internal/api/app_test.go b/internal/api/app_test.go index f1e84015..7e5a752c 100644 --- a/internal/api/app_test.go +++ b/internal/api/app_test.go @@ -519,3 +519,51 @@ func TestClient_DeveloperAppInstall_RequestAppApproval(t *testing.T) { }) } } + +func TestClient_GetAppStatus_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appStatusMethod, + Response: `{"ok":true,"apps":[{"app_id":"A123","status":"installed"}]}`, + }) + defer teardown() + result, err := c.GetAppStatus(ctx, "token", []string{"A123"}, "T123") + require.NoError(t, err) + require.NotNil(t, result) +} + +func TestClient_GetAppStatus_Error(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appStatusMethod, + Response: `{"ok":false,"error":"invalid_app"}`, + }) + defer teardown() + _, err := c.GetAppStatus(ctx, "token", []string{"A123"}, "T123") + require.Error(t, err) + require.Contains(t, err.Error(), "invalid_app") +} + +func TestClient_ConnectionsOpen_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appConnectionsOpenMethod, + Response: `{"ok":true,"url":"wss://example.com/ws"}`, + }) + defer teardown() + result, err := c.ConnectionsOpen(ctx, "token") + require.NoError(t, err) + require.Equal(t, "wss://example.com/ws", result.URL) +} + +func TestClient_ConnectionsOpen_Error(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appConnectionsOpenMethod, + Response: `{"ok":false,"error":"token_revoked"}`, + }) + defer teardown() + _, err := c.ConnectionsOpen(ctx, "token") + require.Error(t, err) + require.Contains(t, err.Error(), "token_revoked") +} diff --git a/internal/api/datastore_test.go b/internal/api/datastore_test.go index c1126118..77f305bc 100644 --- a/internal/api/datastore_test.go +++ b/internal/api/datastore_test.go @@ -490,3 +490,192 @@ func TestClient_AppsDatastoreGet(t *testing.T) { }) } } + +func TestClient_AppsDatastoreBulkPut(t *testing.T) { + tests := map[string]struct { + request types.AppDatastoreBulkPut + httpResponseJSON string + statusCode int + wantErr bool + errMessage string + }{ + "success": { + request: types.AppDatastoreBulkPut{ + Datastore: "my_ds", + App: "A1", + Items: []map[string]interface{}{{"id": "1", "name": "test"}}, + }, + httpResponseJSON: `{"ok":true,"datastore":"my_ds"}`, + }, + "api_error": { + request: types.AppDatastoreBulkPut{ + Datastore: "my_ds", + App: "A1", + Items: []map[string]interface{}{{"id": "1"}}, + }, + httpResponseJSON: `{"ok":false,"error":"datastore_error"}`, + wantErr: true, + errMessage: "datastore_error", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appDatastoreBulkPutMethod, + Response: tc.httpResponseJSON, + StatusCode: tc.statusCode, + }) + defer teardown() + _, err := c.AppsDatastoreBulkPut(ctx, "token", tc.request) + if tc.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tc.errMessage) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestClient_AppsDatastoreCount(t *testing.T) { + tests := map[string]struct { + request types.AppDatastoreCount + httpResponseJSON string + statusCode int + wantCount int + wantErr bool + errMessage string + }{ + "success": { + request: types.AppDatastoreCount{ + Datastore: "my_ds", + App: "A1", + }, + httpResponseJSON: `{"ok":true,"datastore":"my_ds","count":42}`, + wantCount: 42, + }, + "api_error": { + request: types.AppDatastoreCount{ + Datastore: "my_ds", + App: "A1", + }, + httpResponseJSON: `{"ok":false,"error":"datastore_error"}`, + wantErr: true, + errMessage: "datastore_error", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appDatastoreCountMethod, + Response: tc.httpResponseJSON, + StatusCode: tc.statusCode, + }) + defer teardown() + got, err := c.AppsDatastoreCount(ctx, "token", tc.request) + if tc.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tc.errMessage) + } else { + require.NoError(t, err) + require.Equal(t, tc.wantCount, got.Count) + } + }) + } +} + +func TestClient_AppsDatastoreBulkDelete(t *testing.T) { + tests := map[string]struct { + request types.AppDatastoreBulkDelete + httpResponseJSON string + statusCode int + wantErr bool + errMessage string + }{ + "success": { + request: types.AppDatastoreBulkDelete{ + Datastore: "my_ds", + App: "A1", + IDs: []string{"id1", "id2"}, + }, + httpResponseJSON: `{"ok":true}`, + }, + "api_error": { + request: types.AppDatastoreBulkDelete{ + Datastore: "my_ds", + App: "A1", + IDs: []string{"id1"}, + }, + httpResponseJSON: `{"ok":false,"error":"not_found"}`, + wantErr: true, + errMessage: "not_found", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appDatastoreBulkDeleteMethod, + Response: tc.httpResponseJSON, + StatusCode: tc.statusCode, + }) + defer teardown() + _, err := c.AppsDatastoreBulkDelete(ctx, "token", tc.request) + if tc.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tc.errMessage) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestClient_AppsDatastoreBulkGet(t *testing.T) { + tests := map[string]struct { + request types.AppDatastoreBulkGet + httpResponseJSON string + statusCode int + wantErr bool + errMessage string + }{ + "success": { + request: types.AppDatastoreBulkGet{ + Datastore: "my_ds", + App: "A1", + IDs: []string{"id1", "id2"}, + }, + httpResponseJSON: `{"ok":true,"datastore":"my_ds","items":[{"id":"id1","name":"test"}]}`, + }, + "api_error": { + request: types.AppDatastoreBulkGet{ + Datastore: "my_ds", + App: "A1", + IDs: []string{"id1"}, + }, + httpResponseJSON: `{"ok":false,"error":"not_found"}`, + wantErr: true, + errMessage: "not_found", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appDatastoreBulkGetMethod, + Response: tc.httpResponseJSON, + StatusCode: tc.statusCode, + }) + defer teardown() + _, err := c.AppsDatastoreBulkGet(ctx, "token", tc.request) + if tc.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tc.errMessage) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/internal/config/flags_test.go b/internal/config/flags_test.go index 3db25406..6c5d3565 100644 --- a/internal/config/flags_test.go +++ b/internal/config/flags_test.go @@ -24,6 +24,44 @@ import ( "github.com/stretchr/testify/assert" ) +func TestSetFlags(t *testing.T) { + fs := slackdeps.NewFsMock() + os := slackdeps.NewOsMock() + config := NewConfig(fs, os) + cmd := &cobra.Command{} + cmd.Flags().String("test-flag", "default", "a test flag") + + config.SetFlags(cmd) + assert.NotNil(t, config.Flags) + f := config.Flags.Lookup("test-flag") + assert.NotNil(t, f) + assert.Equal(t, "default", f.DefValue) +} + +func TestInitializeGlobalFlags(t *testing.T) { + fs := slackdeps.NewFsMock() + os := slackdeps.NewOsMock() + config := NewConfig(fs, os) + cmd := &cobra.Command{} + + config.InitializeGlobalFlags(cmd) + + // Verify that key persistent flags were registered + flagNames := []string{ + "apihost", "app", "config-dir", "experiment", + "force", "no-color", "skip-update", "slackdev", + "runtime", "team", "token", "verbose", + } + for _, name := range flagNames { + f := cmd.PersistentFlags().Lookup(name) + assert.NotNil(t, f, "flag %s should be registered", name) + } + + // Verify hidden flags + assert.True(t, cmd.PersistentFlags().Lookup("apihost").Hidden) + assert.True(t, cmd.PersistentFlags().Lookup("slackdev").Hidden) +} + func TestDeprecatedFlagSubstitutions(t *testing.T) { tests := map[string]struct { expectedWarnings []string diff --git a/internal/deputil/url_test.go b/internal/deputil/url_test.go index e26fee9f..f69b55e4 100644 --- a/internal/deputil/url_test.go +++ b/internal/deputil/url_test.go @@ -53,6 +53,30 @@ func Test_URLChecker(t *testing.T) { httpClientMock.On("Head", mock.Anything).Return(nil, fmt.Errorf("HTTPClient error")) }, }, + "Returns an empty string for HTTP 500 Internal Server Error": { + url: "https://example.com/server-error", + expectedURL: "", + setupHTTPClientMock: func(httpClientMock *slackhttp.HTTPClientMock) { + res := slackhttp.MockHTTPResponse(http.StatusInternalServerError, "Internal Server Error") + httpClientMock.On("Head", mock.Anything).Return(res, nil) + }, + }, + "Returns an empty string for HTTP 301 redirect": { + url: "https://example.com/redirect", + expectedURL: "", + setupHTTPClientMock: func(httpClientMock *slackhttp.HTTPClientMock) { + res := slackhttp.MockHTTPResponse(http.StatusMovedPermanently, "Moved") + httpClientMock.On("Head", mock.Anything).Return(res, nil) + }, + }, + "Returns an empty string for HTTP 403 Forbidden": { + url: "https://example.com/forbidden", + expectedURL: "", + setupHTTPClientMock: func(httpClientMock *slackhttp.HTTPClientMock) { + res := slackhttp.MockHTTPResponse(http.StatusForbidden, "Forbidden") + httpClientMock.On("Head", mock.Anything).Return(res, nil) + }, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { From 28f1bf2f4ae680b5b25877bdccc5a4af429f0a2b Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 11 Mar 2026 21:21:07 -0700 Subject: [PATCH 2/5] test: rewrite api app tests as table-driven tests Consolidate GetAppStatus and ConnectionsOpen tests from separate ok/error functions into table-driven tests using the map pattern. --- internal/api/app_test.go | 110 ++++++++++++++++++++++++--------------- 1 file changed, 68 insertions(+), 42 deletions(-) diff --git a/internal/api/app_test.go b/internal/api/app_test.go index 7e5a752c..8dce7c51 100644 --- a/internal/api/app_test.go +++ b/internal/api/app_test.go @@ -520,50 +520,76 @@ func TestClient_DeveloperAppInstall_RequestAppApproval(t *testing.T) { } } -func TestClient_GetAppStatus_Ok(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: appStatusMethod, - Response: `{"ok":true,"apps":[{"app_id":"A123","status":"installed"}]}`, - }) - defer teardown() - result, err := c.GetAppStatus(ctx, "token", []string{"A123"}, "T123") - require.NoError(t, err) - require.NotNil(t, result) -} +func Test_Client_GetAppStatus(t *testing.T) { + tests := map[string]struct { + httpResponseJSON string + wantErr bool + errMessage string + }{ + "OK result": { + httpResponseJSON: `{"ok":true,"apps":[{"app_id":"A123","status":"installed"}]}`, + }, + "Error result": { + httpResponseJSON: `{"ok":false,"error":"invalid_app"}`, + wantErr: true, + errMessage: "invalid_app", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appStatusMethod, + Response: tc.httpResponseJSON, + }) + defer teardown() -func TestClient_GetAppStatus_Error(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: appStatusMethod, - Response: `{"ok":false,"error":"invalid_app"}`, - }) - defer teardown() - _, err := c.GetAppStatus(ctx, "token", []string{"A123"}, "T123") - require.Error(t, err) - require.Contains(t, err.Error(), "invalid_app") + result, err := c.GetAppStatus(ctx, "token", []string{"A123"}, "T123") + if tc.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tc.errMessage) + } else { + require.NoError(t, err) + require.NotNil(t, result) + } + }) + } } -func TestClient_ConnectionsOpen_Ok(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: appConnectionsOpenMethod, - Response: `{"ok":true,"url":"wss://example.com/ws"}`, - }) - defer teardown() - result, err := c.ConnectionsOpen(ctx, "token") - require.NoError(t, err) - require.Equal(t, "wss://example.com/ws", result.URL) -} +func Test_Client_ConnectionsOpen(t *testing.T) { + tests := map[string]struct { + httpResponseJSON string + wantErr bool + errMessage string + expectedURL string + }{ + "OK result": { + httpResponseJSON: `{"ok":true,"url":"wss://example.com/ws"}`, + expectedURL: "wss://example.com/ws", + }, + "Error result": { + httpResponseJSON: `{"ok":false,"error":"token_revoked"}`, + wantErr: true, + errMessage: "token_revoked", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appConnectionsOpenMethod, + Response: tc.httpResponseJSON, + }) + defer teardown() -func TestClient_ConnectionsOpen_Error(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: appConnectionsOpenMethod, - Response: `{"ok":false,"error":"token_revoked"}`, - }) - defer teardown() - _, err := c.ConnectionsOpen(ctx, "token") - require.Error(t, err) - require.Contains(t, err.Error(), "token_revoked") + result, err := c.ConnectionsOpen(ctx, "token") + if tc.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tc.errMessage) + } else { + require.NoError(t, err) + require.Equal(t, tc.expectedURL, result.URL) + } + }) + } } From 58c9c3fe7a0a70584a549de3a4e97a0b8446c476 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 11 Mar 2026 21:24:48 -0700 Subject: [PATCH 3/5] test: normalize test function names to use Test_ prefix convention --- internal/api/datastore_test.go | 8 ++++---- internal/config/flags_test.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/api/datastore_test.go b/internal/api/datastore_test.go index 77f305bc..c1e8c4cb 100644 --- a/internal/api/datastore_test.go +++ b/internal/api/datastore_test.go @@ -491,7 +491,7 @@ func TestClient_AppsDatastoreGet(t *testing.T) { } } -func TestClient_AppsDatastoreBulkPut(t *testing.T) { +func Test_Client_AppsDatastoreBulkPut(t *testing.T) { tests := map[string]struct { request types.AppDatastoreBulkPut httpResponseJSON string @@ -538,7 +538,7 @@ func TestClient_AppsDatastoreBulkPut(t *testing.T) { } } -func TestClient_AppsDatastoreCount(t *testing.T) { +func Test_Client_AppsDatastoreCount(t *testing.T) { tests := map[string]struct { request types.AppDatastoreCount httpResponseJSON string @@ -586,7 +586,7 @@ func TestClient_AppsDatastoreCount(t *testing.T) { } } -func TestClient_AppsDatastoreBulkDelete(t *testing.T) { +func Test_Client_AppsDatastoreBulkDelete(t *testing.T) { tests := map[string]struct { request types.AppDatastoreBulkDelete httpResponseJSON string @@ -633,7 +633,7 @@ func TestClient_AppsDatastoreBulkDelete(t *testing.T) { } } -func TestClient_AppsDatastoreBulkGet(t *testing.T) { +func Test_Client_AppsDatastoreBulkGet(t *testing.T) { tests := map[string]struct { request types.AppDatastoreBulkGet httpResponseJSON string diff --git a/internal/config/flags_test.go b/internal/config/flags_test.go index 6c5d3565..8136b8ee 100644 --- a/internal/config/flags_test.go +++ b/internal/config/flags_test.go @@ -24,7 +24,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestSetFlags(t *testing.T) { +func Test_SetFlags(t *testing.T) { fs := slackdeps.NewFsMock() os := slackdeps.NewOsMock() config := NewConfig(fs, os) @@ -38,7 +38,7 @@ func TestSetFlags(t *testing.T) { assert.Equal(t, "default", f.DefValue) } -func TestInitializeGlobalFlags(t *testing.T) { +func Test_InitializeGlobalFlags(t *testing.T) { fs := slackdeps.NewFsMock() os := slackdeps.NewOsMock() config := NewConfig(fs, os) From e2e0ed50452feed54c2f984b8c7bc21dda103e4d Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Thu, 12 Mar 2026 10:25:56 -0700 Subject: [PATCH 4/5] test: try to alphabetized app_test.go --- internal/api/app_test.go | 148 +++++++++++++++++++-------------------- 1 file changed, 74 insertions(+), 74 deletions(-) diff --git a/internal/api/app_test.go b/internal/api/app_test.go index 8dce7c51..873f7b49 100644 --- a/internal/api/app_test.go +++ b/internal/api/app_test.go @@ -34,6 +34,44 @@ import ( "github.com/stretchr/testify/require" ) +func Test_Client_ConnectionsOpen(t *testing.T) { + tests := map[string]struct { + httpResponseJSON string + wantErr bool + errMessage string + expectedURL string + }{ + "OK result": { + httpResponseJSON: `{"ok":true,"url":"wss://example.com/ws"}`, + expectedURL: "wss://example.com/ws", + }, + "Error result": { + httpResponseJSON: `{"ok":false,"error":"token_revoked"}`, + wantErr: true, + errMessage: "token_revoked", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appConnectionsOpenMethod, + Response: tc.httpResponseJSON, + }) + defer teardown() + + result, err := c.ConnectionsOpen(ctx, "token") + if tc.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tc.errMessage) + } else { + require.NoError(t, err) + require.Equal(t, tc.expectedURL, result.URL) + } + }) + } +} + func TestClient_CreateApp_Ok(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ @@ -80,6 +118,42 @@ func TestClient_ExportAppManifest_CommonErrors(t *testing.T) { }) } +func Test_Client_GetAppStatus(t *testing.T) { + tests := map[string]struct { + httpResponseJSON string + wantErr bool + errMessage string + }{ + "OK result": { + httpResponseJSON: `{"ok":true,"apps":[{"app_id":"A123","status":"installed"}]}`, + }, + "Error result": { + httpResponseJSON: `{"ok":false,"error":"invalid_app"}`, + wantErr: true, + errMessage: "invalid_app", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appStatusMethod, + Response: tc.httpResponseJSON, + }) + defer teardown() + + result, err := c.GetAppStatus(ctx, "token", []string{"A123"}, "T123") + if tc.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tc.errMessage) + } else { + require.NoError(t, err) + require.NotNil(t, result) + } + }) + } +} + func TestClient_UpdateApp_OK(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ @@ -519,77 +593,3 @@ func TestClient_DeveloperAppInstall_RequestAppApproval(t *testing.T) { }) } } - -func Test_Client_GetAppStatus(t *testing.T) { - tests := map[string]struct { - httpResponseJSON string - wantErr bool - errMessage string - }{ - "OK result": { - httpResponseJSON: `{"ok":true,"apps":[{"app_id":"A123","status":"installed"}]}`, - }, - "Error result": { - httpResponseJSON: `{"ok":false,"error":"invalid_app"}`, - wantErr: true, - errMessage: "invalid_app", - }, - } - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: appStatusMethod, - Response: tc.httpResponseJSON, - }) - defer teardown() - - result, err := c.GetAppStatus(ctx, "token", []string{"A123"}, "T123") - if tc.wantErr { - require.Error(t, err) - require.Contains(t, err.Error(), tc.errMessage) - } else { - require.NoError(t, err) - require.NotNil(t, result) - } - }) - } -} - -func Test_Client_ConnectionsOpen(t *testing.T) { - tests := map[string]struct { - httpResponseJSON string - wantErr bool - errMessage string - expectedURL string - }{ - "OK result": { - httpResponseJSON: `{"ok":true,"url":"wss://example.com/ws"}`, - expectedURL: "wss://example.com/ws", - }, - "Error result": { - httpResponseJSON: `{"ok":false,"error":"token_revoked"}`, - wantErr: true, - errMessage: "token_revoked", - }, - } - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: appConnectionsOpenMethod, - Response: tc.httpResponseJSON, - }) - defer teardown() - - result, err := c.ConnectionsOpen(ctx, "token") - if tc.wantErr { - require.Error(t, err) - require.Contains(t, err.Error(), tc.errMessage) - } else { - require.NoError(t, err) - require.Equal(t, tc.expectedURL, result.URL) - } - }) - } -} From c4dbb36198510be2aecf6094bd00f19509984cfc Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Thu, 12 Mar 2026 12:20:58 -0700 Subject: [PATCH 5/5] test: refactor InitializeGlobalFlags to table-driven test with flag details --- internal/config/flags_test.go | 71 +++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 12 deletions(-) diff --git a/internal/config/flags_test.go b/internal/config/flags_test.go index 8136b8ee..9be5d929 100644 --- a/internal/config/flags_test.go +++ b/internal/config/flags_test.go @@ -46,20 +46,67 @@ func Test_InitializeGlobalFlags(t *testing.T) { config.InitializeGlobalFlags(cmd) - // Verify that key persistent flags were registered - flagNames := []string{ - "apihost", "app", "config-dir", "experiment", - "force", "no-color", "skip-update", "slackdev", - "runtime", "team", "token", "verbose", + tests := map[string]struct { + longform string + shorthand string + hidden bool + }{ + "apihost": { + longform: "apihost", + hidden: true, + }, + "app": { + longform: "app", + shorthand: "a", + }, + "config-dir": { + longform: "config-dir", + }, + "experiment": { + longform: "experiment", + }, + "force": { + longform: "force", + shorthand: "f", + }, + "no-color": { + longform: "no-color", + }, + "runtime": { + longform: "runtime", + shorthand: "r", + hidden: true, + }, + "skip-update": { + longform: "skip-update", + shorthand: "s", + }, + "slackdev": { + longform: "slackdev", + hidden: true, + }, + "team": { + longform: "team", + shorthand: "w", + }, + "token": { + longform: "token", + }, + "verbose": { + longform: "verbose", + shorthand: "v", + }, } - for _, name := range flagNames { - f := cmd.PersistentFlags().Lookup(name) - assert.NotNil(t, f, "flag %s should be registered", name) + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + f := cmd.PersistentFlags().Lookup(tc.longform) + assert.NotNil(t, f, "flag %s should be registered", tc.longform) + if tc.shorthand != "" { + assert.Equal(t, tc.shorthand, f.Shorthand, "flag %s shorthand mismatch", tc.longform) + } + assert.Equal(t, tc.hidden, f.Hidden, "flag %s hidden mismatch", tc.longform) + }) } - - // Verify hidden flags - assert.True(t, cmd.PersistentFlags().Lookup("apihost").Hidden) - assert.True(t, cmd.PersistentFlags().Lookup("slackdev").Hidden) } func TestDeprecatedFlagSubstitutions(t *testing.T) {