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

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

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`)
 }

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
 }

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

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

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 {

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
 	}
 

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)
+	}
 }

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))
 }

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
 		}
 	}
 }

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

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)

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")

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
 }