Skip to content

Commit fc34422

Browse files
committed
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.
1 parent 6188979 commit fc34422

6 files changed

Lines changed: 132 additions & 118 deletions

File tree

.claude/CLAUDE.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,10 @@ func Test_Client_GetAppStatus(t *testing.T) { ... } // struct method
144144
func Test_getKeyLength(t *testing.T) { ... } // package-level function
145145
```
146146

147+
### Test Ordering Conventions
148+
149+
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`).
150+
147151
### Table-Driven Test Conventions
148152

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

.github/MAINTAINERS_GUIDE.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,16 @@ func Test_Client_GetAppStatus(t *testing.T) { ... }
397397
func Test_getKeyLength(t *testing.T) { ... }
398398
```
399399
400+
#### Test ordering conventions
401+
402+
Test functions should be ordered alphabetically within each file. When a file has
403+
logical sections (separated by comments), tests should be alphabetical within each
404+
section.
405+
406+
Getter and setter functions should be grouped together under the base name. Ignore
407+
the `Get` or `Set` prefix when determining alphabetical order. For example,
408+
`Test_AppName` and `Test_SetAppName` are both sorted under `A` for `AppName`.
409+
400410
#### Contributing tests
401411
402412
If you'd like to add tests, please review our

internal/iostreams/iostreams_test.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,6 @@ import (
2525
"github.com/stretchr/testify/require"
2626
)
2727

28-
func Test_IOSteams_NewIOStreams(t *testing.T) {
29-
var io *IOStreams
30-
fsMock := slackdeps.NewFsMock()
31-
osMock := slackdeps.NewOsMock()
32-
config := config.NewConfig(fsMock, osMock)
33-
config.DebugEnabled = true
34-
io = NewIOStreams(config, fsMock, osMock)
35-
require.True(t, io.config.DebugEnabled, "iostreams references config")
36-
}
37-
3828
func Test_IOStreams_ExitCode(t *testing.T) {
3929
tests := map[string]struct {
4030
setCode ExitCode
@@ -65,6 +55,16 @@ func Test_IOStreams_ExitCode(t *testing.T) {
6555
}
6656
}
6757

58+
func Test_IOSteams_NewIOStreams(t *testing.T) {
59+
var io *IOStreams
60+
fsMock := slackdeps.NewFsMock()
61+
osMock := slackdeps.NewOsMock()
62+
config := config.NewConfig(fsMock, osMock)
63+
config.DebugEnabled = true
64+
io = NewIOStreams(config, fsMock, osMock)
65+
require.True(t, io.config.DebugEnabled, "iostreams references config")
66+
}
67+
6868
func Test_IOStreams_IsTTY(t *testing.T) {
6969
tests := map[string]struct {
7070
fileInfo os.FileInfo
@@ -97,7 +97,7 @@ func Test_IOStreams_IsTTY(t *testing.T) {
9797
}
9898
}
9999

100-
func Test_SetCmdIO(t *testing.T) {
100+
func Test_IOStreams_SetCmdIO(t *testing.T) {
101101
fsMock := slackdeps.NewFsMock()
102102
osMock := slackdeps.NewOsMock()
103103
cfg := config.NewConfig(fsMock, osMock)

internal/iostreams/survey_test.go

Lines changed: 52 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@ func Test_ConfirmPromptConfig(t *testing.T) {
4848
}
4949
}
5050

51+
func Test_DefaultSelectPromptConfig(t *testing.T) {
52+
cfg := DefaultSelectPromptConfig()
53+
assert.True(t, cfg.IsRequired())
54+
assert.Empty(t, cfg.GetFlags())
55+
}
56+
5157
func Test_InputPromptConfig(t *testing.T) {
5258
tests := map[string]struct {
5359
cfg InputPromptConfig
@@ -110,58 +116,6 @@ func Test_PasswordPromptConfig(t *testing.T) {
110116
})
111117
}
112118

113-
func Test_SelectPromptConfig(t *testing.T) {
114-
t.Run("no flags", func(t *testing.T) {
115-
cfg := SelectPromptConfig{Required: true}
116-
assert.True(t, cfg.IsRequired())
117-
assert.Empty(t, cfg.GetFlags())
118-
})
119-
t.Run("single flag", func(t *testing.T) {
120-
var val string
121-
fs := pflag.NewFlagSet("test", pflag.ContinueOnError)
122-
fs.StringVar(&val, "app", "", "app flag")
123-
flag := fs.Lookup("app")
124-
cfg := SelectPromptConfig{Flag: flag}
125-
assert.Len(t, cfg.GetFlags(), 1)
126-
})
127-
t.Run("multiple flags", func(t *testing.T) {
128-
var v1, v2 string
129-
fs := pflag.NewFlagSet("test", pflag.ContinueOnError)
130-
fs.StringVar(&v1, "a", "", "")
131-
fs.StringVar(&v2, "b", "", "")
132-
cfg := SelectPromptConfig{Flags: []*pflag.Flag{fs.Lookup("a"), fs.Lookup("b")}}
133-
assert.Len(t, cfg.GetFlags(), 2)
134-
})
135-
}
136-
137-
func Test_SurveyOptions(t *testing.T) {
138-
tests := map[string]struct {
139-
cfg PromptConfig
140-
expectedLen int
141-
}{
142-
"required config returns 5 options": {
143-
cfg: ConfirmPromptConfig{Required: true},
144-
expectedLen: 5,
145-
},
146-
"optional config returns 5 options": {
147-
cfg: ConfirmPromptConfig{Required: false},
148-
expectedLen: 5,
149-
},
150-
}
151-
for name, tc := range tests {
152-
t.Run(name, func(t *testing.T) {
153-
opts := SurveyOptions(tc.cfg)
154-
assert.Len(t, opts, tc.expectedLen)
155-
})
156-
}
157-
}
158-
159-
func Test_DefaultSelectPromptConfig(t *testing.T) {
160-
cfg := DefaultSelectPromptConfig()
161-
assert.True(t, cfg.IsRequired())
162-
assert.Empty(t, cfg.GetFlags())
163-
}
164-
165119
func Test_retrieveFlagValue(t *testing.T) {
166120
fsMock := slackdeps.NewFsMock()
167121
osMock := slackdeps.NewOsMock()
@@ -234,6 +188,52 @@ func Test_retrieveFlagValue(t *testing.T) {
234188
}
235189
}
236190

191+
func Test_SelectPromptConfig(t *testing.T) {
192+
t.Run("no flags", func(t *testing.T) {
193+
cfg := SelectPromptConfig{Required: true}
194+
assert.True(t, cfg.IsRequired())
195+
assert.Empty(t, cfg.GetFlags())
196+
})
197+
t.Run("single flag", func(t *testing.T) {
198+
var val string
199+
fs := pflag.NewFlagSet("test", pflag.ContinueOnError)
200+
fs.StringVar(&val, "app", "", "app flag")
201+
flag := fs.Lookup("app")
202+
cfg := SelectPromptConfig{Flag: flag}
203+
assert.Len(t, cfg.GetFlags(), 1)
204+
})
205+
t.Run("multiple flags", func(t *testing.T) {
206+
var v1, v2 string
207+
fs := pflag.NewFlagSet("test", pflag.ContinueOnError)
208+
fs.StringVar(&v1, "a", "", "")
209+
fs.StringVar(&v2, "b", "", "")
210+
cfg := SelectPromptConfig{Flags: []*pflag.Flag{fs.Lookup("a"), fs.Lookup("b")}}
211+
assert.Len(t, cfg.GetFlags(), 2)
212+
})
213+
}
214+
215+
func Test_SurveyOptions(t *testing.T) {
216+
tests := map[string]struct {
217+
cfg PromptConfig
218+
expectedLen int
219+
}{
220+
"required config returns 5 options": {
221+
cfg: ConfirmPromptConfig{Required: true},
222+
expectedLen: 5,
223+
},
224+
"optional config returns 5 options": {
225+
cfg: ConfirmPromptConfig{Required: false},
226+
expectedLen: 5,
227+
},
228+
}
229+
for name, tc := range tests {
230+
t.Run(name, func(t *testing.T) {
231+
opts := SurveyOptions(tc.cfg)
232+
assert.Len(t, opts, tc.expectedLen)
233+
})
234+
}
235+
}
236+
237237
func TestPasswordPrompt(t *testing.T) {
238238
tests := map[string]struct {
239239
FlagChanged bool

internal/iostreams/writer_test.go

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

202+
func Test_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+
202211
func Test_WriteIndent(t *testing.T) {
203212
tests := map[string]struct {
204213
input string
@@ -242,15 +251,6 @@ func Test_WriteOut(t *testing.T) {
242251
require.NotNil(t, w)
243252
}
244253

245-
func Test_WriteErr(t *testing.T) {
246-
fsMock := slackdeps.NewFsMock()
247-
osMock := slackdeps.NewOsMock()
248-
cfg := config.NewConfig(fsMock, osMock)
249-
io := NewIOStreams(cfg, fsMock, osMock)
250-
w := io.WriteErr()
251-
require.NotNil(t, w)
252-
}
253-
254254
func Test_WriteSecondary(t *testing.T) {
255255
tests := map[string]struct {
256256
input string

internal/style/format_test.go

Lines changed: 46 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,35 @@ import (
2424
"github.com/stretchr/testify/assert"
2525
)
2626

27+
func Test_Commandf(t *testing.T) {
28+
tests := map[string]struct {
29+
process string
30+
command string
31+
isPrimary bool
32+
}{
33+
"primary command contains process and command": {
34+
process: "renamed-slack-command",
35+
command: "feedback",
36+
isPrimary: true,
37+
},
38+
"secondary command contains process and command": {
39+
process: "a-renamed-slack-cli",
40+
command: "feedback",
41+
isPrimary: false,
42+
},
43+
}
44+
for name, tc := range tests {
45+
t.Run(name, func(t *testing.T) {
46+
processTemp := os.Args[0]
47+
os.Args[0] = tc.process
48+
defer func() { os.Args[0] = processTemp }()
49+
50+
formatted := Commandf(tc.command, tc.isPrimary)
51+
assert.Contains(t, formatted, tc.process+" "+tc.command)
52+
})
53+
}
54+
}
55+
2756
func Test_getKeyLength(t *testing.T) {
2857
tests := map[string]struct {
2958
keys map[string]string
@@ -53,6 +82,12 @@ func Test_getKeyLength(t *testing.T) {
5382
}
5483
}
5584

85+
func TestIndent(t *testing.T) {
86+
text := "a few spaces are expected at the start of this line, but no other changes"
87+
indented := Indent(text)
88+
assert.Contains(t, indented, text)
89+
}
90+
5691
func Test_Sectionf(t *testing.T) {
5792
tests := map[string]struct {
5893
section TextSection
@@ -130,41 +165,29 @@ func Test_SectionSecondaryf(t *testing.T) {
130165
}
131166
}
132167

133-
func Test_Commandf(t *testing.T) {
168+
func TestSurveyIcons(t *testing.T) {
134169
tests := map[string]struct {
135-
process string
136-
command string
137-
isPrimary bool
170+
styleEnabled bool
138171
}{
139-
"primary command contains process and command": {
140-
process: "renamed-slack-command",
141-
command: "feedback",
142-
isPrimary: true,
172+
"styles are not enabled": {
173+
styleEnabled: false,
143174
},
144-
"secondary command contains process and command": {
145-
process: "a-renamed-slack-cli",
146-
command: "feedback",
147-
isPrimary: false,
175+
"styles are enabled": {
176+
styleEnabled: true,
148177
},
149178
}
179+
150180
for name, tc := range tests {
151181
t.Run(name, func(t *testing.T) {
152-
processTemp := os.Args[0]
153-
os.Args[0] = tc.process
154-
defer func() { os.Args[0] = processTemp }()
182+
core.DisableColor = false
183+
isStyleEnabled = tc.styleEnabled
155184

156-
formatted := Commandf(tc.command, tc.isPrimary)
157-
assert.Contains(t, formatted, tc.process+" "+tc.command)
185+
_ = SurveyIcons()
186+
assert.NotEqual(t, tc.styleEnabled, core.DisableColor)
158187
})
159188
}
160189
}
161190

162-
func TestIndent(t *testing.T) {
163-
text := "a few spaces are expected at the start of this line, but no other changes"
164-
indented := Indent(text)
165-
assert.Contains(t, indented, text)
166-
}
167-
168191
func TestTracef(t *testing.T) {
169192
tests := map[string]struct {
170193
traceID string
@@ -199,29 +222,6 @@ func TestTracef(t *testing.T) {
199222
}
200223
}
201224

202-
func TestSurveyIcons(t *testing.T) {
203-
tests := map[string]struct {
204-
styleEnabled bool
205-
}{
206-
"styles are not enabled": {
207-
styleEnabled: false,
208-
},
209-
"styles are enabled": {
210-
styleEnabled: true,
211-
},
212-
}
213-
214-
for name, tc := range tests {
215-
t.Run(name, func(t *testing.T) {
216-
core.DisableColor = false
217-
isStyleEnabled = tc.styleEnabled
218-
219-
_ = SurveyIcons()
220-
assert.NotEqual(t, tc.styleEnabled, core.DisableColor)
221-
})
222-
}
223-
}
224-
225225
/*
226226
* Example commands
227227
*/

0 commit comments

Comments
 (0)