From dbd83a1f0db2c25bc51b53d610f872cd44e03746 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Mon, 30 Dec 2019 21:31:49 +0100 Subject: [PATCH] Fix flapping TestRest_ tests (#513) * replace single-member wait groups with channels * increase TestRest_LastCommentsStreamSince comment write delay * increase timeout for TestRest_InfoStreamCancel * increase TestServerApp timeout * adjust waiting time in TestRest_LastCommentsStreamSince in attempt to fix false positive * adjust comments waiting location in multiple TestRest_ tests --- backend/app/cmd/server_test.go | 2 +- backend/app/main_test.go | 8 ++-- backend/app/rest/api/rest_public_test.go | 52 ++++++++++-------------- 3 files changed, 26 insertions(+), 36 deletions(-) diff --git a/backend/app/cmd/server_test.go b/backend/app/cmd/server_test.go index 98028945..26894f36 100644 --- a/backend/app/cmd/server_test.go +++ b/backend/app/cmd/server_test.go @@ -43,7 +43,7 @@ func TestServerApp(t *testing.T) { assert.Equal(t, "pong", string(body)) // add comment - client := http.Client{Timeout: 5 * time.Second} + client := http.Client{Timeout: 10 * time.Second} req, err := http.NewRequest("POST", fmt.Sprintf("http://localhost:%d/api/v1/comment", port), strings.NewReader(`{"text": "test 123", "locator":{"url": "https://radio-t.com/blah1", "site": "remark"}}`)) require.NoError(t, err) diff --git a/backend/app/main_test.go b/backend/app/main_test.go index 4b739183..65da9ea1 100644 --- a/backend/app/main_test.go +++ b/backend/app/main_test.go @@ -9,7 +9,6 @@ import ( "os" "strconv" "strings" - "sync" "syscall" "testing" "time" @@ -36,11 +35,10 @@ func Test_Main(t *testing.T) { require.NoError(t, err) }() - wg := sync.WaitGroup{} - wg.Add(1) + finished := make(chan struct{}) go func() { main() - wg.Done() + close(finished) }() waitForHTTPServerStart(port) @@ -53,7 +51,7 @@ func Test_Main(t *testing.T) { assert.Equal(t, "pong", string(body)) close(done) - wg.Wait() + <-finished } func TestGetDump(t *testing.T) { diff --git a/backend/app/rest/api/rest_public_test.go b/backend/app/rest/api/rest_public_test.go index 60bf0569..6232ba38 100644 --- a/backend/app/rest/api/rest_public_test.go +++ b/backend/app/rest/api/rest_public_test.go @@ -557,10 +557,9 @@ func TestRest_InfoStream(t *testing.T) { postComment(t, ts.URL) - wg := sync.WaitGroup{} - wg.Add(1) + done := make(chan struct{}) go func() { - defer wg.Done() + defer close(done) for i := 0; i < 10; i++ { time.Sleep(10 * time.Millisecond) postComment(t, ts.URL) @@ -569,7 +568,7 @@ func TestRest_InfoStream(t *testing.T) { body, code := get(t, ts.URL+"/api/v1/stream/info?site=remark42&url=https://radio-t.com/blah1") assert.Equal(t, 200, code) - wg.Wait() + <-done recs := strings.Split(strings.TrimSuffix(string(body), "\n"), "\n") require.Equal(t, 10*3, len(recs), "10 records. each 2 lines +1 emty line") @@ -626,16 +625,15 @@ func TestRest_InfoStreamCancel(t *testing.T) { ts, srv, teardown := startupT(t) defer teardown() srv.pubRest.readOnlyAge = 10000000 // make sure we don't hit read-only - srv.pubRest.streamer.Refresh = 5 * time.Millisecond + srv.pubRest.streamer.Refresh = 10 * time.Millisecond srv.pubRest.streamer.TimeOut = 1500 * time.Millisecond srv.pubRest.streamer.MaxActive = 100 postComment(t, ts.URL) - wg := sync.WaitGroup{} - wg.Add(1) + done := make(chan struct{}) go func() { - defer wg.Done() + defer close(done) for i := 0; i < 5; i++ { time.Sleep(300 * time.Millisecond) postComment(t, ts.URL) @@ -645,19 +643,18 @@ func TestRest_InfoStreamCancel(t *testing.T) { client := http.Client{} req, err := http.NewRequest("GET", ts.URL+"/api/v1/stream/info?site=remark42&url=https://radio-t.com/blah1", nil) require.NoError(t, err) - ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), 1000*time.Millisecond) defer cancel() req = req.WithContext(ctx) r, err := client.Do(req) require.NoError(t, err) defer r.Body.Close() <-ctx.Done() + <-done body, err := ioutil.ReadAll(r.Body) require.EqualError(t, err, "context deadline exceeded") assert.Equal(t, 200, r.StatusCode) - wg.Wait() - recs := strings.Count(string(body), "data:") require.Equal(t, 1, recs, "should have 1 event:\n", string(body)) assert.Contains(t, string(body), `"count":2`) @@ -673,10 +670,9 @@ func TestRest_InfoStreamSince(t *testing.T) { postComment(t, ts.URL) - wg := sync.WaitGroup{} - wg.Add(1) + done := make(chan struct{}) go func() { - defer wg.Done() + defer close(done) for i := 0; i < 10; i++ { time.Sleep(15 * time.Millisecond) postComment(t, ts.URL) @@ -685,7 +681,7 @@ func TestRest_InfoStreamSince(t *testing.T) { body, code := get(t, ts.URL+"/api/v1/stream/info?site=remark42&url=https://radio-t.com/blah1&since=12345678") assert.Equal(t, 200, code) - wg.Wait() + <-done recs := strings.Split(strings.TrimSuffix(body, "\n"), "\n") require.Equal(t, 11*3, len(recs), "include first record, total 11 records. each 2 lines +1 empty line") } @@ -711,10 +707,9 @@ func TestRest_LastCommentsStream(t *testing.T) { postComment(t, ts.URL) defer teardown() - wg := sync.WaitGroup{} - wg.Add(1) + done := make(chan struct{}) go func() { - defer wg.Done() + defer close(done) for i := 1; i < 10; i++ { time.Sleep(100 * time.Millisecond) postComment(t, ts.URL) @@ -727,11 +722,11 @@ func TestRest_LastCommentsStream(t *testing.T) { r, err := client.Do(req) require.NoError(t, err) defer r.Body.Close() + <-done body, err := ioutil.ReadAll(r.Body) require.NoError(t, err) assert.Equal(t, 200, r.StatusCode) - wg.Wait() assert.Equal(t, "text/event-stream", r.Header.Get("content-type")) assert.Equal(t, "keep-alive", r.Header.Get("connection")) @@ -766,10 +761,9 @@ func TestRest_LastCommentsStreamCancel(t *testing.T) { postComment(t, ts.URL) defer teardown() - wg := sync.WaitGroup{} - wg.Add(1) + done := make(chan struct{}) go func() { - defer wg.Done() + defer close(done) for i := 1; i < 10; i++ { time.Sleep(100 * time.Millisecond) postComment(t, ts.URL) @@ -784,13 +778,12 @@ func TestRest_LastCommentsStreamCancel(t *testing.T) { req = req.WithContext(ctx) r, err := client.Do(req) require.NoError(t, err) + <-done defer r.Body.Close() body, err := ioutil.ReadAll(r.Body) require.EqualError(t, err, "context deadline exceeded") assert.Equal(t, 200, r.StatusCode) - wg.Wait() - recs := strings.Split(strings.TrimSuffix(string(body), "\n"), "\n") assert.True(t, len(recs) < 30, "less 10 events") } @@ -834,12 +827,11 @@ func TestRest_LastCommentsStreamSince(t *testing.T) { postComment(t, ts.URL) defer teardown() - wg := sync.WaitGroup{} - wg.Add(1) + done := make(chan struct{}) go func() { - defer wg.Done() + defer close(done) for i := 1; i < 10; i++ { - time.Sleep(100 * time.Millisecond) + time.Sleep(50 * time.Millisecond) postComment(t, ts.URL) } }() @@ -849,16 +841,16 @@ func TestRest_LastCommentsStreamSince(t *testing.T) { require.NoError(t, err) r, err := client.Do(req) require.NoError(t, err) + <-done defer r.Body.Close() body, err := ioutil.ReadAll(r.Body) require.NoError(t, err) assert.Equal(t, 200, r.StatusCode) - wg.Wait() assert.Equal(t, "text/event-stream", r.Header.Get("content-type")) recs := strings.Split(strings.TrimSuffix(string(body), "\n"), "\n") - require.Equal(t, 10*3, len(recs), "10 events, includes first record") + require.Equal(t, 10*3, len(recs), "should be 10 events, including first record:\n", recs) } func postComment(t *testing.T, url string) {