From f416c6c5eb3461c8f7f8726f65dbd99fb92c33e6 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Mon, 30 Dec 2019 19:09:05 +0100 Subject: [PATCH] Fixes for multiple tests (#511) * improve TestServer* reliability * improve TestService_UserReplies reliability * increase timeout for Test_Main * improve TestRest_CreateWithPictures readability and reliability * introduce random port to REST over SSL tests * tinker TestRest_InfoStreamSince to have more slack before failure * finalize test errors check unification * simplify prepServerApp in cmd package tests * improve TestRest_InfoStreamCancel reliability --- .../memory_store/accessor/data_test.go | 10 ++-- backend/app/cmd/backup_test.go | 8 +-- backend/app/cmd/server_test.go | 52 ++++++++----------- backend/app/main_test.go | 10 ++-- backend/app/notify/email_test.go | 18 +++---- backend/app/notify/telegram_test.go | 2 +- backend/app/rest/api/admin_test.go | 14 ++--- backend/app/rest/api/rest_private_test.go | 36 ++++++------- backend/app/rest/api/rest_public_test.go | 32 ++++-------- backend/app/rest/api/rest_test.go | 32 ++++++------ backend/app/rest/httperrors_test.go | 12 ++--- backend/app/store/service/service_test.go | 16 +++--- backend/app/store/service/title_test.go | 16 +++--- backend/app/store/service/tree_test.go | 14 ++--- 14 files changed, 123 insertions(+), 149 deletions(-) diff --git a/backend/_example/memory_store/accessor/data_test.go b/backend/_example/memory_store/accessor/data_test.go index 1695ce3a..9fc84718 100644 --- a/backend/_example/memory_store/accessor/data_test.go +++ b/backend/_example/memory_store/accessor/data_test.go @@ -30,7 +30,7 @@ func TestMemData_CreateAndFind(t *testing.T) { assert.Equal(t, "user1", res[0].User.ID) _, err = m.Create(store.Comment{ID: res[0].ID, Locator: store.Locator{URL: "https://radio-t.com", SiteID: "radio-t"}}) - require.NotNil(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "dup key") id, err := m.Create(store.Comment{ID: "id-3", Locator: store.Locator{URL: "https://radio-t2.com", SiteID: "radio-t2"}}) @@ -58,7 +58,7 @@ func TestMemData_CreateFailedReadOnly(t *testing.T) { assert.Equal(t, true, v) _, err = b.Create(comment) - assert.NotNil(t, err) + assert.Error(t, err) assert.Equal(t, "post https://radio-t.com/ro is read-only", err.Error()) flagReq = engine.FlagRequest{Locator: comment.Locator, Flag: engine.ReadOnly, Update: engine.FlagFalse} @@ -317,11 +317,11 @@ func TestMemData_InfoPost(t *testing.T) { req = engine.InfoRequest{Locator: store.Locator{URL: "https://radio-t.com/error", SiteID: "radio-t"}, ReadOnlyAge: 0} _, err = b.Info(req) - require.NotNil(t, err) + require.Error(t, err) req = engine.InfoRequest{Locator: store.Locator{URL: "https://radio-t.com", SiteID: "radio-t-error"}, ReadOnlyAge: 0} _, err = b.Info(req) - require.NotNil(t, err) + require.Error(t, err) _, err = b.Info(engine.InfoRequest{}) require.Error(t, err) @@ -605,7 +605,7 @@ func TestMemData_DeleteComment(t *testing.T) { delReq.CommentID = "123456" err = b.Delete(delReq) - assert.NotNil(t, err) + assert.Error(t, err) delReq.Locator.SiteID = "bad" delReq.CommentID = res[0].ID diff --git a/backend/app/cmd/backup_test.go b/backend/app/cmd/backup_test.go index 612db960..60a8bf7e 100644 --- a/backend/app/cmd/backup_test.go +++ b/backend/app/cmd/backup_test.go @@ -25,13 +25,13 @@ func TestBackup_Execute(t *testing.T) { cmd.SetCommon(CommonOpts{RemarkURL: ts.URL, SharedSecret: "123456"}) p := flags.NewParser(&cmd, flags.Default) _, err := p.ParseArgs([]string{"--site=remark", "--path=/tmp", "--file={{.SITE}}-test.export", "--admin-passwd=secret"}) - require.Nil(t, err) + require.NoError(t, err) err = cmd.Execute(nil) assert.NoError(t, err) defer os.Remove("/tmp/remark-test.export") data, err := ioutil.ReadFile("/tmp/remark-test.export") - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, "blah\nblah2\n12345678\n", string(data)) } @@ -49,7 +49,7 @@ func TestBackup_ExecuteFailedStatus(t *testing.T) { p := flags.NewParser(&cmd, flags.Default) _, err := p.ParseArgs([]string{"--site=remark", "--path=/tmp", "--file={{.SITE}}-test.export", "--admin-passwd=secret"}) - require.Nil(t, err) + require.NoError(t, err) err = cmd.Execute(nil) assert.EqualError(t, err, `error response "400 Bad Request", some error`) } @@ -68,7 +68,7 @@ func TestBackup_ExecuteFailedWrite(t *testing.T) { p := flags.NewParser(&cmd, flags.Default) _, err := p.ParseArgs([]string{"--site=remark", "--path=/tmp", "--file=/tmp/no-such-dir/{{.SITE}}-test.export", "--admin-passwd=secret"}) - require.Nil(t, err) + require.NoError(t, err) err = cmd.Execute(nil) assert.EqualError(t, err, `can't create backup file /tmp/no-such-dir/remark-test.export: open /tmp/no-such-dir/remark-test.export: no such file or directory`) } diff --git a/backend/app/cmd/server_test.go b/backend/app/cmd/server_test.go index aad9be0a..98028945 100644 --- a/backend/app/cmd/server_test.go +++ b/backend/app/cmd/server_test.go @@ -17,7 +17,6 @@ import ( "github.com/dgrijalva/jwt-go" "github.com/go-pkgz/auth/token" - log "github.com/go-pkgz/lgr" "github.com/jessevdk/go-flags" "github.com/stretchr/testify/assert" @@ -26,7 +25,7 @@ import ( func TestServerApp(t *testing.T) { port := chooseRandomUnusedPort() - app, ctx := prepServerApp(t, 1500*time.Millisecond, func(o ServerCommand) ServerCommand { + app, ctx, cancel := prepServerApp(t, func(o ServerCommand) ServerCommand { o.Port = port return o }) @@ -60,12 +59,13 @@ func TestServerApp(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "admin@demo.remark42.com", email, "default admin email") + cancel() app.Wait() } func TestServerApp_DevMode(t *testing.T) { port := chooseRandomUnusedPort() - app, ctx := prepServerApp(t, 500*time.Millisecond, func(o ServerCommand) ServerCommand { + app, ctx, cancel := prepServerApp(t, func(o ServerCommand) ServerCommand { o.Port = port o.AdminPasswd = "password" o.Auth.Dev = true @@ -86,12 +86,13 @@ func TestServerApp_DevMode(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "pong", string(body)) + cancel() app.Wait() } func TestServerApp_AnonMode(t *testing.T) { port := chooseRandomUnusedPort() - app, ctx := prepServerApp(t, 1000*time.Millisecond, func(o ServerCommand) ServerCommand { + app, ctx, cancel := prepServerApp(t, func(o ServerCommand) ServerCommand { o.Port = port o.Auth.Anonymous = true return o @@ -130,6 +131,7 @@ func TestServerApp_AnonMode(t *testing.T) { defer resp.Body.Close() assert.Equal(t, http.StatusForbidden, resp.StatusCode) + cancel() app.Wait() } @@ -152,12 +154,6 @@ func TestServerApp_WithSSL(t *testing.T) { require.NoError(t, err) ctx, cancel := context.WithCancel(context.Background()) - done := make(chan struct{}) - go func() { - <-done - log.Print("[TEST] terminate app") - cancel() - }() go func() { _ = app.run(ctx) }() waitForHTTPSServerStart(sslPort) @@ -189,7 +185,7 @@ func TestServerApp_WithSSL(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "pong", string(body)) - close(done) + cancel() app.Wait() } @@ -213,12 +209,6 @@ func TestServerApp_WithRemote(t *testing.T) { require.NoError(t, err) ctx, cancel := context.WithCancel(context.Background()) - done := make(chan struct{}) - go func() { - <-done - log.Print("[TEST] terminate app") - cancel() - }() go func() { _ = app.run(ctx) }() waitForHTTPServerStart(port) @@ -231,7 +221,7 @@ func TestServerApp_WithRemote(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "pong", string(body)) - close(done) + cancel() app.Wait() } @@ -281,14 +271,17 @@ func TestServerApp_Failed(t *testing.T) { } func TestServerApp_Shutdown(t *testing.T) { - app, ctx := prepServerApp(t, 500*time.Millisecond, func(o ServerCommand) ServerCommand { + app, ctx, cancel := prepServerApp(t, func(o ServerCommand) ServerCommand { o.Port = chooseRandomUnusedPort() return o }) + time.AfterFunc(100*time.Millisecond, func() { + cancel() + }) st := time.Now() err := app.run(ctx) assert.NoError(t, err) - assert.True(t, time.Since(st).Seconds() < 1, "should take about 500msec") + assert.True(t, time.Since(st).Seconds() < 1, "should take about 100msec") app.Wait() } @@ -360,7 +353,7 @@ func Test_ACMEEmail(t *testing.T) { func TestServerAuthHooks(t *testing.T) { port := chooseRandomUnusedPort() - app, ctx := prepServerApp(t, 5*time.Second, func(o ServerCommand) ServerCommand { + app, ctx, cancel := prepServerApp(t, func(o ServerCommand) ServerCommand { o.Port = port return o }) @@ -418,12 +411,12 @@ func TestServerAuthHooks(t *testing.T) { assert.Equal(t, http.StatusUnauthorized, resp.StatusCode, "user without aud claim rejected, \n"+tkNoAud+"\n"+string(body)) // block user dev as admin - req, e := http.NewRequest(http.MethodPut, + req, err = http.NewRequest(http.MethodPut, fmt.Sprintf("http://localhost:%d/api/v1/admin/user/dev?site=remark&block=1&ttl=10d", port), nil) - assert.Nil(t, e) + assert.NoError(t, err) req.SetBasicAuth("admin", "password") - resp, e = client.Do(req) - require.Nil(t, e) + resp, err = client.Do(req) + require.NoError(t, err) defer resp.Body.Close() assert.Equal(t, http.StatusOK, resp.StatusCode, "user dev blocked") b, err := ioutil.ReadAll(resp.Body) @@ -443,6 +436,7 @@ func TestServerAuthHooks(t *testing.T) { assert.True(t, resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusUnauthorized, "blocked user can't post, \n"+tk+"\n"+string(body)) + cancel() app.Wait() } @@ -496,7 +490,7 @@ func waitForHTTPSServerStart(port int) { } } -func prepServerApp(t *testing.T, duration time.Duration, fn func(o ServerCommand) ServerCommand) (*serverApp, context.Context) { +func prepServerApp(t *testing.T, fn func(o ServerCommand) ServerCommand) (*serverApp, context.Context, context.CancelFunc) { cmd := ServerCommand{} cmd.SetCommon(CommonOpts{RemarkURL: "https://demo.remark42.com", SharedSecret: "secret"}) @@ -533,10 +527,6 @@ func prepServerApp(t *testing.T, duration time.Duration, fn func(o ServerCommand require.NoError(t, err) ctx, cancel := context.WithCancel(context.Background()) - time.AfterFunc(duration, func() { - log.Print("[TEST] terminate app") - cancel() - }) rand.Seed(time.Now().UnixNano()) - return app, ctx + return app, ctx, cancel } diff --git a/backend/app/main_test.go b/backend/app/main_test.go index 3e70c54d..4b739183 100644 --- a/backend/app/main_test.go +++ b/backend/app/main_test.go @@ -32,8 +32,8 @@ func Test_Main(t *testing.T) { done := make(chan struct{}) go func() { <-done - e := syscall.Kill(syscall.Getpid(), syscall.SIGTERM) - require.Nil(t, e) + err := syscall.Kill(syscall.Getpid(), syscall.SIGTERM) + require.NoError(t, err) }() wg := sync.WaitGroup{} @@ -76,10 +76,10 @@ func chooseRandomUnusedPort() (port int) { } func waitForHTTPServerStart(port int) { - // wait for up to 5 seconds for server to start before returning it + // wait for up to 10 seconds for server to start before returning it client := http.Client{Timeout: time.Second} - for i := 0; i < 500; i++ { - time.Sleep(time.Millisecond * 10) + for i := 0; i < 100; i++ { + time.Sleep(time.Millisecond * 100) if resp, err := client.Get(fmt.Sprintf("http://localhost:%d", port)); err == nil { _ = resp.Body.Close() return diff --git a/backend/app/notify/email_test.go b/backend/app/notify/email_test.go index adfbd3ad..e9d080d7 100644 --- a/backend/app/notify/email_test.go +++ b/backend/app/notify/email_test.go @@ -130,10 +130,10 @@ func TestEmailSend_ExitConditions(t *testing.T) { assert.NotNil(t, email, "expecting email returned") // prevent triggering e.autoFlush creation emptyRequest := Request{Comment: store.Comment{ID: "999"}} - assert.Nil(t, email.Send(context.Background(), emptyRequest), + assert.NoError(t, email.Send(context.Background(), emptyRequest), "Message without parent comment User.Email is not sent and returns nil") requestWithEqualUsersWithEmails := Request{Comment: store.Comment{ID: "999"}, Email: "good_example@example.org"} - assert.Nil(t, email.Send(context.Background(), requestWithEqualUsersWithEmails), + assert.NoError(t, email.Send(context.Background(), requestWithEqualUsersWithEmails), "Message with parent comment User equals comment User is not sent and returns nil") } @@ -179,23 +179,23 @@ func TestEmailSendClientError(t *testing.T) { } func TestEmail_Send(t *testing.T) { - e, err := NewEmail(EmailParams{From: "from@example.org"}, SmtpParams{}) + email, err := NewEmail(EmailParams{From: "from@example.org"}, SmtpParams{}) assert.NoError(t, err) - assert.NotNil(t, e) + assert.NotNil(t, email) fakeSmtp := fakeTestSMTP{} - e.smtp = &fakeSmtp - e.TokenGenFn = TokenGenFn - e.UnsubscribeURL = "https://remark42.com/api/v1/email/unsubscribe" + email.smtp = &fakeSmtp + email.TokenGenFn = TokenGenFn + email.UnsubscribeURL = "https://remark42.com/api/v1/email/unsubscribe" req := Request{ Comment: store.Comment{ID: "999", User: store.User{Name: "test_user"}, PostTitle: "test_title"}, parent: store.Comment{ID: "1", User: store.User{Name: "parent_user"}}, Email: "test@example.org"} - assert.NoError(t, e.Send(context.TODO(), req)) + assert.NoError(t, email.Send(context.TODO(), req)) assert.Equal(t, "from@example.org", fakeSmtp.readMail()) assert.Equal(t, 1, fakeSmtp.readQuitCount()) assert.Equal(t, "test@example.org", fakeSmtp.readRcpt()) // test buildMessageFromRequest separately for message text - res, err := e.buildMessageFromRequest(req) + res, err := email.buildMessageFromRequest(req) assert.NoError(t, err) assert.Contains(t, res, `From: from@example.org To: test@example.org diff --git a/backend/app/notify/telegram_test.go b/backend/app/notify/telegram_test.go index 28cc70b5..f29a22a4 100644 --- a/backend/app/notify/telegram_test.go +++ b/backend/app/notify/telegram_test.go @@ -76,7 +76,7 @@ func TestTelegram_Send(t *testing.T) { assert.Contains(t, err.Error(), "unexpected telegram status code 404", "send on broken tg") assert.Equal(t, "telegram: @remark_test", tb.String()) - require.Nil(t, tb.Send(context.TODO(), Request{}), "Empty Comment doesn't send anything") + require.NoError(t, tb.Send(context.TODO(), Request{}), "Empty Comment doesn't send anything") } func mockTelegramServer() *httptest.Server { diff --git a/backend/app/rest/api/admin_test.go b/backend/app/rest/api/admin_test.go index c19d6b17..69e4af1a 100644 --- a/backend/app/rest/api/admin_test.go +++ b/backend/app/rest/api/admin_test.go @@ -268,14 +268,14 @@ func TestAdmin_Block(t *testing.T) { if ttl != "" { url = url + "&ttl=" + ttl } - req, e := http.NewRequest(http.MethodPut, url, nil) - assert.Nil(t, e) + req, err := http.NewRequest(http.MethodPut, url, nil) + assert.NoError(t, err) requireAdminOnly(t, req) - resp, e := sendReq(t, req, adminUmputunToken) - require.Nil(t, e) - body, e = ioutil.ReadAll(resp.Body) - assert.Nil(t, e) - require.Nil(t, resp.Body.Close()) + resp, err := sendReq(t, req, adminUmputunToken) + require.NoError(t, err) + body, err = ioutil.ReadAll(resp.Body) + assert.NoError(t, err) + require.NoError(t, resp.Body.Close()) return resp.StatusCode, body } diff --git a/backend/app/rest/api/rest_private_test.go b/backend/app/rest/api/rest_private_test.go index 7e5963d8..5647c88b 100644 --- a/backend/app/rest/api/rest_private_test.go +++ b/backend/app/rest/api/rest_private_test.go @@ -44,7 +44,6 @@ func TestRest_Create(t *testing.T) { assert.NoError(t, err) require.Equal(t, http.StatusCreated, resp.StatusCode, string(b)) - t.Log(string(b)) c := R.JSON{} err = json.Unmarshal(b, &c) assert.NoError(t, err) @@ -74,7 +73,7 @@ func TestRest_CreateOldPost(t *testing.T) { assert.NoError(t, err) assert.Equal(t, http.StatusCreated, resp.StatusCode) - assert.Nil(t, srv.DataService.DeleteAll("remark42")) + assert.NoError(t, srv.DataService.DeleteAll("remark42")) // make too old comment old = store.Comment{Text: "test test old", ParentID: "", Timestamp: time.Now().AddDate(0, 0, -15), Locator: store.Locator{SiteID: "remark42", URL: "https://radio-t.com/blah1"}, User: store.User{ID: "u1"}} @@ -185,7 +184,6 @@ func TestRest_CreateAndGet(t *testing.T) { assert.Equal(t, store.User{Name: "admin", ID: "admin", Admin: true, Blocked: false, IP: "dbc7c999343f003f189f70aaf52cc04443f90790"}, comment.User) - t.Logf("%+v", comment) // get created comment by id as non-admin res, code = getWithDevAuth(t, fmt.Sprintf("%s/api/v1/id/%s?site=remark42&url=https://radio-t.com/blah1", ts.URL, id)) @@ -730,7 +728,6 @@ func TestRest_UserAllData(t *testing.T) { assert.True(t, strings.HasPrefix(strUungzBody, `{"info": {"name":"developer one","id":"dev","picture":"http://example.com/pic.png","ip":"127.0.0.1","admin":false,"site_id":"remark42"}, "comments":[{`)) assert.Equal(t, 3, strings.Count(strUungzBody, `"text":`), "3 comments inside") - t.Logf("%s", strUungzBody) parsed := struct { Info store.User `json:"info"` @@ -895,13 +892,12 @@ func TestRest_CreateWithPictures(t *testing.T) { Location: "/tmp/remark42/images", MaxSize: 2000, } - imageService.TTL = 300 * time.Millisecond + imageService.TTL = 50 * time.Millisecond svc.privRest.imageService = imageService svc.ImageService = imageService dataService := svc.DataService - dataService.EditDuration = time.Millisecond * 300 dataService.ImageService = svc.ImageService svc.privRest.dataService = dataService @@ -929,15 +925,16 @@ func TestRest_CreateWithPictures(t *testing.T) { err = json.Unmarshal(body, &m) assert.NoError(t, err) assert.Contains(t, m["id"], ".png") - t.Logf(string(body)) return m["id"] } - id1 := uploadPicture("pic1.png") - id2 := uploadPicture("pic2.png") - id3 := uploadPicture("pic3.png") + var ids [3]string - text := fmt.Sprintf(`text 123 ![](/api/v1/picture/%s) *xxx* ![](/api/v1/picture/%s) ![](/api/v1/picture/%s)`, id1, id2, id3) + for i := range ids { + ids[i] = uploadPicture(fmt.Sprintf("pic%d.png", i)) + } + + text := fmt.Sprintf(`text 123 ![](/api/v1/picture/%s) *xxx* ![](/api/v1/picture/%s) ![](/api/v1/picture/%s)`, ids[0], ids[1], ids[2]) body := fmt.Sprintf(`{"text": "%s", "locator":{"url": "https://radio-t.com/blah1", "site": "remark42"}}`, text) resp, err := post(t, ts.URL+"/api/v1/comment", body) @@ -946,14 +943,15 @@ func TestRest_CreateWithPictures(t *testing.T) { assert.NoError(t, err) require.Equal(t, http.StatusCreated, resp.StatusCode, string(b)) - _, err = os.Stat("/tmp/remark42/images/" + id1) - assert.Error(t, err, "not moved from staging yet") + for i := range ids { + _, err = os.Stat("/tmp/remark42/images/" + ids[i]) + assert.Error(t, err, "picture %d not moved from staging yet", i) + } time.Sleep(500 * time.Millisecond) - _, err = os.Stat("/tmp/remark42/images/" + id1) - assert.NoError(t, err, "moved from staging") - _, err = os.Stat("/tmp/remark42/images/" + id2) - assert.NoError(t, err, "moved from staging") - _, err = os.Stat("/tmp/remark42/images/" + id3) - assert.NoError(t, err, "moved from staging") + + for i := range ids { + _, err = os.Stat("/tmp/remark42/images/" + ids[i]) + assert.NoError(t, err, "picture %d moved from staging and available in permanent location", i) + } } diff --git a/backend/app/rest/api/rest_public_test.go b/backend/app/rest/api/rest_public_test.go index 2727ecf2..60bf0569 100644 --- a/backend/app/rest/api/rest_public_test.go +++ b/backend/app/rest/api/rest_public_test.go @@ -13,7 +13,6 @@ import ( "time" cache "github.com/go-pkgz/lcw" - log "github.com/go-pkgz/lgr" R "github.com/go-pkgz/rest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -64,7 +63,6 @@ BKT text = strings.Replace(text, "BKT", "```", -1) j := fmt.Sprintf(`{"text": "%s", "locator":{"url": "https://radio-t.com/blah1", "site": "radio-t"}}`, text) j = strings.Replace(j, "\n", "\\n", -1) - t.Log(j) resp, err := post(t, ts.URL+"/api/v1/preview", j) assert.NoError(t, err) @@ -308,7 +306,6 @@ func TestRest_Last(t *testing.T) { err = json.Unmarshal([]byte(res), &comments) assert.NoError(t, err) assert.Equal(t, 2, len(comments), "should have 2 comments") - t.Logf("%+v", comments) _, code = get(t, ts.URL+"/api/v1/last/2?site=remark42-BLAH") assert.Equal(t, 500, code) @@ -511,7 +508,6 @@ func TestRest_Config(t *testing.T) { assert.Equal(t, 10., j["readonly_age"]) assert.Equal(t, 10000., j["max_image_size"]) assert.Equal(t, true, j["emoji_enabled"].(bool)) - t.Logf("%+v", j) } func TestRest_Info(t *testing.T) { @@ -575,7 +571,6 @@ func TestRest_InfoStream(t *testing.T) { assert.Equal(t, 200, code) wg.Wait() - t.Logf(string(body)) recs := strings.Split(strings.TrimSuffix(string(body), "\n"), "\n") require.Equal(t, 10*3, len(recs), "10 records. each 2 lines +1 emty line") assert.True(t, strings.Contains(recs[0+1], `"count":2`), recs[0]) @@ -642,21 +637,21 @@ func TestRest_InfoStreamCancel(t *testing.T) { go func() { defer wg.Done() for i := 0; i < 5; i++ { - time.Sleep(200 * time.Millisecond) + time.Sleep(300 * time.Millisecond) postComment(t, ts.URL) - log.Printf("write #%d", i) } }() 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(), 350*time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) defer cancel() req = req.WithContext(ctx) r, err := client.Do(req) require.NoError(t, err) defer r.Body.Close() + <-ctx.Done() body, err := ioutil.ReadAll(r.Body) require.EqualError(t, err, "context deadline exceeded") assert.Equal(t, 200, r.StatusCode) @@ -664,8 +659,7 @@ func TestRest_InfoStreamCancel(t *testing.T) { wg.Wait() recs := strings.Count(string(body), "data:") - t.Logf("%s", string(body)) - require.Equal(t, 1, recs, "should have 1 events") + require.Equal(t, 1, recs, "should have 1 event:\n", string(body)) assert.Contains(t, string(body), `"count":2`) } @@ -684,7 +678,7 @@ func TestRest_InfoStreamSince(t *testing.T) { go func() { defer wg.Done() for i := 0; i < 10; i++ { - time.Sleep(10 * time.Millisecond) + time.Sleep(15 * time.Millisecond) postComment(t, ts.URL) } }() @@ -692,9 +686,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() - - t.Logf(string(body)) - recs := strings.Split(strings.TrimSuffix(string(body), "\n"), "\n") + 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") } @@ -740,13 +732,11 @@ func TestRest_LastCommentsStream(t *testing.T) { assert.Equal(t, 200, r.StatusCode) wg.Wait() - t.Logf("headers: %+v", r.Header) assert.Equal(t, "text/event-stream", r.Header.Get("content-type")) assert.Equal(t, "keep-alive", r.Header.Get("connection")) recs := strings.Split(strings.TrimSuffix(string(body), "\n"), "\n") require.Equal(t, 9*3, len(recs), "9 events") - t.Logf("%s", string(body)) assert.True(t, strings.Contains(recs[1], `test 123`), recs[1]) } @@ -865,19 +855,17 @@ func TestRest_LastCommentsStreamSince(t *testing.T) { assert.Equal(t, 200, r.StatusCode) wg.Wait() - t.Logf("headers: %+v", r.Header) 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") - t.Logf("%v", recs) } func postComment(t *testing.T, url string) { - resp, e := post(t, url+"/api/v1/comment", + resp, err := post(t, url+"/api/v1/comment", `{"text": "test 123", "locator":{"url": "https://radio-t.com/blah1", "site": "remark42"}}`) - require.Nil(t, e) - b, e := ioutil.ReadAll(resp.Body) - require.Nil(t, e) + require.NoError(t, err) + b, err := ioutil.ReadAll(resp.Body) + require.NoError(t, err) require.Equal(t, http.StatusCreated, resp.StatusCode, string(b)) } diff --git a/backend/app/rest/api/rest_test.go b/backend/app/rest/api/rest_test.go index a71b0375..94b3ff6d 100644 --- a/backend/app/rest/api/rest_test.go +++ b/backend/app/rest/api/rest_test.go @@ -109,6 +109,7 @@ func TestRest_filterComments(t *testing.T) { } func TestRest_RunStaticSSLMode(t *testing.T) { + sslPort := chooseRandomUnusedPort() srv := Rest{ Authenticator: auth.NewService(auth.Opts{ AvatarStore: avatar.NewLocalFS("/tmp"), @@ -118,11 +119,11 @@ func TestRest_RunStaticSSLMode(t *testing.T) { ImageProxy: &proxy.Image{}, SSLConfig: SSLConfig{ SSLMode: Static, - Port: 8443, + Port: sslPort, Key: "../../cmd/testdata/key.pem", Cert: "../../cmd/testdata/cert.pem", }, - RemarkURL: "https://localhost:8443", + RemarkURL: fmt.Sprintf("https://localhost:%d", sslPort), } port := chooseRandomUnusedPort() @@ -130,7 +131,7 @@ func TestRest_RunStaticSSLMode(t *testing.T) { srv.Run(port) }() - waitForHTTPServerStart(port) + waitForHTTPSServerStart(sslPort) client := http.Client{ // prevent http redirect @@ -148,9 +149,9 @@ func TestRest_RunStaticSSLMode(t *testing.T) { require.NoError(t, err) defer resp.Body.Close() assert.Equal(t, 307, resp.StatusCode) - assert.Equal(t, "https://localhost:8443/blah?param=1", resp.Header.Get("Location")) + assert.Equal(t, fmt.Sprintf("https://localhost:%d/blah?param=1", sslPort), resp.Header.Get("Location")) - resp, err = client.Get("https://localhost:8443/ping") + resp, err = client.Get(fmt.Sprintf("https://localhost:%d/ping", sslPort)) require.NoError(t, err) defer resp.Body.Close() assert.Equal(t, 200, resp.StatusCode) @@ -162,14 +163,15 @@ func TestRest_RunStaticSSLMode(t *testing.T) { } func TestRest_RunAutocertModeHTTPOnly(t *testing.T) { + sslPort := chooseRandomUnusedPort() srv := Rest{ Authenticator: &auth.Service{}, ImageProxy: &proxy.Image{}, SSLConfig: SSLConfig{ SSLMode: Auto, - Port: 8443, + Port: sslPort, }, - RemarkURL: "https://localhost:8443", + RemarkURL: fmt.Sprintf("https://localhost:%d", sslPort), } port := chooseRandomUnusedPort() @@ -178,7 +180,7 @@ func TestRest_RunAutocertModeHTTPOnly(t *testing.T) { srv.Run(port) }() - waitForHTTPServerStart(port) + waitForHTTPSServerStart(sslPort) client := http.Client{ // prevent http redirect @@ -191,7 +193,7 @@ func TestRest_RunAutocertModeHTTPOnly(t *testing.T) { require.NoError(t, err) defer resp.Body.Close() assert.Equal(t, 307, resp.StatusCode) - assert.Equal(t, "https://localhost:8443/blah?param=1", resp.Header.Get("Location")) + assert.Equal(t, fmt.Sprintf("https://localhost:%d/blah?param=1", sslPort), resp.Header.Get("Location")) srv.Shutdown() } @@ -480,14 +482,14 @@ func chooseRandomUnusedPort() (port int) { return port } -func waitForHTTPServerStart(port int) { - // wait for up to 3 seconds for server to start before returning it - client := http.Client{Timeout: time.Second} +func waitForHTTPSServerStart(port int) { + // wait for up to 3 seconds for HTTPS server to start for i := 0; i < 300; i++ { time.Sleep(time.Millisecond * 10) - if resp, err := client.Get(fmt.Sprintf("http://localhost:%d", port)); err == nil { - _ = resp.Body.Close() - return + conn, _ := net.DialTimeout("tcp", fmt.Sprintf("localhost:%d", port), time.Millisecond*10) + if conn != nil { + _ = conn.Close() + break } } } diff --git a/backend/app/rest/httperrors_test.go b/backend/app/rest/httperrors_test.go index adf0ad98..59eb3c78 100644 --- a/backend/app/rest/httperrors_test.go +++ b/backend/app/rest/httperrors_test.go @@ -27,11 +27,11 @@ func TestSendErrorJSON(t *testing.T) { defer ts.Close() resp, err := http.Get(ts.URL + "/error") - require.Nil(t, err) + require.NoError(t, err) defer resp.Body.Close() body, err := ioutil.ReadAll(resp.Body) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, 500, resp.StatusCode) assert.Equal(t, `{"code":123,"details":"error details 123456","error":"error 500"}`+"\n", string(body)) @@ -51,11 +51,11 @@ func TestSendErrorHTML(t *testing.T) { defer ts.Close() resp, err := http.Get(ts.URL + "/error") - require.Nil(t, err) + require.NoError(t, err) defer resp.Body.Close() body, err := ioutil.ReadAll(resp.Body) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, 500, resp.StatusCode) assert.NotContains(t, string(body), `987`, "user html should not contain internal error code") @@ -66,7 +66,7 @@ func TestSendErrorHTML(t *testing.T) { func TestErrorDetailsMsg(t *testing.T) { callerFn := func() { req, err := http.NewRequest("GET", "https://example.com/test?k1=v1&k2=v2", nil) - require.Nil(t, err) + require.NoError(t, err) req.RemoteAddr = "1.2.3.4" msg := errDetailsMsg(req, 500, errors.New("error 500"), "error details 123456", 123) assert.Contains(t, msg, "error details 123456 - error 500 - 500 (123) - https://example.com/test?k1=v1&k2=v2 - [app/rest/httperrors_test.go:") @@ -82,7 +82,7 @@ func TestErrorDetailsMsgWithUser(t *testing.T) { require.NoError(t, err) req.RemoteAddr = "127.0.0.1:1234" req = SetUserInfo(req, store.User{Name: "test", ID: "id"}) - require.Nil(t, err) + require.NoError(t, err) msg := errDetailsMsg(req, 500, errors.New("error 500"), "error details 123456", 34567) assert.Contains(t, msg, "error details 123456 - error 500 - 500 (34567) - test/id - https://example.com/test?k1=v1&k2=v2 - [app/rest/httperrors_test.go:") // error line in the middle of the message is not checked diff --git a/backend/app/store/service/service_test.go b/backend/app/store/service/service_test.go index 17fcbfed..8f43f769 100644 --- a/backend/app/store/service/service_test.go +++ b/backend/app/store/service/service_test.go @@ -663,13 +663,13 @@ func TestService_ValidateComment(t *testing.T) { } for n, tt := range tbl { - e := b.ValidateComment(&tt.inp) + err := b.ValidateComment(&tt.inp) if tt.err == nil { - assert.NoError(t, e, "check #%d", n) + assert.NoError(t, err, "check #%d", n) continue } - require.NotNil(t, e) - assert.EqualError(t, tt.err, e.Error(), "check #%d", n) + require.Error(t, err) + assert.EqualError(t, tt.err, err.Error(), "check #%d", n) } } @@ -886,7 +886,6 @@ func TestService_UserReplies(t *testing.T) { Locator: store.Locator{URL: "https://radio-t.com/blah10", SiteID: "radio-t"}, User: store.User{ID: "u4", Name: "developer one u4"}, } - c5 := store.Comment{ ID: "comment-id-5", ParentID: "comment-id-1", @@ -905,9 +904,7 @@ func TestService_UserReplies(t *testing.T) { require.NoError(t, err) time.Sleep(200 * time.Millisecond) - st := time.Now() _, err = b.Create(c5) - t.Logf("time to create a record %v", time.Since(st)) require.NoError(t, err) cc, u, err := b.UserReplies("radio-t", "u1", 10, time.Hour) @@ -915,10 +912,9 @@ func TestService_UserReplies(t *testing.T) { assert.Equal(t, 3, len(cc), "3 replies to u1") assert.Equal(t, "developer one u1", u) - t.Logf("elpased %v", time.Since(st)) - cc, u, err = b.UserReplies("radio-t", "u1", 10, time.Millisecond*100) + cc, u, err = b.UserReplies("radio-t", "u1", 10, time.Millisecond*150) assert.NoError(t, err) - assert.Equal(t, 1, len(cc), "1 reply to u1 in last 90ms") + assert.Equal(t, 1, len(cc), "1 reply to u1 in last 150ms") assert.Equal(t, "developer one u1", u) cc, u, err = b.UserReplies("radio-t", "u2", 10, time.Hour) diff --git a/backend/app/store/service/title_test.go b/backend/app/store/service/title_test.go index 16a31515..9d1140ae 100644 --- a/backend/app/store/service/title_test.go +++ b/backend/app/store/service/title_test.go @@ -53,15 +53,15 @@ func TestTitle_Get(t *testing.T) { })) title, err := ex.Get(ts.URL + "/good") - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, "blah 123", title) _, err = ex.Get(ts.URL + "/bad") - require.NotNil(t, err) + require.Error(t, err) for i := 0; i < 100; i++ { - r, e := ex.Get(ts.URL + "/good") - require.Nil(t, e) + r, err := ex.Get(ts.URL + "/good") + require.NoError(t, err) assert.Equal(t, "blah 123", r) } assert.Equal(t, int32(1), atomic.LoadInt32(&hits)) @@ -90,7 +90,7 @@ func TestTitle_GetConcurrent(t *testing.T) { ii := i g.Go(func(_ context.Context) { title, err := ex.Get(ts.URL + "/good/" + strconv.Itoa(ii)) - require.Nil(t, err) + require.NoError(t, err) assert.Equal(t, "blah 123 "+"/good/"+strconv.Itoa(ii), title) }) } @@ -107,11 +107,11 @@ func TestTitle_GetFailed(t *testing.T) { })) _, err := ex.Get(ts.URL + "/bad") - require.NotNil(t, err) + require.Error(t, err) for i := 0; i < 100; i++ { - r, e := ex.Get(ts.URL + "/bad") - require.Nil(t, e) + r, err := ex.Get(ts.URL + "/bad") + require.NoError(t, err) assert.Equal(t, "", r) } assert.Equal(t, int32(1), atomic.LoadInt32(&hits), "hit once, errors cached") diff --git a/backend/app/store/service/tree_test.go b/backend/app/store/service/tree_test.go index 9df9c9b7..921d3541 100644 --- a/backend/app/store/service/tree_test.go +++ b/backend/app/store/service/tree_test.go @@ -41,7 +41,7 @@ func TestMakeTree(t *testing.T) { res := MakeTree(comments, "time", 0) resJSON, err := json.Marshal(&res) - require.Nil(t, err) + require.NoError(t, err) expJSON := mustLoadJSONFile(t, "testdata/tree.json") assert.Equal(t, expJSON, resJSON) @@ -83,7 +83,7 @@ func TestMakeEmptySubtree(t *testing.T) { res := MakeTree(comments, "time", 0) resJSON, err := json.Marshal(&res) - require.Nil(t, err) + require.NoError(t, err) log.Print(string(resJSON)) expJSON := mustLoadJSONFile(t, "testdata/tree_del.json") @@ -162,9 +162,9 @@ func TestTreeSortNodes(t *testing.T) { func BenchmarkTree(b *testing.B) { comments := []store.Comment{} data, err := ioutil.ReadFile("testdata/tree_bench.json") - assert.Nil(b, err) + assert.NoError(b, err) err = json.Unmarshal(data, &comments) - assert.Nil(b, err) + assert.NoError(b, err) for i := 0; i < b.N; i++ { res := MakeTree(comments, "time", 0) @@ -175,11 +175,11 @@ func BenchmarkTree(b *testing.B) { // loadJsonFile read fixtrue file and clear any custom json formatting func mustLoadJSONFile(t *testing.T, file string) []byte { expJSON, err := ioutil.ReadFile(file) - require.Nil(t, err) + require.NoError(t, err) expTree := Tree{} err = json.Unmarshal(expJSON, &expTree) - require.Nil(t, err) + require.NoError(t, err) expJSON, err = json.Marshal(expTree) - require.Nil(t, err) + require.NoError(t, err) return expJSON }