Skip to content

refactor(tests): convert the low-risk slice-to-map table tests #321

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

Merged
mwbrooks merged 9 commits into from
Feb 2, 2026
+78 −132
Merged

refactor(tests): convert the low-risk slice-to-map table tests #321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
11aa4f7
refactor(tests): use tests/tc naming in table tests using the slice p…
mwbrooks Jan 28, 2026
048e6cb
Merge branch 'main' into mwbrooks-table-test-consistent
mwbrooks Jan 28, 2026
3933f61
test: update map pattern table tests to use common naming pattern
mwbrooks Jan 28, 2026
3b361c3
test: update slice pattern table tests to map pattern (simple use-cases)
mwbrooks Jan 28, 2026
58dcc8b
Merge branch 'main' into mwbrooks-table-test-consistent-p3-slice-to-m…
mwbrooks Jan 31, 2026
7a210a8
refactor(tests): cleanup formatting
mwbrooks Jan 31, 2026
b26fd64
refactor(tests): convert the low-risk slice-to-map table tests
mwbrooks Jan 31, 2026
3a3c826
test: change TODO to a regular comment
mwbrooks Jan 31, 2026
86e224c
Merge branch 'main' into mwbrooks-table-test-consistent-p4-low-risk
mwbrooks Feb 2, 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
19 changes: 7 additions & 12 deletions internal/runtime/runtime_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,34 +27,29 @@ import (
)

