diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 14a63f2e..fe345cc6 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -135,6 +135,19 @@ 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 +``` + +### 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 20830fc0..02700bdd 100644 --- a/.github/MAINTAINERS_GUIDE.md +++ b/.github/MAINTAINERS_GUIDE.md @@ -383,6 +383,30 @@ 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) { ... } +``` + +#### 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 55c752d3..f508bde4 100644 --- a/internal/iostreams/iostreams_test.go +++ b/internal/iostreams/iostreams_test.go @@ -20,10 +20,41 @@ 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" ) +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_IOSteams_NewIOStreams(t *testing.T) { var io *IOStreams fsMock := slackdeps.NewFsMock() @@ -65,3 +96,15 @@ func Test_IOStreams_IsTTY(t *testing.T) { }) } } + +func Test_IOStreams_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..e22fcedd 100644 --- a/internal/iostreams/survey_test.go +++ b/internal/iostreams/survey_test.go @@ -26,6 +26,214 @@ import ( "github.com/stretchr/testify/assert" ) +func Test_ConfirmPromptConfig(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 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 + 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 Test_IOStreams_retrieveFlagValue(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 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} + 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 d9411b30..b8e270ea 100644 --- a/internal/iostreams/writer_test.go +++ b/internal/iostreams/writer_test.go @@ -199,6 +199,24 @@ func Test_FilteredWriter(t *testing.T) { } } +func Test_IOStreams_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_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