Skip to content

Commit d2f79e1

Browse files
committed
Eliminate a named type and a loop closure in tests
See #1927 (comment) We can remove the reassignment after [this experiment](golang/go#57969) is concluded.
1 parent 76a2cd0 commit d2f79e1

File tree

1 file changed

+58
-62
lines changed

1 file changed

+58
-62
lines changed

agent/pipeline_uploader_test.go

+58-62
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,12 @@ func TestAsyncPipelineUpload(t *testing.T) {
2222
ctx := context.Background()
2323
l := clicommand.CreateLogger(&clicommand.PipelineUploadConfig{LogLevel: "notice"})
2424

25-
type Test struct {
25+
for _, test := range []struct {
2626
replace bool
2727
state string
2828
expectedSleeps []time.Duration
2929
err error
30-
}
31-
32-
tests := []Test{
30+
}{
3331
{
3432
state: "applied",
3533
expectedSleeps: []time.Duration{},
@@ -50,16 +48,15 @@ func TestAsyncPipelineUpload(t *testing.T) {
5048
}(),
5149
err: errors.New("Failed to upload and process pipeline: Pipeline upload not yet applied: pending"),
5250
},
53-
}
54-
55-
for _, test := range tests {
56-
func(test Test) {
57-
t.Run(test.state, func(t *testing.T) {
58-
t.Parallel()
59-
60-
jobID := api.NewUUID()
61-
stepUploadUUID := api.NewUUID()
62-
pipelineStr := `---
51+
} {
52+
// reassign loop variable to ensure it gets the old value when run concurrently (due to t.Parrallel below)
53+
test := test
54+
t.Run(test.state, func(t *testing.T) {
55+
t.Parallel()
56+
57+
jobID := api.NewUUID()
58+
stepUploadUUID := api.NewUUID()
59+
pipelineStr := `---
6360
steps:
6461
- name: ":s3: xxx"
6562
command: "script/buildkite/xxx.sh"
@@ -80,57 +77,56 @@ steps:
8077
agents:
8178
queue: xxx`
8279

83-
parser := agent.PipelineParser{Pipeline: []byte(pipelineStr), Env: nil}
84-
pipeline, err := parser.Parse()
85-
assert.NoError(t, err)
86-
87-
server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
88-
switch req.URL.Path {
89-
case fmt.Sprintf("/jobs/%s/pipelines", jobID):
90-
assert.Equal(t, req.URL.Query().Get("async"), "true")
91-
if req.Method == "POST" {
92-
rw.Header().Add("Retry-After", "5")
93-
rw.Header().Add("Location", fmt.Sprintf("/jobs/%s/pipelines/%s", jobID, stepUploadUUID))
94-
rw.WriteHeader(http.StatusAccepted)
95-
return
96-
}
97-
case fmt.Sprintf("/jobs/%s/pipelines/%s", jobID, stepUploadUUID):
98-
if req.Method == "GET" {
99-
_, _ = fmt.Fprintf(rw, `{"state": "%s"}`, test.state)
100-
return
101-
}
80+
parser := agent.PipelineParser{Pipeline: []byte(pipelineStr), Env: nil}
81+
pipeline, err := parser.Parse()
82+
assert.NoError(t, err)
83+
84+
server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
85+
switch req.URL.Path {
86+
case fmt.Sprintf("/jobs/%s/pipelines", jobID):
87+
assert.Equal(t, req.URL.Query().Get("async"), "true")
88+
if req.Method == "POST" {
89+
rw.Header().Add("Retry-After", "5")
90+
rw.Header().Add("Location", fmt.Sprintf("/jobs/%s/pipelines/%s", jobID, stepUploadUUID))
91+
rw.WriteHeader(http.StatusAccepted)
92+
return
93+
}
94+
case fmt.Sprintf("/jobs/%s/pipelines/%s", jobID, stepUploadUUID):
95+
if req.Method == "GET" {
96+
_, _ = fmt.Fprintf(rw, `{"state": "%s"}`, test.state)
97+
return
10298
}
103-
t.Errorf("Unknown endpoint %s %s", req.Method, req.URL.Path)
104-
http.Error(rw, "Not found", http.StatusNotFound)
105-
}))
106-
defer server.Close()
107-
108-
retrySleeps := []time.Duration{}
109-
uploader := &agent.PipelineUploader{
110-
Client: api.NewClient(logger.Discard, api.Config{
111-
Endpoint: server.URL,
112-
Token: "llamas",
113-
}),
114-
JobID: jobID,
115-
Change: &api.PipelineChange{
116-
UUID: stepUploadUUID,
117-
Pipeline: pipeline,
118-
Replace: test.replace,
119-
},
120-
RetrySleepFunc: func(d time.Duration) {
121-
retrySleeps = append(retrySleeps, d)
122-
},
12399
}
100+
t.Errorf("Unknown endpoint %s %s", req.Method, req.URL.Path)
101+
http.Error(rw, "Not found", http.StatusNotFound)
102+
}))
103+
defer server.Close()
104+
105+
retrySleeps := []time.Duration{}
106+
uploader := &agent.PipelineUploader{
107+
Client: api.NewClient(logger.Discard, api.Config{
108+
Endpoint: server.URL,
109+
Token: "llamas",
110+
}),
111+
JobID: jobID,
112+
Change: &api.PipelineChange{
113+
UUID: stepUploadUUID,
114+
Pipeline: pipeline,
115+
Replace: test.replace,
116+
},
117+
RetrySleepFunc: func(d time.Duration) {
118+
retrySleeps = append(retrySleeps, d)
119+
},
120+
}
124121

125-
err = uploader.AsyncUploadFlow(ctx, l)
126-
if test.err == nil {
127-
assert.NoError(t, err)
128-
} else {
129-
assert.ErrorContains(t, err, test.err.Error())
130-
}
131-
assert.Equal(t, test.expectedSleeps, retrySleeps)
132-
})
133-
}(test)
122+
err = uploader.AsyncUploadFlow(ctx, l)
123+
if test.err == nil {
124+
assert.NoError(t, err)
125+
} else {
126+
assert.ErrorContains(t, err, test.err.Error())
127+
}
128+
assert.Equal(t, test.expectedSleeps, retrySleeps)
129+
})
134130
}
135131
}
136132

0 commit comments

Comments
 (0)