Skip to content

refactor(tests): convert the higher risk slice-to-map table tests #324

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.

Sign up for GitHub

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

Draft
mwbrooks wants to merge 2 commits into
base: mwbrooks-table-test-consistent-p6-medium-high-risk
Choose a base branch
from
Draft

refactor(tests): convert the higher risk slice-to-map table tests #324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a13ed27
refactor(tests): convert the higher risk slice-to-map table tests
mwbrooks Jan 31, 2026
9a782c9
refactor(tests): fix test not wrapped in t.Run
mwbrooks Jan 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 9 additions & 13 deletions internal/api/app_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,42 +450,38 @@ func TestClient_DeleteApp(t *testing.T) {
}

func TestClient_DeveloperAppInstall_RequestAppApproval(t *testing.T) {
tests := []struct {
name string
tests := map[string]struct {
app types.App
orgGrantWorkspaceID string
teamID string
requestJSON string
wantErr bool
errMessage string
}{
{
name: `Standalone workspace, AAA is requested \
(workspace ID passed into apps.approvals.requests.create)`,
`Standalone workspace, AAA is requested \
(workspace ID passed into apps.approvals.requests.create)`: {
Comment on lines +461 to +462
Copy link
Member Author

@mwbrooks mwbrooks Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: This naming convention is a little strange, but I decided to keep it 1-to-1 with current implementation.

app: types.App{AppID: "A1234", TeamID: "T1234"},
orgGrantWorkspaceID: "",
teamID: "T1234",
requestJSON: `{"app":"A1234","reason":"This request has been automatically generated according to project environment settings.","team_id":"T1234"}`,
},
{
name: `User tried to install to a single workspace in an org, AAA is requested \
(workspace ID passed into apps.approvals.requests.create)`,
`User tried to install to a single workspace in an org, AAA is requested \
(workspace ID passed into apps.approvals.requests.create)`: {
app: types.App{AppID: "A1234", EnterpriseID: "E1234", TeamID: "E1234"},
orgGrantWorkspaceID: "T1234",
teamID: "T1234",
requestJSON: `{"app":"A1234","reason":"This request has been automatically generated according to project environment settings.","team_id":"T1234"}`,
},
{
name: `User tried to install to all workspaces in an org, AAA is requested \
(no team_id passed into apps.approvals.requests.create so it will default to creating a request for the auth team ie. the org)`,
`User tried to install to all workspaces in an org, AAA is requested \
(no team_id passed into apps.approvals.requests.create so it will default to creating a request for the auth team ie. the org)`: {
app: types.App{AppID: "A1234", EnterpriseID: "E1234", TeamID: "E1234"},
orgGrantWorkspaceID: "all",
teamID: "E1234",
requestJSON: `{"app":"A1234","reason":"This request has been automatically generated according to project environment settings."}`,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
ctx := slackcontext.MockContext(t.Context())

// prepare
Expand Down
52 changes: 18 additions & 34 deletions internal/api/debug_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,89 +24,73 @@ import (

func Test_RedactPII(t *testing.T) {
home, _ := os.UserHomeDir()
tests := []struct {
name string
tests := map[string]struct {
text string
expected string
}{
{
name: "Simple case",
"Simple case": {
text: "hello world",
expected: "hello world",
},
{
name: "Preserve the word XOXP",
"Preserve the word XOXP": {
text: "This is an XOXP token",
expected: "This is an XOXP token",
},
{
name: "Redact actual XOXP token",
"Redact actual XOXP token": {
text: `{"ok":true,"token":"xoxe.xoxp-123","refresh_token":"xoxe-1-123","team_id":"T0123","user_id":"U0123", "xxtoken":"123"}`,
expected: `{"ok":true,"token":"...","refresh_token":"...","team_id":"T0123","user_id":"U0123", "xxtoken":"..."}`,
},
{
name: "Redact home directory",
"Redact home directory": {
text: "found authorizations at " + home + "/.slack/credentials.json reading",
expected: `found authorizations at .../.slack/credentials.json reading`,
},
{
name: "Redact username with single quotes",
"Redact username with single quotes": {
text: `'user':'username'`,
expected: `'user':"..."`,
},
{
name: "Redact username with double quotes",
"Redact username with double quotes": {
text: `"user":"username"`,
expected: `"user":"..."`,
},
{
name: "Redact username with no quotes",
"Redact username with no quotes": {
text: `user:username`,
expected: `user:username`,
},
{
name: "Redact username in http response",
"Redact username in http response": {
text: `{"ok":true,"token":"xoxe.xoxp-123","refresh_token":"xoxe-1-123","team_id":"T0123","user_id":"U0123", "xxtoken":"123", "user":"username"}`,
expected: `{"ok":true,"token":"...","refresh_token":"...","team_id":"T0123","user_id":"U0123", "xxtoken":"...", "user":"..."}`,
},
{
name: "Preserve the word XOXE",
"Preserve the word XOXE": {
text: "This is an XOXE token",
expected: "This is an XOXE token",
},
{
name: "Redact actual token in HTTP request",
"Redact actual token in HTTP request": {
text: "HTTP Request Body:refresh_token=xoxe-1",
expected: `HTTP Request Body:refresh_token=...`,
},
{
name: "Display Trace ID in log",
"Display Trace ID in log": {
text: "TraceID: 123",
expected: `TraceID: 123`,
},
{
name: "Display Team ID in log",
"Display Team ID in log": {
text: "TeamID: T123",
expected: `TeamID: T123`,
},
{
name: "Display User ID in log",
"Display User ID in log": {
text: "UserID: U123",
expected: `UserID: U123`,
},
{
name: "Display Slack-CLI version in log",
"Display Slack-CLI version in log": {
text: "Slack-CLI Version: v1.10.0",
expected: `Slack-CLI Version: v1.10.0`,
},
{
name: "Display user's OS in log",
"Display user's OS in log": {
text: "Operating System (OS): darwin",
expected: `Operating System (OS): darwin`,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
redacted := goutils.RedactPII(tc.text)
require.Equal(t, redacted, tc.expected)
})
Expand Down
78 changes: 40 additions & 38 deletions internal/app/app_client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,46 +692,48 @@ func TestAppClient_CleanupAppsJSONFiles(t *testing.T) {
},
}

for _, tc := range tests {
ac, _, _, pathToAppsJSON, pathToDevAppsJSON, teardown := setup(t)
defer teardown(t)
ctx := slackcontext.MockContext(t.Context())

err := afero.WriteFile(ac.fs, pathToAppsJSON, tc.appsJSON, 0600)
require.NoError(t, err)
err = afero.WriteFile(ac.fs, pathToDevAppsJSON, tc.devAppsJSON, 0600)
require.NoError(t, err)

_, err = ac.fs.Stat(pathToAppsJSON)
require.NoError(t, err, "failed to access the apps.json file")
deployedApps, _, err := ac.GetDeployedAll(ctx)
require.NoError(t, err)

_, err = ac.fs.Stat(pathToDevAppsJSON)
require.NoError(t, err, "failed to access the apps.dev.json file")
localApps, err := ac.GetLocalAll(ctx)
require.NoError(t, err)

ac.CleanUp()

dotSlackFolder := filepath.Dir(pathToAppsJSON)
_, err = ac.fs.Stat(dotSlackFolder)
require.NoError(t, err, "failed to access the .slack directory")

appsJSON, err := afero.ReadFile(ac.fs, pathToAppsJSON)
if len(deployedApps) == 0 {
require.ErrorIs(t, err, os.ErrNotExist, "apps.json was not deleted")
} else {
for name, tc := range tests {
Copy link
Member Author

@mwbrooks mwbrooks Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: This ---/+++ looks scary, but it's just fixing this test by wrapping each iteration in a t.Run function.

t.Run(name, func(t *testing.T) {
ac, _, _, pathToAppsJSON, pathToDevAppsJSON, teardown := setup(t)
defer teardown(t)
ctx := slackcontext.MockContext(t.Context())

err := afero.WriteFile(ac.fs, pathToAppsJSON, tc.appsJSON, 0600)
require.NoError(t, err)
err = afero.WriteFile(ac.fs, pathToDevAppsJSON, tc.devAppsJSON, 0600)
require.NoError(t, err)

_, err = ac.fs.Stat(pathToAppsJSON)
require.NoError(t, err, "failed to access the apps.json file")
assert.Equal(t, appsJSONExample, appsJSON)
}
deployedApps, _, err := ac.GetDeployedAll(ctx)
require.NoError(t, err)

devAppsJSON, err := afero.ReadFile(ac.fs, pathToDevAppsJSON)
if len(localApps) == 0 {
require.ErrorIs(t, err, os.ErrNotExist, "apps.dev.json was not deleted")
} else {
_, err = ac.fs.Stat(pathToDevAppsJSON)
require.NoError(t, err, "failed to access the apps.dev.json file")
assert.Equal(t, devAppsJSONExample, devAppsJSON)
}
localApps, err := ac.GetLocalAll(ctx)
require.NoError(t, err)

ac.CleanUp()

dotSlackFolder := filepath.Dir(pathToAppsJSON)
_, err = ac.fs.Stat(dotSlackFolder)
require.NoError(t, err, "failed to access the .slack directory")

appsJSON, err := afero.ReadFile(ac.fs, pathToAppsJSON)
if len(deployedApps) == 0 {
require.ErrorIs(t, err, os.ErrNotExist, "apps.json was not deleted")
} else {
require.NoError(t, err, "failed to access the apps.json file")
assert.Equal(t, appsJSONExample, appsJSON)
}

devAppsJSON, err := afero.ReadFile(ac.fs, pathToDevAppsJSON)
if len(localApps) == 0 {
require.ErrorIs(t, err, os.ErrNotExist, "apps.dev.json was not deleted")
} else {
require.NoError(t, err, "failed to access the apps.dev.json file")
assert.Equal(t, devAppsJSONExample, devAppsJSON)
}
})
}
}
19 changes: 7 additions & 12 deletions internal/app/app_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,39 +128,34 @@ func Test_App_UpdateDefaultProjectFiles(t *testing.T) {
}

func Test_RegexReplaceAppNameInManifest(t *testing.T) {
tests := []struct {
name string
tests := map[string]struct {
src []byte
appName string
expectedSrc []byte
}{
{
name: "manifest.json is validate",
"manifest.json is validate": {
src: testdata.ManifestJSON,
appName: "vibrant-butterfly-1234",
expectedSrc: testdata.ManifestJSONAppName,
},
{
name: "manifest.js is validate",
"manifest.js is validate": {
src: testdata.ManifestJS,
appName: "vibrant-butterfly-1234",
expectedSrc: testdata.ManifestJSAppName,
},
{
name: "manifest.ts is validate",
"manifest.ts is validate": {
src: testdata.ManifestTS,
appName: "vibrant-butterfly-1234",
expectedSrc: testdata.ManifestTSAppName,
},
{
name: "manifest.ts with sdk is validate",
"manifest.ts with sdk is validate": {
src: testdata.ManifestSDKTS,
appName: "vibrant-butterfly-1234",
expectedSrc: testdata.ManifestSDKTSAppName,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
actualSrc := regexReplaceAppNameInManifest(tc.src, tc.appName)
require.Equal(t, tc.expectedSrc, actualSrc)
})
Expand Down
Loading
Loading