From 84aab1f6a068d43086e4990090ed422e6ac655e0 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 11 Mar 2026 10:17:27 -0700 Subject: [PATCH 1/9] test: add test coverage for untested files --- internal/api/client_test.go | 52 ++++++ internal/api/collaborators_test.go | 115 +++++++++++++ internal/archiveutil/archiveutil_test.go | 142 ++++++++++++++++ internal/cmdutil/flags_test.go | 43 +++++ internal/config/context_test.go | 138 ++++++++++++++++ internal/goutils/recursivecopy_test.go | 201 +++++++++++++++++++++++ internal/image/image_test.go | 123 ++++++++++++++ internal/iostreams/reader_test.go | 41 +++++ internal/ioutils/host_test.go | 23 +++ internal/prompts/permissions_test.go | 66 ++++++++ 10 files changed, 944 insertions(+) create mode 100644 internal/api/client_test.go create mode 100644 internal/api/collaborators_test.go create mode 100644 internal/archiveutil/archiveutil_test.go create mode 100644 internal/cmdutil/flags_test.go create mode 100644 internal/config/context_test.go create mode 100644 internal/goutils/recursivecopy_test.go create mode 100644 internal/image/image_test.go create mode 100644 internal/iostreams/reader_test.go create mode 100644 internal/ioutils/host_test.go create mode 100644 internal/prompts/permissions_test.go diff --git a/internal/api/client_test.go b/internal/api/client_test.go new file mode 100644 index 00000000..24f2adbc --- /dev/null +++ b/internal/api/client_test.go @@ -0,0 +1,52 @@ +package api + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestHost(t *testing.T) { + tests := map[string]struct { + host string + expected string + }{ + "returns the configured host": { + host: "https://slack.com", + expected: "https://slack.com", + }, + "returns empty when unset": { + host: "", + expected: "", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + c := &Client{host: tc.host} + assert.Equal(t, tc.expected, c.Host()) + }) + } +} + +func TestSetHost(t *testing.T) { + tests := map[string]struct { + initial string + newHost string + }{ + "sets a new host": { + initial: "", + newHost: "https://dev.slack.com", + }, + "overwrites existing host": { + initial: "https://slack.com", + newHost: "https://dev.slack.com", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + c := &Client{host: tc.initial} + c.SetHost(tc.newHost) + assert.Equal(t, tc.newHost, c.Host()) + }) + } +} diff --git a/internal/api/collaborators_test.go b/internal/api/collaborators_test.go new file mode 100644 index 00000000..4adae5ec --- /dev/null +++ b/internal/api/collaborators_test.go @@ -0,0 +1,115 @@ +package api + +import ( + "testing" + + "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackcontext" + "github.com/stretchr/testify/require" +) + +func TestClient_AddCollaborator_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: collaboratorsAddMethod, + Response: `{"ok":true}`, + }) + defer teardown() + err := c.AddCollaborator(ctx, "token", "A123", types.SlackUser{Email: "user@example.com", PermissionType: "owner"}) + require.NoError(t, err) +} + +func TestClient_AddCollaborator_WithUserID(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: collaboratorsAddMethod, + Response: `{"ok":true}`, + }) + defer teardown() + err := c.AddCollaborator(ctx, "token", "A123", types.SlackUser{ID: "U123", PermissionType: "owner"}) + require.NoError(t, err) +} + +func TestClient_AddCollaborator_Error(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: collaboratorsAddMethod, + Response: `{"ok":false,"error":"user_not_found"}`, + }) + defer teardown() + err := c.AddCollaborator(ctx, "token", "A123", types.SlackUser{Email: "bad@example.com"}) + require.Error(t, err) + require.Contains(t, err.Error(), "user_not_found") +} + +func TestClient_ListCollaborators_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: collaboratorsListMethod, + Response: `{"ok":true,"owners":[{"user_id":"U123","username":"Test User"}]}`, + }) + defer teardown() + users, err := c.ListCollaborators(ctx, "token", "A123") + require.NoError(t, err) + require.Len(t, users, 1) + require.Equal(t, "U123", users[0].ID) +} + +func TestClient_ListCollaborators_Error(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: collaboratorsListMethod, + Response: `{"ok":false,"error":"app_not_found"}`, + }) + defer teardown() + _, err := c.ListCollaborators(ctx, "token", "A123") + require.Error(t, err) + require.Contains(t, err.Error(), "app_not_found") +} + +func TestClient_RemoveCollaborator_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: collaboratorsRemoveMethod, + Response: `{"ok":true}`, + }) + defer teardown() + warnings, err := c.RemoveCollaborator(ctx, "token", "A123", types.SlackUser{ID: "U123"}) + require.NoError(t, err) + require.Empty(t, warnings) +} + +func TestClient_RemoveCollaborator_Error(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: collaboratorsRemoveMethod, + Response: `{"ok":false,"error":"cannot_remove_owner"}`, + }) + defer teardown() + _, err := c.RemoveCollaborator(ctx, "token", "A123", types.SlackUser{Email: "owner@example.com"}) + require.Error(t, err) + require.Contains(t, err.Error(), "cannot_remove_owner") +} + +func TestClient_UpdateCollaborator_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: collaboratorsUpdateMethod, + Response: `{"ok":true}`, + }) + defer teardown() + err := c.UpdateCollaborator(ctx, "token", "A123", types.SlackUser{ID: "U123", PermissionType: "collaborator"}) + require.NoError(t, err) +} + +func TestClient_UpdateCollaborator_Error(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: collaboratorsUpdateMethod, + Response: `{"ok":false,"error":"invalid_permission"}`, + }) + defer teardown() + err := c.UpdateCollaborator(ctx, "token", "A123", types.SlackUser{ID: "U123", PermissionType: "invalid"}) + require.Error(t, err) + require.Contains(t, err.Error(), "invalid_permission") +} diff --git a/internal/archiveutil/archiveutil_test.go b/internal/archiveutil/archiveutil_test.go new file mode 100644 index 00000000..059d871c --- /dev/null +++ b/internal/archiveutil/archiveutil_test.go @@ -0,0 +1,142 @@ +package archiveutil + +import ( + "archive/tar" + "archive/zip" + "compress/gzip" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestExtractAndWriteFile(t *testing.T) { + tests := map[string]struct { + fileName string + content string + isDir bool + }{ + "extracts a regular file": { + fileName: "hello.txt", + content: "hello world", + isDir: false, + }, + "creates a directory": { + fileName: "subdir/", + isDir: true, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + destDir := t.TempDir() + reader := strings.NewReader(tc.content) + + path, err := ExtractAndWriteFile(reader, destDir, tc.fileName, tc.isDir, 0644) + require.NoError(t, err) + assert.True(t, strings.HasPrefix(path, destDir)) + + if tc.isDir { + info, err := os.Stat(path) + require.NoError(t, err) + assert.True(t, info.IsDir()) + } else { + content, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, tc.content, string(content)) + } + }) + } + + t.Run("rejects path traversal", func(t *testing.T) { + destDir := t.TempDir() + reader := strings.NewReader("malicious") + _, err := ExtractAndWriteFile(reader, destDir, "../../../etc/passwd", false, 0644) + assert.Error(t, err) + assert.Contains(t, err.Error(), "illegal file path") + }) +} + +func TestUnzip(t *testing.T) { + t.Run("extracts a zip archive", func(t *testing.T) { + srcDir := t.TempDir() + destDir := t.TempDir() + + // Create a zip file + zipPath := filepath.Join(srcDir, "test.zip") + zipFile, err := os.Create(zipPath) + require.NoError(t, err) + + w := zip.NewWriter(zipFile) + f, err := w.Create("test.txt") + require.NoError(t, err) + _, err = f.Write([]byte("zip content")) + require.NoError(t, err) + err = w.Close() + require.NoError(t, err) + err = zipFile.Close() + require.NoError(t, err) + + files, err := Unzip(zipPath, destDir) + require.NoError(t, err) + assert.NotEmpty(t, files) + + content, err := os.ReadFile(filepath.Join(destDir, "test.txt")) + require.NoError(t, err) + assert.Equal(t, "zip content", string(content)) + }) + + t.Run("returns error for non-existent file", func(t *testing.T) { + _, err := Unzip("/nonexistent.zip", t.TempDir()) + assert.Error(t, err) + }) +} + +func TestUntarGzip(t *testing.T) { + t.Run("extracts a tar.gz archive", func(t *testing.T) { + srcDir := t.TempDir() + destDir := t.TempDir() + + // Create a tar.gz file + tgzPath := filepath.Join(srcDir, "test.tar.gz") + tgzFile, err := os.Create(tgzPath) + require.NoError(t, err) + + gw := gzip.NewWriter(tgzFile) + tw := tar.NewWriter(gw) + + content := []byte("tar content") + hdr := &tar.Header{ + Name: "test.txt", + Mode: 0644, + Size: int64(len(content)), + Typeflag: tar.TypeReg, + } + err = tw.WriteHeader(hdr) + require.NoError(t, err) + _, err = tw.Write(content) + require.NoError(t, err) + + err = tw.Close() + require.NoError(t, err) + err = gw.Close() + require.NoError(t, err) + err = tgzFile.Close() + require.NoError(t, err) + + files, err := UntarGzip(tgzPath, destDir) + require.NoError(t, err) + assert.NotEmpty(t, files) + + data, err := os.ReadFile(filepath.Join(destDir, "test.txt")) + require.NoError(t, err) + assert.Equal(t, "tar content", string(data)) + }) + + t.Run("returns error for non-existent file", func(t *testing.T) { + _, err := UntarGzip("/nonexistent.tar.gz", t.TempDir()) + assert.Error(t, err) + }) +} diff --git a/internal/cmdutil/flags_test.go b/internal/cmdutil/flags_test.go new file mode 100644 index 00000000..882f3b1c --- /dev/null +++ b/internal/cmdutil/flags_test.go @@ -0,0 +1,43 @@ +package cmdutil + +import ( + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" +) + +func TestIsFlagChanged(t *testing.T) { + tests := map[string]struct { + flag string + setFlag bool + expected bool + }{ + "returns true when flag is set": { + flag: "app-id", + setFlag: true, + expected: true, + }, + "returns false when flag exists but not set": { + flag: "app-id", + setFlag: false, + expected: false, + }, + "returns false when flag does not exist": { + flag: "nonexistent", + setFlag: false, + expected: false, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cmd.Flags().String("app-id", "", "app ID") + if tc.setFlag && tc.flag == "app-id" { + _ = cmd.Flags().Set("app-id", "A12345") + } + result := IsFlagChanged(cmd, tc.flag) + assert.Equal(t, tc.expected, result) + }) + } +} diff --git a/internal/config/context_test.go b/internal/config/context_test.go new file mode 100644 index 00000000..35df227e --- /dev/null +++ b/internal/config/context_test.go @@ -0,0 +1,138 @@ +package config + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestContextToken(t *testing.T) { + tests := map[string]struct { + token string + expected string + }{ + "set and get a token": { + token: "xoxb-test-token", + expected: "xoxb-test-token", + }, + "set and get an empty token": { + token: "", + expected: "", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := SetContextToken(context.Background(), tc.token) + assert.Equal(t, tc.expected, GetContextToken(ctx)) + }) + } + + t.Run("get from empty context returns empty string", func(t *testing.T) { + assert.Equal(t, "", GetContextToken(context.Background())) + }) +} + +func TestContextEnterpriseID(t *testing.T) { + tests := map[string]struct { + enterpriseID string + expected string + }{ + "set and get an enterprise ID": { + enterpriseID: "E12345", + expected: "E12345", + }, + "set and get an empty enterprise ID": { + enterpriseID: "", + expected: "", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := SetContextEnterpriseID(context.Background(), tc.enterpriseID) + assert.Equal(t, tc.expected, GetContextEnterpriseID(ctx)) + }) + } + + t.Run("get from empty context returns empty string", func(t *testing.T) { + assert.Equal(t, "", GetContextEnterpriseID(context.Background())) + }) +} + +func TestContextTeamID(t *testing.T) { + tests := map[string]struct { + teamID string + expected string + }{ + "set and get a team ID": { + teamID: "T12345", + expected: "T12345", + }, + "set and get an empty team ID": { + teamID: "", + expected: "", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := SetContextTeamID(context.Background(), tc.teamID) + assert.Equal(t, tc.expected, GetContextTeamID(ctx)) + }) + } + + t.Run("get from empty context returns empty string", func(t *testing.T) { + assert.Equal(t, "", GetContextTeamID(context.Background())) + }) +} + +func TestContextTeamDomain(t *testing.T) { + tests := map[string]struct { + teamDomain string + expected string + }{ + "set and get a team domain": { + teamDomain: "subarachnoid", + expected: "subarachnoid", + }, + "set and get an empty team domain": { + teamDomain: "", + expected: "", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := SetContextTeamDomain(context.Background(), tc.teamDomain) + assert.Equal(t, tc.expected, GetContextTeamDomain(ctx)) + }) + } + + t.Run("get from empty context returns empty string", func(t *testing.T) { + assert.Equal(t, "", GetContextTeamDomain(context.Background())) + }) +} + +func TestContextUserID(t *testing.T) { + tests := map[string]struct { + userID string + expected string + }{ + "set and get a user ID": { + userID: "U12345", + expected: "U12345", + }, + "set and get an empty user ID": { + userID: "", + expected: "", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := SetContextUserID(context.Background(), tc.userID) + assert.Equal(t, tc.expected, GetContextUserID(ctx)) + }) + } + + t.Run("get from empty context returns empty string", func(t *testing.T) { + assert.Equal(t, "", GetContextUserID(context.Background())) + }) +} diff --git a/internal/goutils/recursivecopy_test.go b/internal/goutils/recursivecopy_test.go new file mode 100644 index 00000000..c8ac2a32 --- /dev/null +++ b/internal/goutils/recursivecopy_test.go @@ -0,0 +1,201 @@ +package goutils + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestShouldIgnore(t *testing.T) { + tests := map[string]struct { + val string + list []string + ignoreFunc func(string) bool + expected bool + }{ + "returns true when value is in list": { + val: "node_modules", + list: []string{"node_modules", ".git"}, + expected: true, + }, + "returns false when value is not in list": { + val: "src", + list: []string{"node_modules", ".git"}, + expected: false, + }, + "returns false with empty list and nil ignoreFunc": { + val: "anything", + list: []string{}, + expected: false, + }, + "returns false with nil list and nil ignoreFunc": { + val: "anything", + list: nil, + expected: false, + }, + "returns true when ignoreFunc returns true": { + val: "secret.txt", + list: []string{}, + ignoreFunc: func(s string) bool { + return s == "secret.txt" + }, + expected: true, + }, + "returns false when ignoreFunc returns false": { + val: "readme.md", + list: []string{}, + ignoreFunc: func(s string) bool { + return s == "secret.txt" + }, + expected: false, + }, + "list match takes priority over ignoreFunc": { + val: "node_modules", + list: []string{"node_modules"}, + ignoreFunc: func(s string) bool { + return false + }, + expected: true, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + result := ShouldIgnore(tc.val, tc.list, tc.ignoreFunc) + assert.Equal(t, tc.expected, result) + }) + } +} + +func TestCopy(t *testing.T) { + t.Run("copies a regular file", func(t *testing.T) { + srcDir := t.TempDir() + dstDir := t.TempDir() + + srcFile := filepath.Join(srcDir, "test.txt") + dstFile := filepath.Join(dstDir, "test.txt") + + err := os.WriteFile(srcFile, []byte("hello world"), 0644) + require.NoError(t, err) + + err = Copy(srcFile, dstFile) + require.NoError(t, err) + + content, err := os.ReadFile(dstFile) + require.NoError(t, err) + assert.Equal(t, "hello world", string(content)) + }) + + t.Run("returns error for non-existent source", func(t *testing.T) { + dstDir := t.TempDir() + err := Copy("/nonexistent/file.txt", filepath.Join(dstDir, "file.txt")) + assert.Error(t, err) + }) +} + +func TestCopySymLink(t *testing.T) { + t.Run("copies a symbolic link", func(t *testing.T) { + srcDir := t.TempDir() + dstDir := t.TempDir() + + // Create a target file and a symlink to it + targetFile := filepath.Join(srcDir, "target.txt") + err := os.WriteFile(targetFile, []byte("target content"), 0644) + require.NoError(t, err) + + srcLink := filepath.Join(srcDir, "link.txt") + err = os.Symlink(targetFile, srcLink) + require.NoError(t, err) + + dstLink := filepath.Join(dstDir, "link.txt") + err = CopySymLink(srcLink, dstLink) + require.NoError(t, err) + + // Verify the symlink target is the same + linkTarget, err := os.Readlink(dstLink) + require.NoError(t, err) + assert.Equal(t, targetFile, linkTarget) + }) +} + +func TestCopyDirectory(t *testing.T) { + t.Run("copies directory structure", func(t *testing.T) { + srcDir := t.TempDir() + dstDir := filepath.Join(t.TempDir(), "dst") + + // Create source structure + err := os.MkdirAll(filepath.Join(srcDir, "subdir"), 0755) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(srcDir, "file1.txt"), []byte("file1"), 0644) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(srcDir, "subdir", "file2.txt"), []byte("file2"), 0644) + require.NoError(t, err) + + err = CopyDirectory(CopyDirectoryOpts{Src: srcDir, Dst: dstDir}) + require.NoError(t, err) + + // Verify files were copied + content, err := os.ReadFile(filepath.Join(dstDir, "file1.txt")) + require.NoError(t, err) + assert.Equal(t, "file1", string(content)) + + content, err = os.ReadFile(filepath.Join(dstDir, "subdir", "file2.txt")) + require.NoError(t, err) + assert.Equal(t, "file2", string(content)) + }) + + t.Run("ignores specified files", func(t *testing.T) { + srcDir := t.TempDir() + dstDir := filepath.Join(t.TempDir(), "dst") + + err := os.WriteFile(filepath.Join(srcDir, "keep.txt"), []byte("keep"), 0644) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(srcDir, "ignore.txt"), []byte("ignore"), 0644) + require.NoError(t, err) + + err = CopyDirectory(CopyDirectoryOpts{ + Src: srcDir, + Dst: dstDir, + IgnoreFiles: []string{"ignore.txt"}, + }) + require.NoError(t, err) + + assert.FileExists(t, filepath.Join(dstDir, "keep.txt")) + assert.NoFileExists(t, filepath.Join(dstDir, "ignore.txt")) + }) + + t.Run("ignores specified directories", func(t *testing.T) { + srcDir := t.TempDir() + dstDir := filepath.Join(t.TempDir(), "dst") + + err := os.MkdirAll(filepath.Join(srcDir, "keep_dir"), 0755) + require.NoError(t, err) + err = os.MkdirAll(filepath.Join(srcDir, "node_modules"), 0755) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(srcDir, "keep_dir", "file.txt"), []byte("keep"), 0644) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(srcDir, "node_modules", "file.txt"), []byte("ignore"), 0644) + require.NoError(t, err) + + err = CopyDirectory(CopyDirectoryOpts{ + Src: srcDir, + Dst: dstDir, + IgnoreDirectories: []string{"node_modules"}, + }) + require.NoError(t, err) + + assert.FileExists(t, filepath.Join(dstDir, "keep_dir", "file.txt")) + assert.NoDirExists(t, filepath.Join(dstDir, "node_modules")) + }) + + t.Run("returns error for non-existent source", func(t *testing.T) { + dstDir := filepath.Join(t.TempDir(), "dst") + err := CopyDirectory(CopyDirectoryOpts{ + Src: "/nonexistent/path", + Dst: dstDir, + }) + assert.Error(t, err) + }) +} diff --git a/internal/image/image_test.go b/internal/image/image_test.go new file mode 100644 index 00000000..7c5b170e --- /dev/null +++ b/internal/image/image_test.go @@ -0,0 +1,123 @@ +package image + +import ( + "bytes" + "image" + "image/color" + "image/png" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func createTestPNG(t *testing.T, width, height int) []byte { + t.Helper() + img := image.NewRGBA(image.Rect(0, 0, width, height)) + for x := range width { + for y := range height { + img.Set(x, y, color.RGBA{R: 255, G: 0, B: 0, A: 255}) + } + } + var buf bytes.Buffer + err := png.Encode(&buf, img) + require.NoError(t, err) + return buf.Bytes() +} + +func TestResizeImage(t *testing.T) { + pngData := createTestPNG(t, 100, 100) + reader := bytes.NewReader(pngData) + + resized, err := ResizeImage(reader, 50, 50) + require.NoError(t, err) + assert.NotNil(t, resized) + assert.Equal(t, 50, resized.Bounds().Dx()) + assert.Equal(t, 50, resized.Bounds().Dy()) +} + +func TestResizeImageToBytes(t *testing.T) { + pngData := createTestPNG(t, 100, 100) + reader := bytes.NewReader(pngData) + + result, err := ResizeImageToBytes(reader, 50, 50) + require.NoError(t, err) + assert.NotEmpty(t, result) +} + +func TestResizeImageFromFileToBytes(t *testing.T) { + pngData := createTestPNG(t, 100, 100) + fs := afero.NewMemMapFs() + err := afero.WriteFile(fs, "/test.png", pngData, 0644) + require.NoError(t, err) + + result, err := ResizeImageFromFileToBytes(fs, "/test.png", 50, 50) + require.NoError(t, err) + assert.NotEmpty(t, result) +} + +func TestResizeImageFromFileToBytes_FileNotFound(t *testing.T) { + fs := afero.NewMemMapFs() + _, err := ResizeImageFromFileToBytes(fs, "/nonexistent.png", 50, 50) + assert.Error(t, err) +} + +func TestCropResizeImageRatio(t *testing.T) { + pngData := createTestPNG(t, 200, 100) + reader := bytes.NewReader(pngData) + + result, err := CropResizeImageRatio(reader, 100, 1, 1) + require.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, 100, result.Bounds().Dx()) +} + +func TestCropResizeImageRatioToBytes(t *testing.T) { + pngData := createTestPNG(t, 200, 100) + reader := bytes.NewReader(pngData) + + result, err := CropResizeImageRatioToBytes(reader, 100, 1, 1) + require.NoError(t, err) + assert.NotEmpty(t, result) +} + +func TestCropResizeImageRatioFromFile(t *testing.T) { + pngData := createTestPNG(t, 200, 100) + fs := afero.NewMemMapFs() + err := afero.WriteFile(fs, "/test.png", pngData, 0644) + require.NoError(t, err) + + result, err := CropResizeImageRatioFromFile(fs, "/test.png", 100, 1, 1) + require.NoError(t, err) + assert.NotNil(t, result) +} + +func TestCropResizeImageRatioFromFile_FileNotFound(t *testing.T) { + fs := afero.NewMemMapFs() + _, err := CropResizeImageRatioFromFile(fs, "/nonexistent.png", 100, 1, 1) + assert.Error(t, err) +} + +func TestCropResizeImageRatioFromFileToBytes(t *testing.T) { + pngData := createTestPNG(t, 200, 100) + fs := afero.NewMemMapFs() + err := afero.WriteFile(fs, "/test.png", pngData, 0644) + require.NoError(t, err) + + result, err := CropResizeImageRatioFromFileToBytes(fs, "/test.png", 100, 1, 1) + require.NoError(t, err) + assert.NotEmpty(t, result) +} + +func TestCropResizeImageRatioFromFileToBytes_FileNotFound(t *testing.T) { + fs := afero.NewMemMapFs() + _, err := CropResizeImageRatioFromFileToBytes(fs, "/nonexistent.png", 100, 1, 1) + assert.Error(t, err) +} + +func TestResizeImage_InvalidReader(t *testing.T) { + reader := bytes.NewReader([]byte("not a valid image")) + _, err := ResizeImage(reader, 50, 50) + assert.Error(t, err) +} diff --git a/internal/iostreams/reader_test.go b/internal/iostreams/reader_test.go new file mode 100644 index 00000000..5f82ce50 --- /dev/null +++ b/internal/iostreams/reader_test.go @@ -0,0 +1,41 @@ +package iostreams + +import ( + "strings" + "testing" + + "github.com/slackapi/slack-cli/internal/config" + "github.com/slackapi/slack-cli/internal/slackdeps" + "github.com/stretchr/testify/assert" +) + +func TestReadIn(t *testing.T) { + tests := map[string]struct { + input string + expected string + }{ + "returns the configured stdin reader": { + input: "test input", + expected: "test input", + }, + "returns an empty reader": { + input: "", + expected: "", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + fsMock := slackdeps.NewFsMock() + osMock := slackdeps.NewOsMock() + cfg := config.NewConfig(fsMock, osMock) + io := NewIOStreams(cfg, fsMock, osMock) + io.Stdin = strings.NewReader(tc.input) + + reader := io.ReadIn() + + buf := make([]byte, len(tc.input)+1) + n, _ := reader.Read(buf) + assert.Equal(t, tc.expected, string(buf[:n])) + }) + } +} diff --git a/internal/ioutils/host_test.go b/internal/ioutils/host_test.go new file mode 100644 index 00000000..687a9e7f --- /dev/null +++ b/internal/ioutils/host_test.go @@ -0,0 +1,23 @@ +package ioutils + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGetHostname(t *testing.T) { + t.Run("returns a non-empty hashed hostname", func(t *testing.T) { + hostname := GetHostname() + assert.NotEmpty(t, hostname) + // The hostname should be hashed, not the raw hostname + // It should not be "unknown" on a normal system + assert.NotEqual(t, "", hostname) + }) + + t.Run("returns consistent results", func(t *testing.T) { + hostname1 := GetHostname() + hostname2 := GetHostname() + assert.Equal(t, hostname1, hostname2) + }) +} diff --git a/internal/prompts/permissions_test.go b/internal/prompts/permissions_test.go new file mode 100644 index 00000000..e8cf0392 --- /dev/null +++ b/internal/prompts/permissions_test.go @@ -0,0 +1,66 @@ +package prompts + +import ( + "testing" + + "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/stretchr/testify/assert" +) + +func TestAccessLabels(t *testing.T) { + tests := map[string]struct { + current types.Permission + expectedCurrent string + }{ + "app_collaborators as current": { + current: types.PermissionAppCollaborators, + expectedCurrent: "app collaborators only (current)", + }, + "everyone as current": { + current: types.PermissionEveryone, + expectedCurrent: "everyone (current)", + }, + "named_entities as current": { + current: types.PermissionNamedEntities, + expectedCurrent: "specific users (current)", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + labels, distributions := AccessLabels(tc.current) + assert.Len(t, labels, 3) + assert.Len(t, distributions, 3) + assert.Equal(t, tc.expectedCurrent, labels[0]) + assert.Equal(t, tc.current, distributions[0]) + }) + } +} + +func TestTriggerAccessLabels(t *testing.T) { + tests := map[string]struct { + current types.Permission + expectedCurrent string + }{ + "app_collaborators as current": { + current: types.PermissionAppCollaborators, + expectedCurrent: "app collaborators only (current)", + }, + "everyone as current": { + current: types.PermissionEveryone, + expectedCurrent: "everyone (current)", + }, + "named_entities as current": { + current: types.PermissionNamedEntities, + expectedCurrent: "specific entities (current)", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + labels, distributions := TriggerAccessLabels(tc.current) + assert.Len(t, labels, 3) + assert.Len(t, distributions, 3) + assert.Equal(t, tc.expectedCurrent, labels[0]) + assert.Equal(t, tc.current, distributions[0]) + }) + } +} From b6df3ac15260b18f4529b7251e78371f2c2fcec8 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 11 Mar 2026 12:00:20 -0700 Subject: [PATCH 2/9] chore: add license headers to new fils --- internal/api/client_test.go | 14 ++++++++++++++ internal/api/collaborators_test.go | 14 ++++++++++++++ internal/archiveutil/archiveutil_test.go | 14 ++++++++++++++ internal/cmdutil/flags_test.go | 14 ++++++++++++++ internal/config/context_test.go | 14 ++++++++++++++ internal/goutils/recursivecopy_test.go | 14 ++++++++++++++ internal/image/image_test.go | 14 ++++++++++++++ internal/iostreams/reader_test.go | 14 ++++++++++++++ internal/ioutils/host_test.go | 14 ++++++++++++++ internal/prompts/permissions_test.go | 14 ++++++++++++++ 10 files changed, 140 insertions(+) diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 24f2adbc..10190e2f 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -1,3 +1,17 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package api import ( diff --git a/internal/api/collaborators_test.go b/internal/api/collaborators_test.go index 4adae5ec..cfab7e35 100644 --- a/internal/api/collaborators_test.go +++ b/internal/api/collaborators_test.go @@ -1,3 +1,17 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package api import ( diff --git a/internal/archiveutil/archiveutil_test.go b/internal/archiveutil/archiveutil_test.go index 059d871c..99b445e2 100644 --- a/internal/archiveutil/archiveutil_test.go +++ b/internal/archiveutil/archiveutil_test.go @@ -1,3 +1,17 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package archiveutil import ( diff --git a/internal/cmdutil/flags_test.go b/internal/cmdutil/flags_test.go index 882f3b1c..90c93881 100644 --- a/internal/cmdutil/flags_test.go +++ b/internal/cmdutil/flags_test.go @@ -1,3 +1,17 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package cmdutil import ( diff --git a/internal/config/context_test.go b/internal/config/context_test.go index 35df227e..28b47168 100644 --- a/internal/config/context_test.go +++ b/internal/config/context_test.go @@ -1,3 +1,17 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package config import ( diff --git a/internal/goutils/recursivecopy_test.go b/internal/goutils/recursivecopy_test.go index c8ac2a32..4ffe9364 100644 --- a/internal/goutils/recursivecopy_test.go +++ b/internal/goutils/recursivecopy_test.go @@ -1,3 +1,17 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package goutils import ( diff --git a/internal/image/image_test.go b/internal/image/image_test.go index 7c5b170e..ba61bf5b 100644 --- a/internal/image/image_test.go +++ b/internal/image/image_test.go @@ -1,3 +1,17 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package image import ( diff --git a/internal/iostreams/reader_test.go b/internal/iostreams/reader_test.go index 5f82ce50..bef1e8ba 100644 --- a/internal/iostreams/reader_test.go +++ b/internal/iostreams/reader_test.go @@ -1,3 +1,17 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package iostreams import ( diff --git a/internal/ioutils/host_test.go b/internal/ioutils/host_test.go index 687a9e7f..fede3212 100644 --- a/internal/ioutils/host_test.go +++ b/internal/ioutils/host_test.go @@ -1,3 +1,17 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package ioutils import ( diff --git a/internal/prompts/permissions_test.go b/internal/prompts/permissions_test.go index e8cf0392..472aa52f 100644 --- a/internal/prompts/permissions_test.go +++ b/internal/prompts/permissions_test.go @@ -1,3 +1,17 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package prompts import ( From 7986821b970a5ebd25437812ea1c38618bfe741b Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 11 Mar 2026 12:54:18 -0700 Subject: [PATCH 3/9] style: use Test_Function naming convention in new test files --- internal/api/client_test.go | 4 ++-- internal/api/collaborators_test.go | 18 +++++++++--------- internal/archiveutil/archiveutil_test.go | 6 +++--- internal/cmdutil/flags_test.go | 2 +- internal/config/context_test.go | 10 +++++----- internal/goutils/recursivecopy_test.go | 8 ++++---- internal/image/image_test.go | 22 +++++++++++----------- internal/iostreams/reader_test.go | 2 +- internal/ioutils/host_test.go | 2 +- internal/prompts/permissions_test.go | 4 ++-- 10 files changed, 39 insertions(+), 39 deletions(-) diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 10190e2f..c979693d 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -20,7 +20,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestHost(t *testing.T) { +func Test_Client_Host(t *testing.T) { tests := map[string]struct { host string expected string @@ -42,7 +42,7 @@ func TestHost(t *testing.T) { } } -func TestSetHost(t *testing.T) { +func Test_Client_SetHost(t *testing.T) { tests := map[string]struct { initial string newHost string diff --git a/internal/api/collaborators_test.go b/internal/api/collaborators_test.go index cfab7e35..f04cb12e 100644 --- a/internal/api/collaborators_test.go +++ b/internal/api/collaborators_test.go @@ -22,7 +22,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestClient_AddCollaborator_Ok(t *testing.T) { +func Test_Client_AddCollaborator_Ok(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: collaboratorsAddMethod, @@ -33,7 +33,7 @@ func TestClient_AddCollaborator_Ok(t *testing.T) { require.NoError(t, err) } -func TestClient_AddCollaborator_WithUserID(t *testing.T) { +func Test_Client_AddCollaborator_WithUserID(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: collaboratorsAddMethod, @@ -44,7 +44,7 @@ func TestClient_AddCollaborator_WithUserID(t *testing.T) { require.NoError(t, err) } -func TestClient_AddCollaborator_Error(t *testing.T) { +func Test_Client_AddCollaborator_Error(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: collaboratorsAddMethod, @@ -56,7 +56,7 @@ func TestClient_AddCollaborator_Error(t *testing.T) { require.Contains(t, err.Error(), "user_not_found") } -func TestClient_ListCollaborators_Ok(t *testing.T) { +func Test_Client_ListCollaborators_Ok(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: collaboratorsListMethod, @@ -69,7 +69,7 @@ func TestClient_ListCollaborators_Ok(t *testing.T) { require.Equal(t, "U123", users[0].ID) } -func TestClient_ListCollaborators_Error(t *testing.T) { +func Test_Client_ListCollaborators_Error(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: collaboratorsListMethod, @@ -81,7 +81,7 @@ func TestClient_ListCollaborators_Error(t *testing.T) { require.Contains(t, err.Error(), "app_not_found") } -func TestClient_RemoveCollaborator_Ok(t *testing.T) { +func Test_Client_RemoveCollaborator_Ok(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: collaboratorsRemoveMethod, @@ -93,7 +93,7 @@ func TestClient_RemoveCollaborator_Ok(t *testing.T) { require.Empty(t, warnings) } -func TestClient_RemoveCollaborator_Error(t *testing.T) { +func Test_Client_RemoveCollaborator_Error(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: collaboratorsRemoveMethod, @@ -105,7 +105,7 @@ func TestClient_RemoveCollaborator_Error(t *testing.T) { require.Contains(t, err.Error(), "cannot_remove_owner") } -func TestClient_UpdateCollaborator_Ok(t *testing.T) { +func Test_Client_UpdateCollaborator_Ok(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: collaboratorsUpdateMethod, @@ -116,7 +116,7 @@ func TestClient_UpdateCollaborator_Ok(t *testing.T) { require.NoError(t, err) } -func TestClient_UpdateCollaborator_Error(t *testing.T) { +func Test_Client_UpdateCollaborator_Error(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: collaboratorsUpdateMethod, diff --git a/internal/archiveutil/archiveutil_test.go b/internal/archiveutil/archiveutil_test.go index 99b445e2..a15ec00d 100644 --- a/internal/archiveutil/archiveutil_test.go +++ b/internal/archiveutil/archiveutil_test.go @@ -27,7 +27,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestExtractAndWriteFile(t *testing.T) { +func Test_ExtractAndWriteFile(t *testing.T) { tests := map[string]struct { fileName string content string @@ -73,7 +73,7 @@ func TestExtractAndWriteFile(t *testing.T) { }) } -func TestUnzip(t *testing.T) { +func Test_Unzip(t *testing.T) { t.Run("extracts a zip archive", func(t *testing.T) { srcDir := t.TempDir() destDir := t.TempDir() @@ -108,7 +108,7 @@ func TestUnzip(t *testing.T) { }) } -func TestUntarGzip(t *testing.T) { +func Test_UntarGzip(t *testing.T) { t.Run("extracts a tar.gz archive", func(t *testing.T) { srcDir := t.TempDir() destDir := t.TempDir() diff --git a/internal/cmdutil/flags_test.go b/internal/cmdutil/flags_test.go index 90c93881..2189bc95 100644 --- a/internal/cmdutil/flags_test.go +++ b/internal/cmdutil/flags_test.go @@ -21,7 +21,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestIsFlagChanged(t *testing.T) { +func Test_IsFlagChanged(t *testing.T) { tests := map[string]struct { flag string setFlag bool diff --git a/internal/config/context_test.go b/internal/config/context_test.go index 28b47168..66629b64 100644 --- a/internal/config/context_test.go +++ b/internal/config/context_test.go @@ -21,7 +21,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestContextToken(t *testing.T) { +func Test_Context_Token(t *testing.T) { tests := map[string]struct { token string expected string @@ -47,7 +47,7 @@ func TestContextToken(t *testing.T) { }) } -func TestContextEnterpriseID(t *testing.T) { +func Test_Context_EnterpriseID(t *testing.T) { tests := map[string]struct { enterpriseID string expected string @@ -73,7 +73,7 @@ func TestContextEnterpriseID(t *testing.T) { }) } -func TestContextTeamID(t *testing.T) { +func Test_Context_TeamID(t *testing.T) { tests := map[string]struct { teamID string expected string @@ -99,7 +99,7 @@ func TestContextTeamID(t *testing.T) { }) } -func TestContextTeamDomain(t *testing.T) { +func Test_Context_TeamDomain(t *testing.T) { tests := map[string]struct { teamDomain string expected string @@ -125,7 +125,7 @@ func TestContextTeamDomain(t *testing.T) { }) } -func TestContextUserID(t *testing.T) { +func Test_Context_UserID(t *testing.T) { tests := map[string]struct { userID string expected string diff --git a/internal/goutils/recursivecopy_test.go b/internal/goutils/recursivecopy_test.go index 4ffe9364..d3a3cae0 100644 --- a/internal/goutils/recursivecopy_test.go +++ b/internal/goutils/recursivecopy_test.go @@ -23,7 +23,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestShouldIgnore(t *testing.T) { +func Test_ShouldIgnore(t *testing.T) { tests := map[string]struct { val string list []string @@ -83,7 +83,7 @@ func TestShouldIgnore(t *testing.T) { } } -func TestCopy(t *testing.T) { +func Test_Copy(t *testing.T) { t.Run("copies a regular file", func(t *testing.T) { srcDir := t.TempDir() dstDir := t.TempDir() @@ -109,7 +109,7 @@ func TestCopy(t *testing.T) { }) } -func TestCopySymLink(t *testing.T) { +func Test_CopySymLink(t *testing.T) { t.Run("copies a symbolic link", func(t *testing.T) { srcDir := t.TempDir() dstDir := t.TempDir() @@ -134,7 +134,7 @@ func TestCopySymLink(t *testing.T) { }) } -func TestCopyDirectory(t *testing.T) { +func Test_CopyDirectory(t *testing.T) { t.Run("copies directory structure", func(t *testing.T) { srcDir := t.TempDir() dstDir := filepath.Join(t.TempDir(), "dst") diff --git a/internal/image/image_test.go b/internal/image/image_test.go index ba61bf5b..a24b57ab 100644 --- a/internal/image/image_test.go +++ b/internal/image/image_test.go @@ -40,7 +40,7 @@ func createTestPNG(t *testing.T, width, height int) []byte { return buf.Bytes() } -func TestResizeImage(t *testing.T) { +func Test_ResizeImage(t *testing.T) { pngData := createTestPNG(t, 100, 100) reader := bytes.NewReader(pngData) @@ -51,7 +51,7 @@ func TestResizeImage(t *testing.T) { assert.Equal(t, 50, resized.Bounds().Dy()) } -func TestResizeImageToBytes(t *testing.T) { +func Test_ResizeImageToBytes(t *testing.T) { pngData := createTestPNG(t, 100, 100) reader := bytes.NewReader(pngData) @@ -60,7 +60,7 @@ func TestResizeImageToBytes(t *testing.T) { assert.NotEmpty(t, result) } -func TestResizeImageFromFileToBytes(t *testing.T) { +func Test_ResizeImageFromFileToBytes(t *testing.T) { pngData := createTestPNG(t, 100, 100) fs := afero.NewMemMapFs() err := afero.WriteFile(fs, "/test.png", pngData, 0644) @@ -71,13 +71,13 @@ func TestResizeImageFromFileToBytes(t *testing.T) { assert.NotEmpty(t, result) } -func TestResizeImageFromFileToBytes_FileNotFound(t *testing.T) { +func Test_ResizeImageFromFileToBytes_FileNotFound(t *testing.T) { fs := afero.NewMemMapFs() _, err := ResizeImageFromFileToBytes(fs, "/nonexistent.png", 50, 50) assert.Error(t, err) } -func TestCropResizeImageRatio(t *testing.T) { +func Test_CropResizeImageRatio(t *testing.T) { pngData := createTestPNG(t, 200, 100) reader := bytes.NewReader(pngData) @@ -87,7 +87,7 @@ func TestCropResizeImageRatio(t *testing.T) { assert.Equal(t, 100, result.Bounds().Dx()) } -func TestCropResizeImageRatioToBytes(t *testing.T) { +func Test_CropResizeImageRatioToBytes(t *testing.T) { pngData := createTestPNG(t, 200, 100) reader := bytes.NewReader(pngData) @@ -96,7 +96,7 @@ func TestCropResizeImageRatioToBytes(t *testing.T) { assert.NotEmpty(t, result) } -func TestCropResizeImageRatioFromFile(t *testing.T) { +func Test_CropResizeImageRatioFromFile(t *testing.T) { pngData := createTestPNG(t, 200, 100) fs := afero.NewMemMapFs() err := afero.WriteFile(fs, "/test.png", pngData, 0644) @@ -107,13 +107,13 @@ func TestCropResizeImageRatioFromFile(t *testing.T) { assert.NotNil(t, result) } -func TestCropResizeImageRatioFromFile_FileNotFound(t *testing.T) { +func Test_CropResizeImageRatioFromFile_FileNotFound(t *testing.T) { fs := afero.NewMemMapFs() _, err := CropResizeImageRatioFromFile(fs, "/nonexistent.png", 100, 1, 1) assert.Error(t, err) } -func TestCropResizeImageRatioFromFileToBytes(t *testing.T) { +func Test_CropResizeImageRatioFromFileToBytes(t *testing.T) { pngData := createTestPNG(t, 200, 100) fs := afero.NewMemMapFs() err := afero.WriteFile(fs, "/test.png", pngData, 0644) @@ -124,13 +124,13 @@ func TestCropResizeImageRatioFromFileToBytes(t *testing.T) { assert.NotEmpty(t, result) } -func TestCropResizeImageRatioFromFileToBytes_FileNotFound(t *testing.T) { +func Test_CropResizeImageRatioFromFileToBytes_FileNotFound(t *testing.T) { fs := afero.NewMemMapFs() _, err := CropResizeImageRatioFromFileToBytes(fs, "/nonexistent.png", 100, 1, 1) assert.Error(t, err) } -func TestResizeImage_InvalidReader(t *testing.T) { +func Test_ResizeImage_InvalidReader(t *testing.T) { reader := bytes.NewReader([]byte("not a valid image")) _, err := ResizeImage(reader, 50, 50) assert.Error(t, err) diff --git a/internal/iostreams/reader_test.go b/internal/iostreams/reader_test.go index bef1e8ba..3295f4d0 100644 --- a/internal/iostreams/reader_test.go +++ b/internal/iostreams/reader_test.go @@ -23,7 +23,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestReadIn(t *testing.T) { +func Test_ReadIn(t *testing.T) { tests := map[string]struct { input string expected string diff --git a/internal/ioutils/host_test.go b/internal/ioutils/host_test.go index fede3212..cdc4ca74 100644 --- a/internal/ioutils/host_test.go +++ b/internal/ioutils/host_test.go @@ -20,7 +20,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestGetHostname(t *testing.T) { +func Test_GetHostname(t *testing.T) { t.Run("returns a non-empty hashed hostname", func(t *testing.T) { hostname := GetHostname() assert.NotEmpty(t, hostname) diff --git a/internal/prompts/permissions_test.go b/internal/prompts/permissions_test.go index 472aa52f..c64395c9 100644 --- a/internal/prompts/permissions_test.go +++ b/internal/prompts/permissions_test.go @@ -21,7 +21,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestAccessLabels(t *testing.T) { +func Test_AccessLabels(t *testing.T) { tests := map[string]struct { current types.Permission expectedCurrent string @@ -50,7 +50,7 @@ func TestAccessLabels(t *testing.T) { } } -func TestTriggerAccessLabels(t *testing.T) { +func Test_TriggerAccessLabels(t *testing.T) { tests := map[string]struct { current types.Permission expectedCurrent string From d88ed1f0d6dfe883c7954775ee3e27151bb32b47 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 11 Mar 2026 12:58:49 -0700 Subject: [PATCH 4/9] refactor: consolidate collaborator tests into table-driven tests --- internal/api/collaborators_test.go | 238 +++++++++++++++++------------ 1 file changed, 140 insertions(+), 98 deletions(-) diff --git a/internal/api/collaborators_test.go b/internal/api/collaborators_test.go index f04cb12e..d61c307b 100644 --- a/internal/api/collaborators_test.go +++ b/internal/api/collaborators_test.go @@ -22,108 +22,150 @@ import ( "github.com/stretchr/testify/require" ) -func Test_Client_AddCollaborator_Ok(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: collaboratorsAddMethod, - Response: `{"ok":true}`, - }) - defer teardown() - err := c.AddCollaborator(ctx, "token", "A123", types.SlackUser{Email: "user@example.com", PermissionType: "owner"}) - require.NoError(t, err) +func Test_Client_AddCollaborator(t *testing.T) { + tests := map[string]struct { + response string + user types.SlackUser + expectErr string + }{ + "adds a collaborator by email": { + response: `{"ok":true}`, + user: types.SlackUser{Email: "user@example.com", PermissionType: "owner"}, + }, + "adds a collaborator by user ID": { + response: `{"ok":true}`, + user: types.SlackUser{ID: "U123", PermissionType: "owner"}, + }, + "returns error when user not found": { + response: `{"ok":false,"error":"user_not_found"}`, + user: types.SlackUser{Email: "bad@example.com"}, + expectErr: "user_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: collaboratorsAddMethod, + Response: tc.response, + }) + defer teardown() + err := c.AddCollaborator(ctx, "token", "A123", tc.user) + if tc.expectErr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectErr) + } else { + require.NoError(t, err) + } + }) + } } -func Test_Client_AddCollaborator_WithUserID(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: collaboratorsAddMethod, - Response: `{"ok":true}`, - }) - defer teardown() - err := c.AddCollaborator(ctx, "token", "A123", types.SlackUser{ID: "U123", PermissionType: "owner"}) - require.NoError(t, err) +func Test_Client_ListCollaborators(t *testing.T) { + tests := map[string]struct { + response string + expectedCount int + expectedID string + expectErr string + }{ + "returns a list of collaborators": { + response: `{"ok":true,"owners":[{"user_id":"U123","username":"Test User"}]}`, + expectedCount: 1, + expectedID: "U123", + }, + "returns error when app not found": { + response: `{"ok":false,"error":"app_not_found"}`, + expectErr: "app_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: collaboratorsListMethod, + Response: tc.response, + }) + defer teardown() + users, err := c.ListCollaborators(ctx, "token", "A123") + if tc.expectErr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectErr) + } else { + require.NoError(t, err) + require.Len(t, users, tc.expectedCount) + require.Equal(t, tc.expectedID, users[0].ID) + } + }) + } } -func Test_Client_AddCollaborator_Error(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: collaboratorsAddMethod, - Response: `{"ok":false,"error":"user_not_found"}`, - }) - defer teardown() - err := c.AddCollaborator(ctx, "token", "A123", types.SlackUser{Email: "bad@example.com"}) - require.Error(t, err) - require.Contains(t, err.Error(), "user_not_found") +func Test_Client_RemoveCollaborator(t *testing.T) { + tests := map[string]struct { + response string + user types.SlackUser + expectErr string + }{ + "removes a collaborator": { + response: `{"ok":true}`, + user: types.SlackUser{ID: "U123"}, + }, + "returns error when removing owner": { + response: `{"ok":false,"error":"cannot_remove_owner"}`, + user: types.SlackUser{Email: "owner@example.com"}, + expectErr: "cannot_remove_owner", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: collaboratorsRemoveMethod, + Response: tc.response, + }) + defer teardown() + warnings, err := c.RemoveCollaborator(ctx, "token", "A123", tc.user) + if tc.expectErr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectErr) + } else { + require.NoError(t, err) + require.Empty(t, warnings) + } + }) + } } -func Test_Client_ListCollaborators_Ok(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: collaboratorsListMethod, - Response: `{"ok":true,"owners":[{"user_id":"U123","username":"Test User"}]}`, - }) - defer teardown() - users, err := c.ListCollaborators(ctx, "token", "A123") - require.NoError(t, err) - require.Len(t, users, 1) - require.Equal(t, "U123", users[0].ID) -} - -func Test_Client_ListCollaborators_Error(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: collaboratorsListMethod, - Response: `{"ok":false,"error":"app_not_found"}`, - }) - defer teardown() - _, err := c.ListCollaborators(ctx, "token", "A123") - require.Error(t, err) - require.Contains(t, err.Error(), "app_not_found") -} - -func Test_Client_RemoveCollaborator_Ok(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: collaboratorsRemoveMethod, - Response: `{"ok":true}`, - }) - defer teardown() - warnings, err := c.RemoveCollaborator(ctx, "token", "A123", types.SlackUser{ID: "U123"}) - require.NoError(t, err) - require.Empty(t, warnings) -} - -func Test_Client_RemoveCollaborator_Error(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: collaboratorsRemoveMethod, - Response: `{"ok":false,"error":"cannot_remove_owner"}`, - }) - defer teardown() - _, err := c.RemoveCollaborator(ctx, "token", "A123", types.SlackUser{Email: "owner@example.com"}) - require.Error(t, err) - require.Contains(t, err.Error(), "cannot_remove_owner") -} - -func Test_Client_UpdateCollaborator_Ok(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: collaboratorsUpdateMethod, - Response: `{"ok":true}`, - }) - defer teardown() - err := c.UpdateCollaborator(ctx, "token", "A123", types.SlackUser{ID: "U123", PermissionType: "collaborator"}) - require.NoError(t, err) -} - -func Test_Client_UpdateCollaborator_Error(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - c, teardown := NewFakeClient(t, FakeClientParams{ - ExpectedMethod: collaboratorsUpdateMethod, - Response: `{"ok":false,"error":"invalid_permission"}`, - }) - defer teardown() - err := c.UpdateCollaborator(ctx, "token", "A123", types.SlackUser{ID: "U123", PermissionType: "invalid"}) - require.Error(t, err) - require.Contains(t, err.Error(), "invalid_permission") +func Test_Client_UpdateCollaborator(t *testing.T) { + tests := map[string]struct { + response string + user types.SlackUser + expectErr string + }{ + "updates a collaborator": { + response: `{"ok":true}`, + user: types.SlackUser{ID: "U123", PermissionType: "collaborator"}, + }, + "returns error for invalid permission": { + response: `{"ok":false,"error":"invalid_permission"}`, + user: types.SlackUser{ID: "U123", PermissionType: "invalid"}, + expectErr: "invalid_permission", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: collaboratorsUpdateMethod, + Response: tc.response, + }) + defer teardown() + err := c.UpdateCollaborator(ctx, "token", "A123", tc.user) + if tc.expectErr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectErr) + } else { + require.NoError(t, err) + } + }) + } } From e3340c9946f09da822fcf471be46a269a1f8daca Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 11 Mar 2026 13:06:30 -0700 Subject: [PATCH 5/9] chore: add TODO comments for afero.Fs refactor in archiveutil tests --- internal/archiveutil/archiveutil_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/archiveutil/archiveutil_test.go b/internal/archiveutil/archiveutil_test.go index a15ec00d..a37983d0 100644 --- a/internal/archiveutil/archiveutil_test.go +++ b/internal/archiveutil/archiveutil_test.go @@ -27,6 +27,7 @@ import ( "github.com/stretchr/testify/require" ) +// TODO: Refactor to use afero.Fs once ExtractAndWriteFile accepts it. Currently uses t.TempDir() which is safe. func Test_ExtractAndWriteFile(t *testing.T) { tests := map[string]struct { fileName string @@ -73,6 +74,7 @@ func Test_ExtractAndWriteFile(t *testing.T) { }) } +// TODO: Refactor to use afero.Fs once Unzip accepts it. Currently uses t.TempDir() which is safe. func Test_Unzip(t *testing.T) { t.Run("extracts a zip archive", func(t *testing.T) { srcDir := t.TempDir() @@ -108,6 +110,7 @@ func Test_Unzip(t *testing.T) { }) } +// TODO: Refactor to use afero.Fs once UntarGzip accepts it. Currently uses t.TempDir() which is safe. func Test_UntarGzip(t *testing.T) { t.Run("extracts a tar.gz archive", func(t *testing.T) { srcDir := t.TempDir() From 57362fbdcc117b3e66cd61e6aaae81c0d38fe2f8 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 11 Mar 2026 13:11:42 -0700 Subject: [PATCH 6/9] test: fix casing of README.md in ShouldIgnore test data --- internal/goutils/recursivecopy_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/goutils/recursivecopy_test.go b/internal/goutils/recursivecopy_test.go index d3a3cae0..b14d6bfa 100644 --- a/internal/goutils/recursivecopy_test.go +++ b/internal/goutils/recursivecopy_test.go @@ -59,7 +59,7 @@ func Test_ShouldIgnore(t *testing.T) { expected: true, }, "returns false when ignoreFunc returns false": { - val: "readme.md", + val: "README.md", list: []string{}, ignoreFunc: func(s string) bool { return s == "secret.txt" From 0be8c062c39ca16518353868bbd27f47a839297d Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 11 Mar 2026 13:15:21 -0700 Subject: [PATCH 7/9] chore: add TODO comments for afero.Fs refactor in recursivecopy tests --- internal/goutils/recursivecopy_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/goutils/recursivecopy_test.go b/internal/goutils/recursivecopy_test.go index b14d6bfa..f98f423c 100644 --- a/internal/goutils/recursivecopy_test.go +++ b/internal/goutils/recursivecopy_test.go @@ -83,6 +83,7 @@ func Test_ShouldIgnore(t *testing.T) { } } +// TODO: Refactor to use afero.Fs once Copy accepts it. Currently uses t.TempDir() which is safe. func Test_Copy(t *testing.T) { t.Run("copies a regular file", func(t *testing.T) { srcDir := t.TempDir() @@ -109,6 +110,7 @@ func Test_Copy(t *testing.T) { }) } +// TODO: Refactor to use afero.Fs once CopySymLink accepts it. Currently uses t.TempDir() which is safe. func Test_CopySymLink(t *testing.T) { t.Run("copies a symbolic link", func(t *testing.T) { srcDir := t.TempDir() @@ -134,6 +136,7 @@ func Test_CopySymLink(t *testing.T) { }) } +// TODO: Refactor to use afero.Fs once CopyDirectory accepts it. Currently uses t.TempDir() which is safe. func Test_CopyDirectory(t *testing.T) { t.Run("copies directory structure", func(t *testing.T) { srcDir := t.TempDir() From 8bee097bf21f18a55d2996a20c5fec3aa1f1cc02 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 11 Mar 2026 13:26:05 -0700 Subject: [PATCH 8/9] refactor: use slackdeps.NewFsMock() in image tests --- internal/image/image_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/internal/image/image_test.go b/internal/image/image_test.go index a24b57ab..10712e91 100644 --- a/internal/image/image_test.go +++ b/internal/image/image_test.go @@ -21,6 +21,7 @@ import ( "image/png" "testing" + "github.com/slackapi/slack-cli/internal/slackdeps" "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -62,7 +63,7 @@ func Test_ResizeImageToBytes(t *testing.T) { func Test_ResizeImageFromFileToBytes(t *testing.T) { pngData := createTestPNG(t, 100, 100) - fs := afero.NewMemMapFs() + fs := slackdeps.NewFsMock() err := afero.WriteFile(fs, "/test.png", pngData, 0644) require.NoError(t, err) @@ -72,7 +73,7 @@ func Test_ResizeImageFromFileToBytes(t *testing.T) { } func Test_ResizeImageFromFileToBytes_FileNotFound(t *testing.T) { - fs := afero.NewMemMapFs() + fs := slackdeps.NewFsMock() _, err := ResizeImageFromFileToBytes(fs, "/nonexistent.png", 50, 50) assert.Error(t, err) } @@ -98,7 +99,7 @@ func Test_CropResizeImageRatioToBytes(t *testing.T) { func Test_CropResizeImageRatioFromFile(t *testing.T) { pngData := createTestPNG(t, 200, 100) - fs := afero.NewMemMapFs() + fs := slackdeps.NewFsMock() err := afero.WriteFile(fs, "/test.png", pngData, 0644) require.NoError(t, err) @@ -108,14 +109,14 @@ func Test_CropResizeImageRatioFromFile(t *testing.T) { } func Test_CropResizeImageRatioFromFile_FileNotFound(t *testing.T) { - fs := afero.NewMemMapFs() + fs := slackdeps.NewFsMock() _, err := CropResizeImageRatioFromFile(fs, "/nonexistent.png", 100, 1, 1) assert.Error(t, err) } func Test_CropResizeImageRatioFromFileToBytes(t *testing.T) { pngData := createTestPNG(t, 200, 100) - fs := afero.NewMemMapFs() + fs := slackdeps.NewFsMock() err := afero.WriteFile(fs, "/test.png", pngData, 0644) require.NoError(t, err) @@ -125,7 +126,7 @@ func Test_CropResizeImageRatioFromFileToBytes(t *testing.T) { } func Test_CropResizeImageRatioFromFileToBytes_FileNotFound(t *testing.T) { - fs := afero.NewMemMapFs() + fs := slackdeps.NewFsMock() _, err := CropResizeImageRatioFromFileToBytes(fs, "/nonexistent.png", 100, 1, 1) assert.Error(t, err) } From 758ce76b60b23ec29fff0bebdc5a2f9f2ca99672 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 11 Mar 2026 19:24:40 -0700 Subject: [PATCH 9/9] test: fix GetHostname() test for unknown --- internal/ioutils/host_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ioutils/host_test.go b/internal/ioutils/host_test.go index cdc4ca74..0c7c3dc7 100644 --- a/internal/ioutils/host_test.go +++ b/internal/ioutils/host_test.go @@ -26,7 +26,7 @@ func Test_GetHostname(t *testing.T) { assert.NotEmpty(t, hostname) // The hostname should be hashed, not the raw hostname // It should not be "unknown" on a normal system - assert.NotEqual(t, "", hostname) + assert.NotEqual(t, "unknown", hostname) }) t.Run("returns consistent results", func(t *testing.T) {