Skip to content

Commit df800e1

Browse files
AlexanderYastrebovAlexanderYastrebov
committed
filters/scheduler: refactor fifo tests (zalando#2693)
* separate argument tests from behaviour test * use eskip to define filter configuration * use TestFifo prefix for all test names Signed-off-by: Alexander Yastrebov <[email protected]>
1 parent f0c6ab5 commit df800e1

File tree

2 files changed

+79
-187
lines changed

2 files changed

+79
-187
lines changed

‎filters/scheduler/fifo_test.go

+79-181
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ import (
55
"io"
66
"net/http"
77
stdlibhttptest "net/http/httptest"
8-
"net/url"
98
"testing"
109
"time"
1110

1211
"github.com/opentracing/opentracing-go/mocktracer"
12+
"github.com/zalando/skipper/eskip"
1313
"github.com/zalando/skipper/filters"
1414
"github.com/zalando/skipper/metrics/metricstest"
1515
"github.com/zalando/skipper/net/httptest"
@@ -19,11 +19,12 @@ import (
1919
"github.com/zalando/skipper/scheduler"
2020
)
2121

22-
func TestCreateFifoFilter(t *testing.T) {
22+
func TestFifoCreateFilter(t *testing.T) {
2323
for _, tt := range []struct {
2424
name string
2525
args []interface{}
2626
wantParseErr bool
27+
wantConfig scheduler.Config
2728
}{
2829
{
2930
name: "fifo no args",
@@ -51,6 +52,11 @@ func TestCreateFifoFilter(t *testing.T) {
5152
5,
5253
"1s",
5354
},
55+
wantConfig: scheduler.Config{
56+
MaxConcurrency: 3,
57+
MaxQueueSize: 5,
58+
Timeout: 1 * time.Second,
59+
},
5460
},
5561
{
5662
name: "fifo wrong type arg1",
@@ -108,91 +114,59 @@ func TestCreateFifoFilter(t *testing.T) {
108114
if err == nil && tt.wantParseErr {
109115
t.Fatal("Failed to get wanted error on create filter")
110116
}
117+
if tt.wantParseErr {
118+
return
119+
}
111120

112-
if _, ok := ff.(*fifoFilter); !ok && err == nil {
121+
f, ok := ff.(*fifoFilter)
122+
if !ok {
113123
t.Fatal("Failed to convert filter to *fifoFilter")
114124
}
125+
126+
// validate config
127+
config := f.Config()
128+
if config != tt.wantConfig {
129+
t.Fatalf("Failed to get Config, got: %v, want: %v", config, tt.wantConfig)
130+
}
131+
if f.queue != f.GetQueue() {
132+
t.Fatal("Failed to get expected queue")
133+
}
115134
})
116135
}
117136
}
118137

119138
func TestFifo(t *testing.T) {
120139
for _, tt := range []struct {
121-
name string
122-
args []interface{}
123-
freq int
124-
per time.Duration
125-
backendTime time.Duration
126-
clientTimeout time.Duration
127-
wantConfig scheduler.Config
128-
wantParseErr bool
129-
wantOkRate float64
130-
epsilon float64
140+
name string
141+
filter string
142+
freq int
143+
per time.Duration
144+
backendTime time.Duration
145+
wantOkRate float64
131146
}{
132147
{
133-
name: "fifo defaults",
134-
args: []interface{}{},
135-
wantParseErr: true,
136-
},
137-
{
138-
name: "fifo simple ok",
139-
args: []interface{}{
140-
3,
141-
5,
142-
"1s",
143-
},
144-
freq: 20,
145-
per: 100 * time.Millisecond,
146-
backendTime: 1 * time.Millisecond,
147-
clientTimeout: time.Second,
148-
wantConfig: scheduler.Config{
149-
MaxConcurrency: 3,
150-
MaxQueueSize: 5,
151-
Timeout: time.Second,
152-
},
153-
wantParseErr: false,
154-
wantOkRate: 1.0,
155-
epsilon: 1,
148+
name: "fifo simple ok",
149+
filter: `fifo(3, 5, "1s")`,
150+
freq: 20,
151+
per: 100 * time.Millisecond,
152+
backendTime: 1 * time.Millisecond,
153+
wantOkRate: 1.0,
156154
},
157155
{
158-
name: "fifo with reaching max concurrency and queue timeouts",
159-
args: []interface{}{
160-
3,
161-
5,
162-
"10ms",
163-
},
164-
freq: 200,
165-
per: 100 * time.Millisecond,
166-
backendTime: 10 * time.Millisecond,
167-
clientTimeout: time.Second,
168-
wantConfig: scheduler.Config{
169-
MaxConcurrency: 3,
170-
MaxQueueSize: 5,
171-
Timeout: 10 * time.Millisecond,
172-
},
173-
wantParseErr: false,
174-
wantOkRate: 0.1,
175-
epsilon: 1,
156+
name: "fifo with reaching max concurrency and queue timeouts",
157+
filter: `fifo(3, 5, "10ms")`,
158+
freq: 200,
159+
per: 100 * time.Millisecond,
160+
backendTime: 10 * time.Millisecond,
161+
wantOkRate: 0.1,
176162
},
177163
{
178-
name: "fifo with reaching max concurrency and queue full",
179-
args: []interface{}{
180-
1,
181-
1,
182-
"250ms",
183-
},
184-
freq: 200,
185-
per: 100 * time.Millisecond,
186-
backendTime: 100 * time.Millisecond,
187-
clientTimeout: time.Second,
188-
wantConfig: scheduler.Config{
189-
MaxConcurrency: 1,
190-
MaxQueueSize: 1,
191-
Timeout: 250 * time.Millisecond,
192-
},
193-
wantParseErr: false,
194-
wantOkRate: 0.0008,
195-
epsilon: 1,
164+
name: "fifo with reaching max concurrency and queue full",
165+
filter: `fifo(3, 5, "250ms")`,
166+
freq: 200,
167+
per: 100 * time.Millisecond,
168+
backendTime: 100 * time.Millisecond,
169+
wantOkRate: 0.0008,
196170
},
197171
} {
198172
t.Run(tt.name, func(t *testing.T) {
@@ -201,29 +175,6 @@ func TestFifo(t *testing.T) {
201175
t.Fatalf("Failed to get name got %s want %s", fs.Name(), filters.FifoName)
202176
}
203177

204-
// no parse error
205-
ff, err := fs.CreateFilter(tt.args)
206-
if err != nil && !tt.wantParseErr {
207-
t.Fatalf("Failed to parse filter: %v", err)
208-
}
209-
if err == nil && tt.wantParseErr {
210-
t.Fatalf("want parse error but have no: %v", err)
211-
}
212-
if tt.wantParseErr {
213-
return
214-
}
215-
216-
// validate config
217-
if f, ok := ff.(*fifoFilter); ok {
218-
config := f.Config()
219-
if config != tt.wantConfig {
220-
t.Fatalf("Failed to get Config, got: %v, want: %v", config, tt.wantConfig)
221-
}
222-
if f.queue != f.GetQueue() {
223-
t.Fatal("Failed to get expected queue")
224-
}
225-
}
226-
227178
metrics := &metricstest.MockMetrics{}
228179
reg := scheduler.RegistryWith(scheduler.Options{
229180
Metrics: metrics,
@@ -241,22 +192,11 @@ func TestFifo(t *testing.T) {
241192
}))
242193
defer backend.Close()
243194

244-
var fmtStr string
245-
switch len(tt.args) {
246-
case 0:
247-
fmtStr = `aroute: * -> fifo() -> "%s"`
248-
case 1:
249-
fmtStr = `aroute: * -> fifo(%v) -> "%s"`
250-
case 2:
251-
fmtStr = `aroute: * -> fifo(%v, %v) -> "%s"`
252-
case 3:
253-
fmtStr = `aroute: * -> fifo(%v, %v, "%v") -> "%s"`
254-
default:
255-
t.Fatalf("Test not possible %d >3", len(tt.args))
195+
if ff := eskip.MustParseFilters(tt.filter); len(ff) != 1 {
196+
t.Fatalf("expected one filter, got %d", len(ff))
256197
}
257198

258-
args := append(tt.args, backend.URL)
259-
doc := fmt.Sprintf(fmtStr, args...)
199+
doc := fmt.Sprintf(`aroute: * -> %s -> "%s"`, tt.filter, backend.URL)
260200
t.Logf("%s", doc)
261201

262202
dc, err := testdataclient.NewDoc(doc)
@@ -285,22 +225,19 @@ func TestFifo(t *testing.T) {
285225
ts := stdlibhttptest.NewServer(pr)
286226
defer ts.Close()
287227

288-
reqURL, err := url.Parse(ts.URL)
228+
rsp, err := ts.Client().Get(ts.URL)
289229
if err != nil {
290-
t.Fatalf("Failed to parse url %s: %v", ts.URL, err)
291-
}
292-
293-
rsp, err := http.DefaultClient.Get(reqURL.String())
294-
if err != nil {
295-
t.Fatalf("Failed to get response from %s: %v", reqURL.String(), err)
230+
t.Fatalf("Failed to get response from %s: %v", ts.URL, err)
296231
}
297232
defer rsp.Body.Close()
298233

299234
if rsp.StatusCode != http.StatusOK {
300235
t.Fatalf("Failed to get valid response from endpoint: %d", rsp.StatusCode)
301236
}
302237

303-
va := httptest.NewVegetaAttacker(reqURL.String(), tt.freq, tt.per, tt.clientTimeout)
238+
const clientTimeout = 1 * time.Second
239+
240+
va := httptest.NewVegetaAttacker(ts.URL, tt.freq, tt.per, clientTimeout)
304241
va.Attack(io.Discard, 1*time.Second, tt.name)
305242

306243
t.Logf("Success [0..1]: %0.2f", va.Success())
@@ -327,40 +264,24 @@ func TestFifo(t *testing.T) {
327264
}
328265
}
329266

330-
func TestConstantRouteUpdatesFifo(t *testing.T) {
267+
func TestFifoConstantRouteUpdates(t *testing.T) {
331268
for _, tt := range []struct {
332-
name string
333-
args []interface{}
334-
freq int
335-
per time.Duration
336-
updateRate time.Duration
337-
backendTime time.Duration
338-
clientTimeout time.Duration
339-
wantConfig scheduler.Config
340-
wantParseErr bool
341-
wantOkRate float64
342-
epsilon float64
269+
name string
270+
filter string
271+
freq int
272+
per time.Duration
273+
updateRate time.Duration
274+
backendTime time.Duration
275+
wantOkRate float64
343276
}{
344277
{
345-
name: "fifo simple ok",
346-
args: []interface{}{
347-
3,
348-
5,
349-
"1s",
350-
},
351-
freq: 20,
352-
per: 100 * time.Millisecond,
353-
updateRate: 25 * time.Millisecond,
354-
backendTime: 1 * time.Millisecond,
355-
clientTimeout: time.Second,
356-
wantConfig: scheduler.Config{
357-
MaxConcurrency: 3,
358-
MaxQueueSize: 5,
359-
Timeout: time.Second,
360-
},
361-
wantParseErr: false,
362-
wantOkRate: 1.0,
363-
epsilon: 1,
278+
name: "fifo simple ok",
279+
filter: `fifo(3, 5, "1s")`,
280+
freq: 20,
281+
per: 100 * time.Millisecond,
282+
updateRate: 25 * time.Millisecond,
283+
backendTime: 1 * time.Millisecond,
284+
wantOkRate: 1.0,
364285
},
365286
} {
366287
t.Run(tt.name, func(t *testing.T) {
@@ -369,29 +290,6 @@ func TestConstantRouteUpdatesFifo(t *testing.T) {
369290
t.Fatalf("Failed to get name got %s want %s", fs.Name(), filters.FifoName)
370291
}
371292

372-
// no parse error
373-
ff, err := fs.CreateFilter(tt.args)
374-
if err != nil && !tt.wantParseErr {
375-
t.Fatalf("Failed to parse filter: %v", err)
376-
}
377-
if err == nil && tt.wantParseErr {
378-
t.Fatalf("want parse error but have no: %v", err)
379-
}
380-
if tt.wantParseErr {
381-
return
382-
}
383-
384-
// validate config
385-
if f, ok := ff.(*fifoFilter); ok {
386-
config := f.Config()
387-
if config != tt.wantConfig {
388-
t.Fatalf("Failed to get Config, got: %v, want: %v", config, tt.wantConfig)
389-
}
390-
if f.queue != f.GetQueue() {
391-
t.Fatal("Failed to get expected queue")
392-
}
393-
}
394-
395293
metrics := &metricstest.MockMetrics{}
396294
reg := scheduler.RegistryWith(scheduler.Options{
397295
Metrics: metrics,
@@ -409,8 +307,11 @@ func TestConstantRouteUpdatesFifo(t *testing.T) {
409307
}))
410308
defer backend.Close()
411309

412-
args := append(tt.args, backend.URL)
413-
doc := fmt.Sprintf(`aroute: * -> fifo(%v, %v, "%v") -> "%s"`, args...)
310+
if ff := eskip.MustParseFilters(tt.filter); len(ff) != 1 {
311+
t.Fatalf("expected one filter, got %d", len(ff))
312+
}
313+
314+
doc := fmt.Sprintf(`aroute: * -> %s -> "%s"`, tt.filter, backend.URL)
414315

415316
dc, err := testdataclient.NewDoc(doc)
416317
if err != nil {
@@ -438,14 +339,9 @@ func TestConstantRouteUpdatesFifo(t *testing.T) {
438339
ts := stdlibhttptest.NewServer(pr)
439340
defer ts.Close()
440341

441-
reqURL, err := url.Parse(ts.URL)
342+
rsp, err := ts.Client().Get(ts.URL)
442343
if err != nil {
443-
t.Fatalf("Failed to parse url %s: %v", ts.URL, err)
444-
}
445-
446-
rsp, err := http.DefaultClient.Get(reqURL.String())
447-
if err != nil {
448-
t.Fatalf("Failed to get response from %s: %v", reqURL.String(), err)
344+
t.Fatalf("Failed to get response from %s: %v", ts.URL, err)
449345
}
450346
defer rsp.Body.Close()
451347

@@ -475,7 +371,9 @@ func TestConstantRouteUpdatesFifo(t *testing.T) {
475371

476372
}(quit, tt.updateRate, doc, newDoc)
477373

478-
va := httptest.NewVegetaAttacker(reqURL.String(), tt.freq, tt.per, tt.clientTimeout)
374+
const clientTimeout = 1 * time.Second
375+
376+
va := httptest.NewVegetaAttacker(ts.URL, tt.freq, tt.per, clientTimeout)
479377
va.Attack(io.Discard, 1*time.Second, tt.name)
480378
quit <- struct{}{}
481379

0 commit comments

Comments
 (0)