From 62cc504600699f29bcdd9c89cc54b23b647c6f75 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Mon, 30 Dec 2019 10:44:10 +0100 Subject: [PATCH] Improve Test_Main reliability (#509) * adjust TestServerApp_WithSSL to use sslPort in all test checks * make Test_Main reliable and remove 5s sleep * make test finishing reliable using "done" channel for TestServerApp* --- backend/app/cmd/server_test.go | 14 +++++--- backend/app/main_test.go | 64 +++++++++++++++++++++------------- 2 files changed, 48 insertions(+), 30 deletions(-) diff --git a/backend/app/cmd/server_test.go b/backend/app/cmd/server_test.go index 321b84bc..aad9be0a 100644 --- a/backend/app/cmd/server_test.go +++ b/backend/app/cmd/server_test.go @@ -135,12 +135,12 @@ func TestServerApp_AnonMode(t *testing.T) { func TestServerApp_WithSSL(t *testing.T) { opts := ServerCommand{} - opts.SetCommon(CommonOpts{RemarkURL: "https://localhost:18443", SharedSecret: "123456"}) + sslPort := chooseRandomUnusedPort() + opts.SetCommon(CommonOpts{RemarkURL: fmt.Sprintf("https://localhost:%d", sslPort), SharedSecret: "123456"}) // prepare options p := flags.NewParser(&opts, flags.Default) port := chooseRandomUnusedPort() - sslPort := chooseRandomUnusedPort() _, err := p.ParseArgs([]string{"--admin-passwd=password", "--port=" + strconv.Itoa(port), "--store.bolt.path=/tmp/xyz", "--backup=/tmp", "--avatar.type=bolt", "--avatar.bolt.file=/tmp/ava-test.db", "--notify.type=none", "--ssl.type=static", "--ssl.cert=testdata/cert.pem", "--ssl.key=testdata/key.pem", @@ -152,8 +152,9 @@ func TestServerApp_WithSSL(t *testing.T) { require.NoError(t, err) ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) go func() { - time.Sleep(1 * time.Second) + <-done log.Print("[TEST] terminate app") cancel() }() @@ -177,7 +178,7 @@ func TestServerApp_WithSSL(t *testing.T) { require.NoError(t, err) defer resp.Body.Close() assert.Equal(t, 307, resp.StatusCode) - assert.Equal(t, "https://localhost:18443/blah?param=1", resp.Header.Get("Location")) + assert.Equal(t, fmt.Sprintf("https://localhost:%d/blah?param=1", sslPort), resp.Header.Get("Location")) // check https server resp, err = client.Get(fmt.Sprintf("https://localhost:%d/ping", sslPort)) @@ -188,6 +189,7 @@ func TestServerApp_WithSSL(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "pong", string(body)) + close(done) app.Wait() } @@ -211,8 +213,9 @@ func TestServerApp_WithRemote(t *testing.T) { require.NoError(t, err) ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) go func() { - time.Sleep(5 * time.Second) + <-done log.Print("[TEST] terminate app") cancel() }() @@ -228,6 +231,7 @@ func TestServerApp_WithRemote(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "pong", string(body)) + close(done) app.Wait() } diff --git a/backend/app/main_test.go b/backend/app/main_test.go index 259e2b3e..3e70c54d 100644 --- a/backend/app/main_test.go +++ b/backend/app/main_test.go @@ -1,10 +1,13 @@ package main import ( - "context" + "fmt" "io/ioutil" + "math/rand" + "net" "net/http" "os" + "strconv" "strings" "sync" "syscall" @@ -12,8 +15,6 @@ import ( "time" log "github.com/go-pkgz/lgr" - "github.com/go-pkgz/repeater" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -24,11 +25,13 @@ func Test_Main(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(dir) + port := chooseRandomUnusedPort() os.Args = []string{"test", "server", "--secret=123456", "--store.bolt.path=" + dir, "--backup=/tmp", - "--avatar.fs.path=" + dir, "--port=18222", "--url=https://demo.remark42.com", "--dbg", "--notify.type=none"} + "--avatar.fs.path=" + dir, "--port=" + strconv.Itoa(port), "--url=https://demo.remark42.com", "--dbg", "--notify.type=none"} + done := make(chan struct{}) go func() { - time.Sleep(5000 * time.Millisecond) + <-done e := syscall.Kill(syscall.Getpid(), syscall.SIGTERM) require.Nil(t, e) }() @@ -36,32 +39,20 @@ func Test_Main(t *testing.T) { wg := sync.WaitGroup{} wg.Add(1) go func() { - st := time.Now() main() - assert.True(t, time.Since(st).Seconds() >= 4, "should take about 5s, took %s", time.Since(st)) wg.Done() }() - var passed bool - err = repeater.NewDefault(10, time.Millisecond*1000).Do(context.Background(), func() error { - resp, e := http.Get("http://localhost:18222/api/v1/ping") - if e != nil { - t.Logf("%+v", e) - return e - } - require.Nil(t, e) - defer resp.Body.Close() - assert.Equal(t, 200, resp.StatusCode) - body, e := ioutil.ReadAll(resp.Body) - assert.Nil(t, e) - assert.Equal(t, "pong", string(body)) - passed = true - return nil - }) - + waitForHTTPServerStart(port) + resp, err := http.Get(fmt.Sprintf("http://localhost:%d/api/v1/ping", port)) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, 200, resp.StatusCode) + body, err := ioutil.ReadAll(resp.Body) assert.NoError(t, err) - assert.Equal(t, true, passed, "at least on ping passed") + assert.Equal(t, "pong", string(body)) + close(done) wg.Wait() } @@ -72,3 +63,26 @@ func TestGetDump(t *testing.T) { assert.True(t, strings.Contains(dump, "backend/app/main.go")) log.Printf("\n dump: %s", dump) } + +func chooseRandomUnusedPort() (port int) { + for i := 0; i < 10; i++ { + port = 40000 + int(rand.Int31n(10000)) + if ln, err := net.Listen("tcp", fmt.Sprintf(":%d", port)); err == nil { + _ = ln.Close() + break + } + } + return port +} + +func waitForHTTPServerStart(port int) { + // wait for up to 5 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) + if resp, err := client.Get(fmt.Sprintf("http://localhost:%d", port)); err == nil { + _ = resp.Body.Close() + return + } + } +}