Skip to content

Commit 4998899

Browse files
authored
test: increase test coverage for iostreams (#394)
* test: increase test coverage for iostreams * 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. * 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. * docs: fix whitespace alignment in CLAUDE.md test naming example * test: apply Test_StructName_FunctionName format and reorder alphabetically * test: reorder format_test.go alphabetically and adjust test names * docs: add constructor-first rule to test ordering conventions Update CLAUDE.md and MAINTAINERS_GUIDE.md to state that constructor functions (NewXYZ) should always be declared first at the top of test files, before alphabetically ordered test functions. Also allow exceptions when alphabetical ordering doesn't work well. Apply the new rule to iostreams_test.go by moving Test_IOSteams_NewIOStreams to the top of the file. * test: revert survey_test.go to main for PR#404 compatibility Remove tests added in this branch since PR#404 refactors this code and includes similar test coverage.
1 parent 742f261 commit 4998899

4 files changed

Lines changed: 102 additions & 0 deletions

File tree

.claude/CLAUDE.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,19 @@ func NewExampleCommand(clients *shared.ClientFactory) *cobra.Command {
135135
- Mock the `ClientFactory` and its dependencies for testing
136136
- Always mock file system operations using `afero.Fs` to enable testability
137137

138+
### Test Naming Conventions
139+
140+
Test function names use the format `Test_StructName_FunctionName` for methods on a struct, or `Test_FunctionName` for package-level functions:
141+
142+
```go
143+
func Test_Client_GetAppStatus(t *testing.T) { ... } // struct method
144+
func Test_getKeyLength(t *testing.T) { ... } // package-level function
145+
```
146+
147+
### Test Ordering Conventions
148+
149+
Constructor functions (`NewXYZ`) should always be declared first, at the top of the test file. After constructors, 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`). Exceptions to alphabetical ordering can be made when it doesn't work well for readability or logical grouping.
150+
138151
### Table-Driven Test Conventions
139152

140153
**Preferred: Map pattern** - uses `tc` for test case variable:

.github/MAINTAINERS_GUIDE.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,34 @@ The branch name can also be set by changing
383383
for the `build-lint-test-e2e-test` workflow in the `.circleci/config.yml` file,
384384
but take care not to merge this change into `main`!
385385
386+
#### Test naming conventions
387+
388+
Test function names should use the format `Test_StructName_FunctionName` for methods
389+
on a struct, or `Test_FunctionName` for package-level functions. The underscore after
390+
`Test` separates the Go test prefix from the identifier being tested:
391+
392+
```go
393+
// Testing a method on a struct
394+
func Test_Client_GetAppStatus(t *testing.T) { ... }
395+
396+
// Testing a package-level function
397+
func Test_getKeyLength(t *testing.T) { ... }
398+
```
399+
400+
#### Test ordering conventions
401+
402+
Constructor functions (`NewXYZ`) should always be declared first, at the top of the
403+
test file. After constructors, test functions should be ordered alphabetically. When
404+
a file has logical sections (separated by comments), tests should be alphabetical
405+
within each section.
406+
407+
Getter and setter functions should be grouped together under the base name. Ignore
408+
the `Get` or `Set` prefix when determining alphabetical order. For example,
409+
`Test_AppName` and `Test_SetAppName` are both sorted under `A` for `AppName`.
410+
411+
Exceptions to alphabetical ordering can be made when it doesn't work well for
412+
readability or logical grouping.
413+
386414
#### Contributing tests
387415

388416
If you'd like to add tests, please review our

internal/iostreams/iostreams_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020

2121
"github.com/slackapi/slack-cli/internal/config"
2222
"github.com/slackapi/slack-cli/internal/slackdeps"
23+
"github.com/spf13/cobra"
2324
"github.com/stretchr/testify/assert"
2425
"github.com/stretchr/testify/require"
2526
)
@@ -34,6 +35,36 @@ func Test_IOSteams_NewIOStreams(t *testing.T) {
3435
require.True(t, io.config.DebugEnabled, "iostreams references config")
3536
}
3637

38+
func Test_IOStreams_ExitCode(t *testing.T) {
39+
tests := map[string]struct {
40+
setCode ExitCode
41+
expected ExitCode
42+
}{
43+
"default is ExitOK": {
44+
setCode: ExitOK,
45+
expected: ExitOK,
46+
},
47+
"set to ExitError": {
48+
setCode: ExitError,
49+
expected: ExitError,
50+
},
51+
"set to ExitCancel": {
52+
setCode: ExitCancel,
53+
expected: ExitCancel,
54+
},
55+
}
56+
for name, tc := range tests {
57+
t.Run(name, func(t *testing.T) {
58+
fsMock := slackdeps.NewFsMock()
59+
osMock := slackdeps.NewOsMock()
60+
cfg := config.NewConfig(fsMock, osMock)
61+
io := NewIOStreams(cfg, fsMock, osMock)
62+
io.SetExitCode(tc.setCode)
63+
assert.Equal(t, tc.expected, io.GetExitCode())
64+
})
65+
}
66+
}
67+
3768
func Test_IOStreams_IsTTY(t *testing.T) {
3869
tests := map[string]struct {
3970
fileInfo os.FileInfo
@@ -65,3 +96,15 @@ func Test_IOStreams_IsTTY(t *testing.T) {
6596
})
6697
}
6798
}
99+
100+
func Test_IOStreams_SetCmdIO(t *testing.T) {
101+
fsMock := slackdeps.NewFsMock()
102+
osMock := slackdeps.NewOsMock()
103+
cfg := config.NewConfig(fsMock, osMock)
104+
io := NewIOStreams(cfg, fsMock, osMock)
105+
cmd := &cobra.Command{Use: "test"}
106+
io.SetCmdIO(cmd)
107+
assert.NotNil(t, cmd.InOrStdin())
108+
assert.NotNil(t, cmd.OutOrStdout())
109+
assert.NotNil(t, cmd.ErrOrStderr())
110+
}

internal/iostreams/writer_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,24 @@ func Test_FilteredWriter(t *testing.T) {
199199
}
200200
}
201201

202+
func Test_IOStreams_WriteErr(t *testing.T) {
203+
fsMock := slackdeps.NewFsMock()
204+
osMock := slackdeps.NewOsMock()
205+
cfg := config.NewConfig(fsMock, osMock)
206+
io := NewIOStreams(cfg, fsMock, osMock)
207+
w := io.WriteErr()
208+
require.NotNil(t, w)
209+
}
210+
211+
func Test_IOStreams_WriteOut(t *testing.T) {
212+
fsMock := slackdeps.NewFsMock()
213+
osMock := slackdeps.NewOsMock()
214+
cfg := config.NewConfig(fsMock, osMock)
215+
io := NewIOStreams(cfg, fsMock, osMock)
216+
w := io.WriteOut()
217+
require.NotNil(t, w)
218+
}
219+
202220
func Test_WriteIndent(t *testing.T) {
203221
tests := map[string]struct {
204222
input string

0 commit comments

Comments
 (0)