From 5fee7f6f7cfb171467873db16838f76725547bb4 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Thu, 12 Mar 2026 09:24:46 -0700 Subject: [PATCH 1/6] test: increase test coverage for iostreams --- internal/iostreams/iostreams_test.go | 43 ++++++ internal/iostreams/survey_test.go | 208 +++++++++++++++++++++++++++ internal/iostreams/writer_test.go | 18 +++ 3 files changed, 269 insertions(+) diff --git a/internal/iostreams/iostreams_test.go b/internal/iostreams/iostreams_test.go index 55c752d3..9bb52c3d 100644 --- a/internal/iostreams/iostreams_test.go +++ b/internal/iostreams/iostreams_test.go @@ -20,6 +20,7 @@ import ( "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/slackdeps" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -34,6 +35,36 @@ func Test_IOSteams_NewIOStreams(t *testing.T) { require.True(t, io.config.DebugEnabled, "iostreams references config") } +func Test_IOStreams_ExitCode(t *testing.T) { + tests := map[string]struct { + setCode ExitCode + expected ExitCode + }{ + "default is ExitOK": { + setCode: ExitOK, + expected: ExitOK, + }, + "set to ExitError": { + setCode: ExitError, + expected: ExitError, + }, + "set to ExitCancel": { + setCode: ExitCancel, + expected: ExitCancel, + }, + } + 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.SetExitCode(tc.setCode) + assert.Equal(t, tc.expected, io.GetExitCode()) + }) + } +} + func Test_IOStreams_IsTTY(t *testing.T) { tests := map[string]struct { fileInfo os.FileInfo @@ -65,3 +96,15 @@ func Test_IOStreams_IsTTY(t *testing.T) { }) } } + +func Test_SetCmdIO(t *testing.T) { + fsMock := slackdeps.NewFsMock() + osMock := slackdeps.NewOsMock() + cfg := config.NewConfig(fsMock, osMock) + io := NewIOStreams(cfg, fsMock, osMock) + cmd := &cobra.Command{Use: "test"} + io.SetCmdIO(cmd) + assert.NotNil(t, cmd.InOrStdin()) + assert.NotNil(t, cmd.OutOrStdout()) + assert.NotNil(t, cmd.ErrOrStderr()) +} diff --git a/internal/iostreams/survey_test.go b/internal/iostreams/survey_test.go index c874b598..a33f8441 100644 --- a/internal/iostreams/survey_test.go +++ b/internal/iostreams/survey_test.go @@ -26,6 +26,214 @@ import ( "github.com/stretchr/testify/assert" ) +func TestConfirmPromptConfig(t *testing.T) { + tests := map[string]struct { + cfg ConfirmPromptConfig + required bool + }{ + "required true": { + cfg: ConfirmPromptConfig{Required: true}, + required: true, + }, + "required false": { + cfg: ConfirmPromptConfig{Required: false}, + required: false, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.required, tc.cfg.IsRequired()) + assert.Empty(t, tc.cfg.GetFlags()) + }) + } +} + +func TestInputPromptConfig(t *testing.T) { + tests := map[string]struct { + cfg InputPromptConfig + required bool + }{ + "required true": { + cfg: InputPromptConfig{Required: true, Placeholder: "hint"}, + required: true, + }, + "required false": { + cfg: InputPromptConfig{Required: false}, + required: false, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.required, tc.cfg.IsRequired()) + assert.Empty(t, tc.cfg.GetFlags()) + }) + } +} + +func TestMultiSelectPromptConfig(t *testing.T) { + tests := map[string]struct { + cfg MultiSelectPromptConfig + required bool + }{ + "required true": { + cfg: MultiSelectPromptConfig{Required: true}, + required: true, + }, + "required false": { + cfg: MultiSelectPromptConfig{Required: false}, + required: false, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.required, tc.cfg.IsRequired()) + assert.Empty(t, tc.cfg.GetFlags()) + }) + } +} + +func TestPasswordPromptConfig(t *testing.T) { + t.Run("without flag", func(t *testing.T) { + cfg := PasswordPromptConfig{Required: true} + assert.True(t, cfg.IsRequired()) + assert.Empty(t, cfg.GetFlags()) + }) + t.Run("with flag", func(t *testing.T) { + var val string + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + fs.StringVar(&val, "token", "", "token flag") + flag := fs.Lookup("token") + cfg := PasswordPromptConfig{Required: false, Flag: flag} + assert.False(t, cfg.IsRequired()) + assert.Len(t, cfg.GetFlags(), 1) + assert.Equal(t, "token", cfg.GetFlags()[0].Name) + }) +} + +func TestSelectPromptConfig(t *testing.T) { + t.Run("no flags", func(t *testing.T) { + cfg := SelectPromptConfig{Required: true} + assert.True(t, cfg.IsRequired()) + assert.Empty(t, cfg.GetFlags()) + }) + t.Run("single flag", func(t *testing.T) { + var val string + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + fs.StringVar(&val, "app", "", "app flag") + flag := fs.Lookup("app") + cfg := SelectPromptConfig{Flag: flag} + assert.Len(t, cfg.GetFlags(), 1) + }) + t.Run("multiple flags", func(t *testing.T) { + var v1, v2 string + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + fs.StringVar(&v1, "a", "", "") + fs.StringVar(&v2, "b", "", "") + cfg := SelectPromptConfig{Flags: []*pflag.Flag{fs.Lookup("a"), fs.Lookup("b")}} + assert.Len(t, cfg.GetFlags(), 2) + }) +} + +func TestSurveyOptions(t *testing.T) { + tests := map[string]struct { + cfg PromptConfig + expectedLen int + }{ + "required config returns 5 options": { + cfg: ConfirmPromptConfig{Required: true}, + expectedLen: 5, + }, + "optional config returns 5 options": { + cfg: ConfirmPromptConfig{Required: false}, + expectedLen: 5, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + opts := SurveyOptions(tc.cfg) + assert.Len(t, opts, tc.expectedLen) + }) + } +} + +func TestDefaultSelectPromptConfig(t *testing.T) { + cfg := DefaultSelectPromptConfig() + assert.True(t, cfg.IsRequired()) + assert.Empty(t, cfg.GetFlags()) +} + +func TestRetrieveFlagValue(t *testing.T) { + fsMock := slackdeps.NewFsMock() + osMock := slackdeps.NewOsMock() + cfg := config.NewConfig(fsMock, osMock) + io := NewIOStreams(cfg, fsMock, osMock) + + tests := map[string]struct { + flagset []*pflag.Flag + expectedFlag bool + expectedError string + }{ + "nil flagset returns nil": { + flagset: nil, + expectedFlag: false, + }, + "no changed flags returns nil": { + flagset: func() []*pflag.Flag { + var v string + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + fs.StringVar(&v, "x", "", "") + return []*pflag.Flag{fs.Lookup("x")} + }(), + expectedFlag: false, + }, + "one changed flag returns it": { + flagset: func() []*pflag.Flag { + var v string + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + fs.StringVar(&v, "y", "", "") + f := fs.Lookup("y") + f.Changed = true + return []*pflag.Flag{f} + }(), + expectedFlag: true, + }, + "two changed flags returns error": { + flagset: func() []*pflag.Flag { + var v1, v2 string + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + fs.StringVar(&v1, "a", "", "") + fs.StringVar(&v2, "b", "", "") + fa := fs.Lookup("a") + fa.Changed = true + fb := fs.Lookup("b") + fb.Changed = true + return []*pflag.Flag{fa, fb} + }(), + expectedError: slackerror.ErrMismatchedFlags, + }, + "nil flag in set is skipped": { + flagset: []*pflag.Flag{nil}, + expectedFlag: false, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + flag, err := io.retrieveFlagValue(tc.flagset) + if tc.expectedError != "" { + assert.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedError) + } else { + assert.NoError(t, err) + if tc.expectedFlag { + assert.NotNil(t, flag) + } else { + assert.Nil(t, flag) + } + } + }) + } +} + func TestPasswordPrompt(t *testing.T) { tests := map[string]struct { FlagChanged bool diff --git a/internal/iostreams/writer_test.go b/internal/iostreams/writer_test.go index d9411b30..9921bec0 100644 --- a/internal/iostreams/writer_test.go +++ b/internal/iostreams/writer_test.go @@ -233,6 +233,24 @@ func Test_WriteIndent(t *testing.T) { } } +func Test_WriteOut(t *testing.T) { + fsMock := slackdeps.NewFsMock() + osMock := slackdeps.NewOsMock() + cfg := config.NewConfig(fsMock, osMock) + io := NewIOStreams(cfg, fsMock, osMock) + w := io.WriteOut() + require.NotNil(t, w) +} + +func Test_WriteErr(t *testing.T) { + fsMock := slackdeps.NewFsMock() + osMock := slackdeps.NewOsMock() + cfg := config.NewConfig(fsMock, osMock) + io := NewIOStreams(cfg, fsMock, osMock) + w := io.WriteErr() + require.NotNil(t, w) +} + func Test_WriteSecondary(t *testing.T) { tests := map[string]struct { input string From 618897972930e2631c32dbbcd72575088a6a15bc Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Thu, 12 Mar 2026 13:28:04 -0700 Subject: [PATCH 2/6] test: rename test functions to use Test_FunctionName convention Rename 12 test functions to follow the Test_StructName_FunctionName and Test_FunctionName naming convention. Document the convention in MAINTAINERS_GUIDE.md and CLAUDE.md. --- .claude/CLAUDE.md | 9 +++++++++ .github/MAINTAINERS_GUIDE.md | 14 ++++++++++++++ internal/iostreams/survey_test.go | 16 ++++++++-------- internal/style/format_test.go | 8 ++++---- 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 14a63f2e..dec8de84 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -135,6 +135,15 @@ func NewExampleCommand(clients *shared.ClientFactory) *cobra.Command { - Mock the `ClientFactory` and its dependencies for testing - Always mock file system operations using `afero.Fs` to enable testability +### Test Naming Conventions + +Test function names use the format `Test_StructName_FunctionName` for methods on a struct, or `Test_FunctionName` for package-level functions: + +```go +func Test_Client_GetAppStatus(t *testing.T) { ... } // struct method +func Test_getKeyLength(t *testing.T) { ... } // package-level function +``` + ### Table-Driven Test Conventions **Preferred: Map pattern** - uses `tc` for test case variable: diff --git a/.github/MAINTAINERS_GUIDE.md b/.github/MAINTAINERS_GUIDE.md index 20830fc0..259a5f2f 100644 --- a/.github/MAINTAINERS_GUIDE.md +++ b/.github/MAINTAINERS_GUIDE.md @@ -383,6 +383,20 @@ The branch name can also be set by changing for the `build-lint-test-e2e-test` workflow in the `.circleci/config.yml` file, but take care not to merge this change into `main`! +#### Test naming conventions + +Test function names should use the format `Test_StructName_FunctionName` for methods +on a struct, or `Test_FunctionName` for package-level functions. The underscore after +`Test` separates the Go test prefix from the identifier being tested: + +```go +// Testing a method on a struct +func Test_Client_GetAppStatus(t *testing.T) { ... } + +// Testing a package-level function +func Test_getKeyLength(t *testing.T) { ... } +``` + #### Contributing tests If you'd like to add tests, please review our diff --git a/internal/iostreams/survey_test.go b/internal/iostreams/survey_test.go index a33f8441..8061b03e 100644 --- a/internal/iostreams/survey_test.go +++ b/internal/iostreams/survey_test.go @@ -26,7 +26,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestConfirmPromptConfig(t *testing.T) { +func Test_ConfirmPromptConfig(t *testing.T) { tests := map[string]struct { cfg ConfirmPromptConfig required bool @@ -48,7 +48,7 @@ func TestConfirmPromptConfig(t *testing.T) { } } -func TestInputPromptConfig(t *testing.T) { +func Test_InputPromptConfig(t *testing.T) { tests := map[string]struct { cfg InputPromptConfig required bool @@ -70,7 +70,7 @@ func TestInputPromptConfig(t *testing.T) { } } -func TestMultiSelectPromptConfig(t *testing.T) { +func Test_MultiSelectPromptConfig(t *testing.T) { tests := map[string]struct { cfg MultiSelectPromptConfig required bool @@ -92,7 +92,7 @@ func TestMultiSelectPromptConfig(t *testing.T) { } } -func TestPasswordPromptConfig(t *testing.T) { +func Test_PasswordPromptConfig(t *testing.T) { t.Run("without flag", func(t *testing.T) { cfg := PasswordPromptConfig{Required: true} assert.True(t, cfg.IsRequired()) @@ -110,7 +110,7 @@ func TestPasswordPromptConfig(t *testing.T) { }) } -func TestSelectPromptConfig(t *testing.T) { +func Test_SelectPromptConfig(t *testing.T) { t.Run("no flags", func(t *testing.T) { cfg := SelectPromptConfig{Required: true} assert.True(t, cfg.IsRequired()) @@ -134,7 +134,7 @@ func TestSelectPromptConfig(t *testing.T) { }) } -func TestSurveyOptions(t *testing.T) { +func Test_SurveyOptions(t *testing.T) { tests := map[string]struct { cfg PromptConfig expectedLen int @@ -156,13 +156,13 @@ func TestSurveyOptions(t *testing.T) { } } -func TestDefaultSelectPromptConfig(t *testing.T) { +func Test_DefaultSelectPromptConfig(t *testing.T) { cfg := DefaultSelectPromptConfig() assert.True(t, cfg.IsRequired()) assert.Empty(t, cfg.GetFlags()) } -func TestRetrieveFlagValue(t *testing.T) { +func Test_retrieveFlagValue(t *testing.T) { fsMock := slackdeps.NewFsMock() osMock := slackdeps.NewOsMock() cfg := config.NewConfig(fsMock, osMock) diff --git a/internal/style/format_test.go b/internal/style/format_test.go index 41504e9e..c42e39f7 100644 --- a/internal/style/format_test.go +++ b/internal/style/format_test.go @@ -24,7 +24,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestGetKeyLength(t *testing.T) { +func Test_getKeyLength(t *testing.T) { tests := map[string]struct { keys map[string]string expected int @@ -53,7 +53,7 @@ func TestGetKeyLength(t *testing.T) { } } -func TestSectionf(t *testing.T) { +func Test_Sectionf(t *testing.T) { tests := map[string]struct { section TextSection expected string @@ -82,7 +82,7 @@ func TestSectionHeaderfEmpty(t *testing.T) { assert.Equal(t, "", SectionHeaderf("tada", "")) } -func TestSectionSecondaryf(t *testing.T) { +func Test_SectionSecondaryf(t *testing.T) { tests := map[string]struct { format string args []interface{} @@ -130,7 +130,7 @@ func TestSectionSecondaryf(t *testing.T) { } } -func TestCommandf(t *testing.T) { +func Test_Commandf(t *testing.T) { tests := map[string]struct { process string command string From fc34422c7c3757c0d22f1c75db738c05a251d7da Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Thu, 12 Mar 2026 13:41:36 -0700 Subject: [PATCH 3/6] test: order test functions alphabetically and document convention Reorder test functions added on this branch alphabetically within their files, grouping getter/setter tests under the base name. Document the test ordering convention in MAINTAINERS_GUIDE.md and CLAUDE.md. --- .claude/CLAUDE.md | 4 ++ .github/MAINTAINERS_GUIDE.md | 10 +++ internal/iostreams/iostreams_test.go | 22 +++--- internal/iostreams/survey_test.go | 104 +++++++++++++-------------- internal/iostreams/writer_test.go | 18 ++--- internal/style/format_test.go | 92 ++++++++++++------------ 6 files changed, 132 insertions(+), 118 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index dec8de84..2ac1d37b 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -144,6 +144,10 @@ func Test_Client_GetAppStatus(t *testing.T) { ... } // struct method func Test_getKeyLength(t *testing.T) { ... } // package-level function ``` +### Test Ordering Conventions + +Test functions should be ordered alphabetically within each file. When a file has logical sections (separated by comments), tests should be alphabetical within each section. Getter and setter functions are grouped together under the base name — ignore the `Get` or `Set` prefix when determining order (e.g. `Test_AppName` and `Test_SetAppName` both sort under `A`). + ### Table-Driven Test Conventions **Preferred: Map pattern** - uses `tc` for test case variable: diff --git a/.github/MAINTAINERS_GUIDE.md b/.github/MAINTAINERS_GUIDE.md index 259a5f2f..02700bdd 100644 --- a/.github/MAINTAINERS_GUIDE.md +++ b/.github/MAINTAINERS_GUIDE.md @@ -397,6 +397,16 @@ func Test_Client_GetAppStatus(t *testing.T) { ... } func Test_getKeyLength(t *testing.T) { ... } ``` +#### Test ordering conventions + +Test functions should be ordered alphabetically within each file. When a file has +logical sections (separated by comments), tests should be alphabetical within each +section. + +Getter and setter functions should be grouped together under the base name. Ignore +the `Get` or `Set` prefix when determining alphabetical order. For example, +`Test_AppName` and `Test_SetAppName` are both sorted under `A` for `AppName`. + #### Contributing tests If you'd like to add tests, please review our diff --git a/internal/iostreams/iostreams_test.go b/internal/iostreams/iostreams_test.go index 9bb52c3d..f508bde4 100644 --- a/internal/iostreams/iostreams_test.go +++ b/internal/iostreams/iostreams_test.go @@ -25,16 +25,6 @@ import ( "github.com/stretchr/testify/require" ) -func Test_IOSteams_NewIOStreams(t *testing.T) { - var io *IOStreams - fsMock := slackdeps.NewFsMock() - osMock := slackdeps.NewOsMock() - config := config.NewConfig(fsMock, osMock) - config.DebugEnabled = true - io = NewIOStreams(config, fsMock, osMock) - require.True(t, io.config.DebugEnabled, "iostreams references config") -} - func Test_IOStreams_ExitCode(t *testing.T) { tests := map[string]struct { setCode ExitCode @@ -65,6 +55,16 @@ func Test_IOStreams_ExitCode(t *testing.T) { } } +func Test_IOSteams_NewIOStreams(t *testing.T) { + var io *IOStreams + fsMock := slackdeps.NewFsMock() + osMock := slackdeps.NewOsMock() + config := config.NewConfig(fsMock, osMock) + config.DebugEnabled = true + io = NewIOStreams(config, fsMock, osMock) + require.True(t, io.config.DebugEnabled, "iostreams references config") +} + func Test_IOStreams_IsTTY(t *testing.T) { tests := map[string]struct { fileInfo os.FileInfo @@ -97,7 +97,7 @@ func Test_IOStreams_IsTTY(t *testing.T) { } } -func Test_SetCmdIO(t *testing.T) { +func Test_IOStreams_SetCmdIO(t *testing.T) { fsMock := slackdeps.NewFsMock() osMock := slackdeps.NewOsMock() cfg := config.NewConfig(fsMock, osMock) diff --git a/internal/iostreams/survey_test.go b/internal/iostreams/survey_test.go index 8061b03e..45398610 100644 --- a/internal/iostreams/survey_test.go +++ b/internal/iostreams/survey_test.go @@ -48,6 +48,12 @@ func Test_ConfirmPromptConfig(t *testing.T) { } } +func Test_DefaultSelectPromptConfig(t *testing.T) { + cfg := DefaultSelectPromptConfig() + assert.True(t, cfg.IsRequired()) + assert.Empty(t, cfg.GetFlags()) +} + func Test_InputPromptConfig(t *testing.T) { tests := map[string]struct { cfg InputPromptConfig @@ -110,58 +116,6 @@ func Test_PasswordPromptConfig(t *testing.T) { }) } -func Test_SelectPromptConfig(t *testing.T) { - t.Run("no flags", func(t *testing.T) { - cfg := SelectPromptConfig{Required: true} - assert.True(t, cfg.IsRequired()) - assert.Empty(t, cfg.GetFlags()) - }) - t.Run("single flag", func(t *testing.T) { - var val string - fs := pflag.NewFlagSet("test", pflag.ContinueOnError) - fs.StringVar(&val, "app", "", "app flag") - flag := fs.Lookup("app") - cfg := SelectPromptConfig{Flag: flag} - assert.Len(t, cfg.GetFlags(), 1) - }) - t.Run("multiple flags", func(t *testing.T) { - var v1, v2 string - fs := pflag.NewFlagSet("test", pflag.ContinueOnError) - fs.StringVar(&v1, "a", "", "") - fs.StringVar(&v2, "b", "", "") - cfg := SelectPromptConfig{Flags: []*pflag.Flag{fs.Lookup("a"), fs.Lookup("b")}} - assert.Len(t, cfg.GetFlags(), 2) - }) -} - -func Test_SurveyOptions(t *testing.T) { - tests := map[string]struct { - cfg PromptConfig - expectedLen int - }{ - "required config returns 5 options": { - cfg: ConfirmPromptConfig{Required: true}, - expectedLen: 5, - }, - "optional config returns 5 options": { - cfg: ConfirmPromptConfig{Required: false}, - expectedLen: 5, - }, - } - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - opts := SurveyOptions(tc.cfg) - assert.Len(t, opts, tc.expectedLen) - }) - } -} - -func Test_DefaultSelectPromptConfig(t *testing.T) { - cfg := DefaultSelectPromptConfig() - assert.True(t, cfg.IsRequired()) - assert.Empty(t, cfg.GetFlags()) -} - func Test_retrieveFlagValue(t *testing.T) { fsMock := slackdeps.NewFsMock() osMock := slackdeps.NewOsMock() @@ -234,6 +188,52 @@ func Test_retrieveFlagValue(t *testing.T) { } } +func Test_SelectPromptConfig(t *testing.T) { + t.Run("no flags", func(t *testing.T) { + cfg := SelectPromptConfig{Required: true} + assert.True(t, cfg.IsRequired()) + assert.Empty(t, cfg.GetFlags()) + }) + t.Run("single flag", func(t *testing.T) { + var val string + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + fs.StringVar(&val, "app", "", "app flag") + flag := fs.Lookup("app") + cfg := SelectPromptConfig{Flag: flag} + assert.Len(t, cfg.GetFlags(), 1) + }) + t.Run("multiple flags", func(t *testing.T) { + var v1, v2 string + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + fs.StringVar(&v1, "a", "", "") + fs.StringVar(&v2, "b", "", "") + cfg := SelectPromptConfig{Flags: []*pflag.Flag{fs.Lookup("a"), fs.Lookup("b")}} + assert.Len(t, cfg.GetFlags(), 2) + }) +} + +func Test_SurveyOptions(t *testing.T) { + tests := map[string]struct { + cfg PromptConfig + expectedLen int + }{ + "required config returns 5 options": { + cfg: ConfirmPromptConfig{Required: true}, + expectedLen: 5, + }, + "optional config returns 5 options": { + cfg: ConfirmPromptConfig{Required: false}, + expectedLen: 5, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + opts := SurveyOptions(tc.cfg) + assert.Len(t, opts, tc.expectedLen) + }) + } +} + func TestPasswordPrompt(t *testing.T) { tests := map[string]struct { FlagChanged bool diff --git a/internal/iostreams/writer_test.go b/internal/iostreams/writer_test.go index 9921bec0..6f165b68 100644 --- a/internal/iostreams/writer_test.go +++ b/internal/iostreams/writer_test.go @@ -199,6 +199,15 @@ func Test_FilteredWriter(t *testing.T) { } } +func Test_WriteErr(t *testing.T) { + fsMock := slackdeps.NewFsMock() + osMock := slackdeps.NewOsMock() + cfg := config.NewConfig(fsMock, osMock) + io := NewIOStreams(cfg, fsMock, osMock) + w := io.WriteErr() + require.NotNil(t, w) +} + func Test_WriteIndent(t *testing.T) { tests := map[string]struct { input string @@ -242,15 +251,6 @@ func Test_WriteOut(t *testing.T) { require.NotNil(t, w) } -func Test_WriteErr(t *testing.T) { - fsMock := slackdeps.NewFsMock() - osMock := slackdeps.NewOsMock() - cfg := config.NewConfig(fsMock, osMock) - io := NewIOStreams(cfg, fsMock, osMock) - w := io.WriteErr() - require.NotNil(t, w) -} - func Test_WriteSecondary(t *testing.T) { tests := map[string]struct { input string diff --git a/internal/style/format_test.go b/internal/style/format_test.go index c42e39f7..d945eff4 100644 --- a/internal/style/format_test.go +++ b/internal/style/format_test.go @@ -24,6 +24,35 @@ import ( "github.com/stretchr/testify/assert" ) +func Test_Commandf(t *testing.T) { + tests := map[string]struct { + process string + command string + isPrimary bool + }{ + "primary command contains process and command": { + process: "renamed-slack-command", + command: "feedback", + isPrimary: true, + }, + "secondary command contains process and command": { + process: "a-renamed-slack-cli", + command: "feedback", + isPrimary: false, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + processTemp := os.Args[0] + os.Args[0] = tc.process + defer func() { os.Args[0] = processTemp }() + + formatted := Commandf(tc.command, tc.isPrimary) + assert.Contains(t, formatted, tc.process+" "+tc.command) + }) + } +} + func Test_getKeyLength(t *testing.T) { tests := map[string]struct { keys map[string]string @@ -53,6 +82,12 @@ func Test_getKeyLength(t *testing.T) { } } +func TestIndent(t *testing.T) { + text := "a few spaces are expected at the start of this line, but no other changes" + indented := Indent(text) + assert.Contains(t, indented, text) +} + func Test_Sectionf(t *testing.T) { tests := map[string]struct { section TextSection @@ -130,41 +165,29 @@ func Test_SectionSecondaryf(t *testing.T) { } } -func Test_Commandf(t *testing.T) { +func TestSurveyIcons(t *testing.T) { tests := map[string]struct { - process string - command string - isPrimary bool + styleEnabled bool }{ - "primary command contains process and command": { - process: "renamed-slack-command", - command: "feedback", - isPrimary: true, + "styles are not enabled": { + styleEnabled: false, }, - "secondary command contains process and command": { - process: "a-renamed-slack-cli", - command: "feedback", - isPrimary: false, + "styles are enabled": { + styleEnabled: true, }, } + for name, tc := range tests { t.Run(name, func(t *testing.T) { - processTemp := os.Args[0] - os.Args[0] = tc.process - defer func() { os.Args[0] = processTemp }() + core.DisableColor = false + isStyleEnabled = tc.styleEnabled - formatted := Commandf(tc.command, tc.isPrimary) - assert.Contains(t, formatted, tc.process+" "+tc.command) + _ = SurveyIcons() + assert.NotEqual(t, tc.styleEnabled, core.DisableColor) }) } } -func TestIndent(t *testing.T) { - text := "a few spaces are expected at the start of this line, but no other changes" - indented := Indent(text) - assert.Contains(t, indented, text) -} - func TestTracef(t *testing.T) { tests := map[string]struct { traceID string @@ -199,29 +222,6 @@ func TestTracef(t *testing.T) { } } -func TestSurveyIcons(t *testing.T) { - tests := map[string]struct { - styleEnabled bool - }{ - "styles are not enabled": { - styleEnabled: false, - }, - "styles are enabled": { - styleEnabled: true, - }, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - core.DisableColor = false - isStyleEnabled = tc.styleEnabled - - _ = SurveyIcons() - assert.NotEqual(t, tc.styleEnabled, core.DisableColor) - }) - } -} - /* * Example commands */ From 4e0ebd1fe02ebf3e07a924c76cdbadb4ba4571e8 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Thu, 12 Mar 2026 14:38:52 -0700 Subject: [PATCH 4/6] docs: fix whitespace alignment in CLAUDE.md test naming example --- .claude/CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 2ac1d37b..fe345cc6 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -141,7 +141,7 @@ Test function names use the format `Test_StructName_FunctionName` for methods on ```go func Test_Client_GetAppStatus(t *testing.T) { ... } // struct method -func Test_getKeyLength(t *testing.T) { ... } // package-level function +func Test_getKeyLength(t *testing.T) { ... } // package-level function ``` ### Test Ordering Conventions From afa07295d2a1b96d99992b86f26e6b5df884d2ae Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Thu, 12 Mar 2026 14:43:30 -0700 Subject: [PATCH 5/6] test: apply Test_StructName_FunctionName format and reorder alphabetically --- internal/iostreams/survey_test.go | 82 +++++++++++++++---------------- internal/iostreams/writer_test.go | 20 ++++---- 2 files changed, 51 insertions(+), 51 deletions(-) diff --git a/internal/iostreams/survey_test.go b/internal/iostreams/survey_test.go index 45398610..e22fcedd 100644 --- a/internal/iostreams/survey_test.go +++ b/internal/iostreams/survey_test.go @@ -76,47 +76,7 @@ func Test_InputPromptConfig(t *testing.T) { } } -func Test_MultiSelectPromptConfig(t *testing.T) { - tests := map[string]struct { - cfg MultiSelectPromptConfig - required bool - }{ - "required true": { - cfg: MultiSelectPromptConfig{Required: true}, - required: true, - }, - "required false": { - cfg: MultiSelectPromptConfig{Required: false}, - required: false, - }, - } - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - assert.Equal(t, tc.required, tc.cfg.IsRequired()) - assert.Empty(t, tc.cfg.GetFlags()) - }) - } -} - -func Test_PasswordPromptConfig(t *testing.T) { - t.Run("without flag", func(t *testing.T) { - cfg := PasswordPromptConfig{Required: true} - assert.True(t, cfg.IsRequired()) - assert.Empty(t, cfg.GetFlags()) - }) - t.Run("with flag", func(t *testing.T) { - var val string - fs := pflag.NewFlagSet("test", pflag.ContinueOnError) - fs.StringVar(&val, "token", "", "token flag") - flag := fs.Lookup("token") - cfg := PasswordPromptConfig{Required: false, Flag: flag} - assert.False(t, cfg.IsRequired()) - assert.Len(t, cfg.GetFlags(), 1) - assert.Equal(t, "token", cfg.GetFlags()[0].Name) - }) -} - -func Test_retrieveFlagValue(t *testing.T) { +func Test_IOStreams_retrieveFlagValue(t *testing.T) { fsMock := slackdeps.NewFsMock() osMock := slackdeps.NewOsMock() cfg := config.NewConfig(fsMock, osMock) @@ -188,6 +148,46 @@ func Test_retrieveFlagValue(t *testing.T) { } } +func Test_MultiSelectPromptConfig(t *testing.T) { + tests := map[string]struct { + cfg MultiSelectPromptConfig + required bool + }{ + "required true": { + cfg: MultiSelectPromptConfig{Required: true}, + required: true, + }, + "required false": { + cfg: MultiSelectPromptConfig{Required: false}, + required: false, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.required, tc.cfg.IsRequired()) + assert.Empty(t, tc.cfg.GetFlags()) + }) + } +} + +func Test_PasswordPromptConfig(t *testing.T) { + t.Run("without flag", func(t *testing.T) { + cfg := PasswordPromptConfig{Required: true} + assert.True(t, cfg.IsRequired()) + assert.Empty(t, cfg.GetFlags()) + }) + t.Run("with flag", func(t *testing.T) { + var val string + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + fs.StringVar(&val, "token", "", "token flag") + flag := fs.Lookup("token") + cfg := PasswordPromptConfig{Required: false, Flag: flag} + assert.False(t, cfg.IsRequired()) + assert.Len(t, cfg.GetFlags(), 1) + assert.Equal(t, "token", cfg.GetFlags()[0].Name) + }) +} + func Test_SelectPromptConfig(t *testing.T) { t.Run("no flags", func(t *testing.T) { cfg := SelectPromptConfig{Required: true} diff --git a/internal/iostreams/writer_test.go b/internal/iostreams/writer_test.go index 6f165b68..b8e270ea 100644 --- a/internal/iostreams/writer_test.go +++ b/internal/iostreams/writer_test.go @@ -199,7 +199,7 @@ func Test_FilteredWriter(t *testing.T) { } } -func Test_WriteErr(t *testing.T) { +func Test_IOStreams_WriteErr(t *testing.T) { fsMock := slackdeps.NewFsMock() osMock := slackdeps.NewOsMock() cfg := config.NewConfig(fsMock, osMock) @@ -208,6 +208,15 @@ func Test_WriteErr(t *testing.T) { require.NotNil(t, w) } +func Test_IOStreams_WriteOut(t *testing.T) { + fsMock := slackdeps.NewFsMock() + osMock := slackdeps.NewOsMock() + cfg := config.NewConfig(fsMock, osMock) + io := NewIOStreams(cfg, fsMock, osMock) + w := io.WriteOut() + require.NotNil(t, w) +} + func Test_WriteIndent(t *testing.T) { tests := map[string]struct { input string @@ -242,15 +251,6 @@ func Test_WriteIndent(t *testing.T) { } } -func Test_WriteOut(t *testing.T) { - fsMock := slackdeps.NewFsMock() - osMock := slackdeps.NewOsMock() - cfg := config.NewConfig(fsMock, osMock) - io := NewIOStreams(cfg, fsMock, osMock) - w := io.WriteOut() - require.NotNil(t, w) -} - func Test_WriteSecondary(t *testing.T) { tests := map[string]struct { input string From 5552c10ba7b41c151e01e88ec9791e4c6f41c3c9 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Thu, 12 Mar 2026 14:44:50 -0700 Subject: [PATCH 6/6] test: reorder format_test.go alphabetically and adjust test names --- internal/style/format_test.go | 98 +++++++++++++++++------------------ 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/internal/style/format_test.go b/internal/style/format_test.go index d945eff4..41504e9e 100644 --- a/internal/style/format_test.go +++ b/internal/style/format_test.go @@ -24,36 +24,7 @@ import ( "github.com/stretchr/testify/assert" ) -func Test_Commandf(t *testing.T) { - tests := map[string]struct { - process string - command string - isPrimary bool - }{ - "primary command contains process and command": { - process: "renamed-slack-command", - command: "feedback", - isPrimary: true, - }, - "secondary command contains process and command": { - process: "a-renamed-slack-cli", - command: "feedback", - isPrimary: false, - }, - } - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - processTemp := os.Args[0] - os.Args[0] = tc.process - defer func() { os.Args[0] = processTemp }() - - formatted := Commandf(tc.command, tc.isPrimary) - assert.Contains(t, formatted, tc.process+" "+tc.command) - }) - } -} - -func Test_getKeyLength(t *testing.T) { +func TestGetKeyLength(t *testing.T) { tests := map[string]struct { keys map[string]string expected int @@ -82,13 +53,7 @@ func Test_getKeyLength(t *testing.T) { } } -func TestIndent(t *testing.T) { - text := "a few spaces are expected at the start of this line, but no other changes" - indented := Indent(text) - assert.Contains(t, indented, text) -} - -func Test_Sectionf(t *testing.T) { +func TestSectionf(t *testing.T) { tests := map[string]struct { section TextSection expected string @@ -117,7 +82,7 @@ func TestSectionHeaderfEmpty(t *testing.T) { assert.Equal(t, "", SectionHeaderf("tada", "")) } -func Test_SectionSecondaryf(t *testing.T) { +func TestSectionSecondaryf(t *testing.T) { tests := map[string]struct { format string args []interface{} @@ -165,29 +130,41 @@ func Test_SectionSecondaryf(t *testing.T) { } } -func TestSurveyIcons(t *testing.T) { +func TestCommandf(t *testing.T) { tests := map[string]struct { - styleEnabled bool + process string + command string + isPrimary bool }{ - "styles are not enabled": { - styleEnabled: false, + "primary command contains process and command": { + process: "renamed-slack-command", + command: "feedback", + isPrimary: true, }, - "styles are enabled": { - styleEnabled: true, + "secondary command contains process and command": { + process: "a-renamed-slack-cli", + command: "feedback", + isPrimary: false, }, } - for name, tc := range tests { t.Run(name, func(t *testing.T) { - core.DisableColor = false - isStyleEnabled = tc.styleEnabled + processTemp := os.Args[0] + os.Args[0] = tc.process + defer func() { os.Args[0] = processTemp }() - _ = SurveyIcons() - assert.NotEqual(t, tc.styleEnabled, core.DisableColor) + formatted := Commandf(tc.command, tc.isPrimary) + assert.Contains(t, formatted, tc.process+" "+tc.command) }) } } +func TestIndent(t *testing.T) { + text := "a few spaces are expected at the start of this line, but no other changes" + indented := Indent(text) + assert.Contains(t, indented, text) +} + func TestTracef(t *testing.T) { tests := map[string]struct { traceID string @@ -222,6 +199,29 @@ func TestTracef(t *testing.T) { } } +func TestSurveyIcons(t *testing.T) { + tests := map[string]struct { + styleEnabled bool + }{ + "styles are not enabled": { + styleEnabled: false, + }, + "styles are enabled": { + styleEnabled: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + core.DisableColor = false + isStyleEnabled = tc.styleEnabled + + _ = SurveyIcons() + assert.NotEqual(t, tc.styleEnabled, core.DisableColor) + }) + } +} + /* * Example commands */