func Test_Runtime_New(t *testing.T) {
tests := []struct {
name string
tests := map[string]struct {
runtime string
expectedRuntimeType Runtime
}{
{
name: "Deno SDK",
"Deno SDK": {
runtime: "deno",
expectedRuntimeType: deno.New(),
},
{
name: "Bolt for JavaScript",
"Bolt for JavaScript": {
runtime: "node",
expectedRuntimeType: node.New(),
},
{
name: "Bolt for Python",
"Bolt for Python": {
runtime: "python",
expectedRuntimeType: python.New(),
},
{
name: "Unsupported Runtime",
"Unsupported Runtime": {
runtime: "biggly-boo",
expectedRuntimeType: nil,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
// Run the test
rt, _ := New(tc.runtime)
require.IsType(t, tc.expectedRuntimeType, rt)
Expand Down
10 changes: 4 additions & 6 deletions internal/shared/types/app_manifest_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,19 @@ import (
)

func Test_RawJSON_UnmarshalJSON(t *testing.T) {
tests := []struct {
name string
tests := map[string]struct {
blob string
expectedErrorType error
expectedJSONData string
}{
{
name: "Unmarshal data",
"Unmarshal data": {
blob: `{ "name": "foo" }`,
expectedErrorType: nil,
expectedJSONData: `{ "name": "foo" }`,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
rawJSON := &RawJSON{}
err := rawJSON.UnmarshalJSON([]byte(tc.blob))

Expand Down
57 changes: 21 additions & 36 deletions internal/shared/types/auth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,21 @@ import (
)

func Test_SlackAuth_AuthLevel(t *testing.T) {
tests := []struct {
name string
tests := map[string]struct {
auth *SlackAuth
expectedAuthLevel string
}{
{
name: "Workspace-level Auth",
"Workspace-level Auth": {
auth: &SlackAuth{IsEnterpriseInstall: false},
expectedAuthLevel: AuthLevelWorkspace,
},
{
name: "Enterprise-level Auth",
"Enterprise-level Auth": {
auth: &SlackAuth{IsEnterpriseInstall: true},
expectedAuthLevel: AuthLevelEnterprise,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
require.Equal(t, tc.auth.AuthLevel(), tc.expectedAuthLevel)
})
}
Expand All @@ -50,44 +47,37 @@ func Test_SlackAuth_ShouldRotateToken(t *testing.T) {
var refreshToken = "fakeRefreshToken"
var timeNow = int(time.Now().Unix())

tests := []struct {
name string
tests := map[string]struct {
input *SlackAuth
expected bool
}{
{
name: "nil case",
"nil case": {
input: nil,
expected: false,
},
{
name: "token but no refresh token",
"token but no refresh token": {
input: &SlackAuth{Token: token},
expected: false,
},
{
name: "token + refresh token present but expiration time is absent",
"token + refresh token present but expiration time is absent": {
input: &SlackAuth{Token: token, RefreshToken: refreshToken},
expected: false,
},
{
name: "token + refresh token + expiration present - but token expires in less than 5min",
"token + refresh token + expiration present - but token expires in less than 5min": {
input: &SlackAuth{Token: token, RefreshToken: refreshToken, ExpiresAt: timeNow + 290},
expected: true,
},
{
name: "token + refresh token + expiration present - and token does not expire in less than 5min",
"token + refresh token + expiration present - and token does not expire in less than 5min": {
input: &SlackAuth{Token: token, RefreshToken: refreshToken, ExpiresAt: timeNow + 310},
expected: false,
},
{
name: "token + refresh token + expiration present - and token expires in exactly 5min",
"token + refresh token + expiration present - and token expires in exactly 5min": {
input: &SlackAuth{Token: token, RefreshToken: refreshToken, ExpiresAt: timeNow + 300},
expected: true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
require.Equal(t, tc.expected, tc.input.ShouldRotateToken())
})
}
Expand All @@ -97,34 +87,29 @@ func Test_SlackAuth_TokenIsExpired(t *testing.T) {
var token = "fakeToken"
var timeNow = int(time.Now().Unix())

tests := []struct {
name string
tests := map[string]struct {
input *SlackAuth
expected bool
}{
{
name: "nil case",
"nil case": {
input: nil,
expected: false,
},
{
name: "token but no expiration",
"token but no expiration": {
input: &SlackAuth{Token: token},
expected: false,
},
{
name: "token + expiration present - but token is expired",
"token + expiration present - but token is expired": {
input: &SlackAuth{Token: token, ExpiresAt: timeNow - 1},
expected: true,
},
{
name: "token + expiration present - and token is not expired",
"token + expiration present - and token is not expired": {
input: &SlackAuth{Token: token, ExpiresAt: timeNow + 1},
expected: false,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
require.Equal(t, tc.expected, tc.input.TokenIsExpired())
})
}
Expand Down
4 changes: 2 additions & 2 deletions internal/shared/types/channel_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ func Test_SlackChannel_String(t *testing.T) {
channel: &SlackChannel{ChannelName: "#general", ID: "C01234"},
expectedString: "#general (C01234)",
},
// TODO(@mbrooks) This test represents the current behaviour, but should the expectedString be "#general" instead?
// This test represents the current behaviour, but should the expectedString be "#general" instead?
"Only ChannelName exists": {
channel: &SlackChannel{ChannelName: "#general"},
expectedString: "(#general)",
},
// TODO(@mbrooks) This test represents the current behaviour, but should the expectedString be "C01234" instead?
// This test represents the current behaviour, but should the expectedString be "C01234" instead?
Copy link
Member

@zimeg zimeg Feb 2, 2026

Choose a reason for hiding this comment

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

⭐ praise: Thanks for keeping note of this! Calling out strange but expected behavior IMHO is so useful in later refactors with assumptions removed of the unknown!

Copy link
Member Author

@mwbrooks mwbrooks Feb 2, 2026

Choose a reason for hiding this comment

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

Agreed! Documenting unexpected-expected behaviour can save us maintainers a huge amount of mental load. Since we don't have an immediate plan to change this, I thought I'd remove the TODO pointing back at myself 😆

"Only ID exists": {
channel: &SlackChannel{ID: "C01234"},
expectedString: "",
Expand Down
44 changes: 16 additions & 28 deletions internal/shared/types/icon_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,28 +22,24 @@ import (
)

func Test_Icons_MarshalJSON(t *testing.T) {
tests := []struct {
name string
tests := map[string]struct {
icons *Icons
expectedErrorType error
expectedBlobs []string
}{
{
name: "Marshal no icons",
"Marshal no icons": {
icons: &Icons{},
expectedErrorType: nil,
expectedBlobs: []string{},
},
{
name: "Marshal 1 icon",
"Marshal 1 icon": {
icons: &Icons{"image_96": "path/to/image_96.png"},
expectedErrorType: nil,
expectedBlobs: []string{
`"image_96":"path/to/image_96.png"`,
},
},
{
name: "Marshal 2 icons",
"Marshal 2 icons": {
icons: &Icons{"image_96": "path/to/image_96.png", "image_192": "path/to/image_192.png"},
expectedErrorType: nil,
expectedBlobs: []string{
Expand All @@ -52,8 +48,8 @@ func Test_Icons_MarshalJSON(t *testing.T) {
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
returnedBlob, err := json.Marshal(tc.icons)

require.IsType(t, err, tc.expectedErrorType)
Expand All @@ -65,65 +61,57 @@ func Test_Icons_MarshalJSON(t *testing.T) {
}

func Test_Icons_UnmarshalJSON(t *testing.T) {
tests := []struct {
name string
tests := map[string]struct {
blob string
icons *Icons
expectedErrorType error
expectedIcons *Icons
}{
{
name: "JSON unmarshal error",
"JSON unmarshal error": {
blob: `{ "image_96": 100 }`, // expects type to be string not int
icons: &Icons{},
expectedErrorType: &json.UnmarshalTypeError{},
expectedIcons: &Icons{},
},
{
name: "image_96 and image_192 do not exist",
"image_96 and image_192 do not exist": {
blob: `{ "cat": "meow", "dog": "bark" }`,
icons: &Icons{},
expectedErrorType: nil,
expectedIcons: &Icons{},
},
{
name: "image_96 exists",
"image_96 exists": {
blob: `{ "image_96": "path/to/image_96.png" }`,
icons: &Icons{},
expectedErrorType: nil,
expectedIcons: &Icons{"image_96": "path/to/image_96.png"},
},
{
name: "image_192 exists",
"image_192 exists": {
blob: `{ "image_192": "path/to/image_192.png" }`,
icons: &Icons{},
expectedErrorType: nil,
expectedIcons: &Icons{"image_192": "path/to/image_192.png"},
},
{
name: "image_96 and image_192 exist",
"image_96 and image_192 exist": {
blob: `{ "image_96": "path/to/image_96.png", "image_192": "path/to/image_192.png" }`,
icons: &Icons{},
expectedErrorType: nil,
expectedIcons: &Icons{"image_96": "path/to/image_96.png", "image_192": "path/to/image_192.png"},
},
{
name: "image_96 exists and unsupported properties exists",
"image_96 exists and unsupported properties exists": {
blob: `{ "image_96": "path/to/image_96.png", "foo": "bar" }`,
icons: &Icons{},
expectedErrorType: nil,
expectedIcons: &Icons{"image_96": "path/to/image_96.png", "foo": "bar"},
},
{
name: "Icons is nil, image_96 and image_192 exist",
"Icons is nil, image_96 and image_192 exist": {
blob: `{ "image_96": "path/to/image_96.png", "image_192": "path/to/image_192.png" }`,
icons: nil,
expectedErrorType: nil,
expectedIcons: &Icons{"image_96": "path/to/image_96.png", "image_192": "path/to/image_192.png"},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
err := json.Unmarshal([]byte(tc.blob), &tc.icons)

require.IsType(t, err, tc.expectedErrorType)
Expand Down
Loading
Loading