From 0e20dc105b10e5675a2c36aea6f91dd2e1563b69 Mon Sep 17 00:00:00 2001 From: Fedor Korotkiy Date: Sun, 16 Feb 2020 16:24:43 +0300 Subject: [PATCH 1/9] Check for goroutine leaks --- batcher/batcher_test.go | 7 +++++++ go.mod | 1 + go.sum | 12 ++++++++++++ 3 files changed, 20 insertions(+) diff --git a/batcher/batcher_test.go b/batcher/batcher_test.go index 80a355d..15ca7d3 100644 --- a/batcher/batcher_test.go +++ b/batcher/batcher_test.go @@ -7,11 +7,14 @@ import ( "time" "github.com/stretchr/testify/require" + "go.uber.org/goleak" "gitlab.com/slon/shad-go/batcher/slow" ) func TestSimple(t *testing.T) { + defer goleak.VerifyNone(t) + var value slow.Value b := NewBatcher(&value) @@ -25,6 +28,8 @@ func TestSimple(t *testing.T) { } func TestStaleRead(t *testing.T) { + defer goleak.VerifyNone(t) + const ( N = 100 K = 100 @@ -69,6 +74,8 @@ func TestStaleRead(t *testing.T) { } func TestSpeed(t *testing.T) { + defer goleak.VerifyNone(t) + const ( N = 100 K = 200 diff --git a/go.mod b/go.mod index 029fc61..78dbe0c 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.13 require ( github.com/spf13/cobra v0.0.5 github.com/stretchr/testify v1.4.0 + go.uber.org/goleak v1.0.0 golang.org/x/tools v0.0.0-20200125223703-d33eef8e6825 gopkg.in/yaml.v2 v2.2.8 ) diff --git a/go.sum b/go.sum index 4094bc3..25e688a 100644 --- a/go.sum +++ b/go.sum @@ -11,6 +11,9 @@ github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMo github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= +github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= +github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= @@ -33,10 +36,15 @@ github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJy github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q= +go.uber.org/goleak v1.0.0 h1:qsup4IcBdlmsnGfqyLl4Ntn3C2XCCuKAE7DwHpScyUo= +go.uber.org/goleak v1.0.0/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs= +golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= +golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU= @@ -45,12 +53,16 @@ golang.org/x/sys v0.0.0-20181205085412-a5c9d58dba9a/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= +golang.org/x/tools v0.0.0-20191108193012-7d206e10da11/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200125223703-d33eef8e6825 h1:aNQeSIHKi0RWpKA5NO0CqyLjx6Beh5l0LLUEnndEjz0= golang.org/x/tools v0.0.0-20200125223703-d33eef8e6825/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898 h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= From 57ab50a4a7bd7dd301f3310735bb8e650f505b6d Mon Sep 17 00:00:00 2001 From: Fedor Korotkiy Date: Sun, 16 Feb 2020 17:18:26 +0300 Subject: [PATCH 2/9] Fix bincache on windows --- tools/testtool/bincache.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/testtool/bincache.go b/tools/testtool/bincache.go index 24ae2ca..aa60c01 100644 --- a/tools/testtool/bincache.go +++ b/tools/testtool/bincache.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "sync" ) @@ -107,5 +108,9 @@ func (c *ciBuildCache) GetBinary(importPath string) (string, error) { func RandomName() string { var raw [8]byte _, _ = rand.Read(raw[:]) - return hex.EncodeToString(raw[:]) + name := hex.EncodeToString(raw[:]) + if runtime.GOOS == "windows" { + name += ".exe" + } + return name } From 8f414883a77df99b8d8340e74aa50d779475e1c1 Mon Sep 17 00:00:00 2001 From: Fedor Korotkiy Date: Sun, 16 Feb 2020 17:18:51 +0300 Subject: [PATCH 3/9] Use require.NoError where appropriate --- fetchall/main_test.go | 18 +++++++++--------- .../testtool/commands/test_submission_test.go | 8 ++++---- urlfetch/main_test.go | 10 +++++----- wordcount/main_test.go | 10 +++++----- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/fetchall/main_test.go b/fetchall/main_test.go index 71c3855..2dca656 100644 --- a/fetchall/main_test.go +++ b/fetchall/main_test.go @@ -35,7 +35,7 @@ func TestMain(m *testing.M) { func TestFetchall_valid(t *testing.T) { binary, err := binCache.GetBinary(fetchallImportPath) - require.Nil(t, err) + require.NoError(t, err) type endpoint string @@ -73,14 +73,14 @@ func TestFetchall_valid(t *testing.T) { cmd.Stdout = nil cmd.Stderr = os.Stderr - require.Nil(t, cmd.Run()) + require.NoError(t, cmd.Run()) }) } } func TestFetchall_multipleURLs(t *testing.T) { binary, err := binCache.GetBinary(fetchallImportPath) - require.Nil(t, err) + require.NoError(t, err) var fooHit, barHit int32 @@ -101,7 +101,7 @@ func TestFetchall_multipleURLs(t *testing.T) { cmd.Stdout = nil cmd.Stderr = os.Stderr - require.Nil(t, cmd.Run()) + require.NoError(t, cmd.Run()) require.Equal(t, int32(1), atomic.LoadInt32(&fooHit)) require.Equal(t, int32(1), atomic.LoadInt32(&barHit)) @@ -109,7 +109,7 @@ func TestFetchall_multipleURLs(t *testing.T) { func TestFetchall_malformed(t *testing.T) { binary, err := binCache.GetBinary(fetchallImportPath) - require.Nil(t, err) + require.NoError(t, err) hit := int32(0) s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -123,14 +123,14 @@ func TestFetchall_malformed(t *testing.T) { cmd.Stderr = os.Stderr err = cmd.Run() - require.Nil(t, err) + require.NoError(t, err) require.True(t, atomic.LoadInt32(&hit) >= 2) } func TestFetchall_concurrency(t *testing.T) { binary, err := binCache.GetBinary(fetchallImportPath) - require.Nil(t, err) + require.NoError(t, err) var mu sync.Mutex var callOrder []time.Duration @@ -140,7 +140,7 @@ func TestFetchall_concurrency(t *testing.T) { require.NotEmpty(t, s) d, err := time.ParseDuration(s) - require.Nil(t, err) + require.NoError(t, err) time.Sleep(d) @@ -165,7 +165,7 @@ func TestFetchall_concurrency(t *testing.T) { cmd.Stdout = nil cmd.Stderr = os.Stderr - require.Nil(t, cmd.Run()) + require.NoError(t, cmd.Run()) mu.Lock() defer mu.Unlock() diff --git a/tools/testtool/commands/test_submission_test.go b/tools/testtool/commands/test_submission_test.go index 6031b74..8a64296 100644 --- a/tools/testtool/commands/test_submission_test.go +++ b/tools/testtool/commands/test_submission_test.go @@ -29,11 +29,11 @@ func listDirs(dir string) ([]string, error) { func Test_testSubmission_correct(t *testing.T) { testDirs, err := listDirs("../testdata/submissions/correct") - require.Nil(t, err) + require.NoError(t, err) for _, dir := range testDirs { absDir, err := filepath.Abs(dir) - require.Nil(t, err) + require.NoError(t, err) problem := path.Base(absDir) t.Run(problem, func(t *testing.T) { studentRepo := path.Join(absDir, "student") @@ -46,11 +46,11 @@ func Test_testSubmission_correct(t *testing.T) { func Test_testSubmission_incorrect(t *testing.T) { testDirs, err := listDirs("../testdata/submissions/incorrect") - require.Nil(t, err) + require.NoError(t, err) for _, dir := range testDirs { absDir, err := filepath.Abs(dir) - require.Nil(t, err) + require.NoError(t, err) problem := path.Base(absDir) t.Run(problem, func(t *testing.T) { diff --git a/urlfetch/main_test.go b/urlfetch/main_test.go index 99b441d..dc5b763 100644 --- a/urlfetch/main_test.go +++ b/urlfetch/main_test.go @@ -33,7 +33,7 @@ func TestMain(m *testing.M) { func TestUrlFetch_valid(t *testing.T) { binary, err := binCache.GetBinary(urlfetchImportPath) - require.Nil(t, err) + require.NoError(t, err) type endpoint string type response string @@ -91,7 +91,7 @@ func TestUrlFetch_valid(t *testing.T) { cmd.Stderr = os.Stderr data, err := cmd.Output() - require.Nil(t, err) + require.NoError(t, err) for _, r := range tc.expected { require.True(t, bytes.Contains(data, []byte(r)), @@ -104,7 +104,7 @@ func TestUrlFetch_valid(t *testing.T) { func TestUrlFetch_malformed(t *testing.T) { binary, err := binCache.GetBinary(urlfetchImportPath) - require.Nil(t, err) + require.NoError(t, err) s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("success")) @@ -141,7 +141,7 @@ func TestUrlFetch_malformed(t *testing.T) { func TestUrlFetch_order(t *testing.T) { binary, err := binCache.GetBinary(urlfetchImportPath) - require.Nil(t, err) + require.NoError(t, err) mux := http.NewServeMux() @@ -175,7 +175,7 @@ func TestUrlFetch_order(t *testing.T) { cmd.Stdout = nil cmd.Stderr = os.Stderr - require.Nil(t, cmd.Run()) + require.NoError(t, cmd.Run()) mu.Lock() defer mu.Unlock() diff --git a/wordcount/main_test.go b/wordcount/main_test.go index e2413dd..179c227 100644 --- a/wordcount/main_test.go +++ b/wordcount/main_test.go @@ -33,7 +33,7 @@ func TestMain(m *testing.M) { func TestWordCount(t *testing.T) { binary, err := binCache.GetBinary(wordcountImportPath) - require.Nil(t, err) + require.NoError(t, err) type counts map[string]int64 type files []string @@ -77,7 +77,7 @@ b`, t.Run(tc.name, func(t *testing.T) { // Create temp directory. testDir, err := ioutil.TempDir("", "wordcount-testdata-") - require.Nil(t, err) + require.NoError(t, err) defer func() { _ = os.RemoveAll(testDir) }() // Create test files in temp directory. @@ -85,7 +85,7 @@ b`, for _, f := range tc.files { file := path.Join(testDir, testtool.RandomName()) err = ioutil.WriteFile(file, []byte(f), 0644) - require.Nil(t, err) + require.NoError(t, err) files = append(files, file) } @@ -94,11 +94,11 @@ b`, cmd.Stderr = os.Stderr output, err := cmd.Output() - require.Nil(t, err) + require.NoError(t, err) // Parse output and compare with an expected one. counts, err := parseStdout(output) - require.Nil(t, err) + require.NoError(t, err) require.True(t, reflect.DeepEqual(tc.expected, counts), fmt.Sprintf("expected: %v; got: %v", tc.expected, counts)) From 37e262c88d6b7614554f43995db22a96f0b59d58 Mon Sep 17 00:00:00 2001 From: Arseny Balobanov Date: Sat, 15 Feb 2020 22:39:40 +0300 Subject: [PATCH 4/9] Add test helpers to find free tcp ports and wait for ports to become available. --- tools/testtool/freeport.go | 57 +++++++++++++++++++++++++++++++++ tools/testtool/freeport_test.go | 27 ++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 tools/testtool/freeport.go create mode 100644 tools/testtool/freeport_test.go diff --git a/tools/testtool/freeport.go b/tools/testtool/freeport.go new file mode 100644 index 0000000..8f07e38 --- /dev/null +++ b/tools/testtool/freeport.go @@ -0,0 +1,57 @@ +package testtool + +import ( + "context" + "fmt" + "net" + "os" + "strconv" + "time" +) + +// GetFreePort returns free local tcp port. +func GetFreePort() (string, error) { + addr, err := net.ResolveTCPAddr("tcp", "localhost:0") + if err != nil { + return "", err + } + + l, err := net.ListenTCP("tcp", addr) + if err != nil { + return "", err + } + defer func() { _ = l.Close() }() + + p := l.Addr().(*net.TCPAddr).Port + + return strconv.Itoa(p), nil +} + +// WaitForPort tries to connect to given local port with constant backoff. +// +// Can be canceled via ctx. +func WaitForPort(ctx context.Context, port string) { + t := time.NewTicker(time.Millisecond * 100) + defer t.Stop() + + for { + select { + case <-ctx.Done(): + return + case <-t.C: + if err := portIsReady(port); err != nil { + _, _ = fmt.Fprintf(os.Stderr, "waiting for port: %s\n", err) + break + } + return + } + } +} + +func portIsReady(port string) error { + conn, err := net.Dial("tcp", net.JoinHostPort("localhost", port)) + if err != nil { + return err + } + return conn.Close() +} diff --git a/tools/testtool/freeport_test.go b/tools/testtool/freeport_test.go new file mode 100644 index 0000000..2cd4177 --- /dev/null +++ b/tools/testtool/freeport_test.go @@ -0,0 +1,27 @@ +package testtool + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestGetFreePort(t *testing.T) { + p, err := GetFreePort() + require.NoError(t, err) + require.NotEmpty(t, p) +} + +func TestWaitForPort(t *testing.T) { + p, err := GetFreePort() + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + WaitForPort(ctx, p) + + require.Error(t, ctx.Err()) +} From aa7c9ae7c5da857bb0b6508161ce36de79c436d6 Mon Sep 17 00:00:00 2001 From: Arseny Balobanov Date: Sat, 15 Feb 2020 22:56:46 +0300 Subject: [PATCH 5/9] Test success case of using WaitForPort func. --- tools/testtool/freeport_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tools/testtool/freeport_test.go b/tools/testtool/freeport_test.go index 2cd4177..4314c3b 100644 --- a/tools/testtool/freeport_test.go +++ b/tools/testtool/freeport_test.go @@ -2,6 +2,10 @@ package testtool import ( "context" + "net" + "net/http" + "net/http/httptest" + "net/url" "testing" "time" @@ -15,6 +19,23 @@ func TestGetFreePort(t *testing.T) { } func TestWaitForPort(t *testing.T) { + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + defer s.Close() + + u, err := url.Parse(s.URL) + require.Nil(t, err) + _, port, err := net.SplitHostPort(u.Host) + require.Nil(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + WaitForPort(ctx, port) + + require.NoError(t, ctx.Err()) +} + +func TestWaitForPort_timeout(t *testing.T) { p, err := GetFreePort() require.NoError(t, err) From e1c3ec8cd0ab7fbc68d84b5d7081d663e5b37b2a Mon Sep 17 00:00:00 2001 From: Arseny Balobanov Date: Wed, 19 Feb 2020 00:08:37 +0300 Subject: [PATCH 6/9] Replace context in WaitForPort with timeout. --- tools/testtool/freeport.go | 14 ++++++++------ tools/testtool/freeport_test.go | 15 ++------------- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/tools/testtool/freeport.go b/tools/testtool/freeport.go index 8f07e38..b31ef87 100644 --- a/tools/testtool/freeport.go +++ b/tools/testtool/freeport.go @@ -1,7 +1,6 @@ package testtool import ( - "context" "fmt" "net" "os" @@ -29,21 +28,24 @@ func GetFreePort() (string, error) { // WaitForPort tries to connect to given local port with constant backoff. // -// Can be canceled via ctx. -func WaitForPort(ctx context.Context, port string) { +// Returns error if port is not ready after timeout. +func WaitForPort(timeout time.Duration, port string) error { + stopTimer := time.NewTimer(timeout) + defer stopTimer.Stop() + t := time.NewTicker(time.Millisecond * 100) defer t.Stop() for { select { - case <-ctx.Done(): - return + case <-stopTimer.C: + return fmt.Errorf("timeout") case <-t.C: if err := portIsReady(port); err != nil { _, _ = fmt.Fprintf(os.Stderr, "waiting for port: %s\n", err) break } - return + return nil } } } diff --git a/tools/testtool/freeport_test.go b/tools/testtool/freeport_test.go index 4314c3b..f9cbbbb 100644 --- a/tools/testtool/freeport_test.go +++ b/tools/testtool/freeport_test.go @@ -1,7 +1,6 @@ package testtool import ( - "context" "net" "net/http" "net/http/httptest" @@ -27,22 +26,12 @@ func TestWaitForPort(t *testing.T) { _, port, err := net.SplitHostPort(u.Host) require.Nil(t, err) - ctx, cancel := context.WithTimeout(context.Background(), time.Second) - defer cancel() - - WaitForPort(ctx, port) - - require.NoError(t, ctx.Err()) + require.NoError(t, WaitForPort(time.Second, port)) } func TestWaitForPort_timeout(t *testing.T) { p, err := GetFreePort() require.NoError(t, err) - ctx, cancel := context.WithTimeout(context.Background(), time.Second) - defer cancel() - - WaitForPort(ctx, p) - - require.Error(t, ctx.Err()) + require.Error(t, WaitForPort(time.Second, p)) } From b67bb030c38e8f5d35eb673837790e9b89879b70 Mon Sep 17 00:00:00 2001 From: Arseny Balobanov Date: Wed, 19 Feb 2020 00:11:10 +0300 Subject: [PATCH 7/9] Return more detailed error message on timeout in WaitForPort. --- tools/testtool/freeport.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testtool/freeport.go b/tools/testtool/freeport.go index b31ef87..197b3bf 100644 --- a/tools/testtool/freeport.go +++ b/tools/testtool/freeport.go @@ -39,7 +39,7 @@ func WaitForPort(timeout time.Duration, port string) error { for { select { case <-stopTimer.C: - return fmt.Errorf("timeout") + return fmt.Errorf("no server started listening on port %s after timeout %d", port, timeout) case <-t.C: if err := portIsReady(port); err != nil { _, _ = fmt.Fprintf(os.Stderr, "waiting for port: %s\n", err) From b93d1be1dc3d60cee396d950a3f339afdf6d78d0 Mon Sep 17 00:00:00 2001 From: Arseny Balobanov Date: Wed, 19 Feb 2020 00:22:00 +0300 Subject: [PATCH 8/9] Support testing.TB as logger for WaitForPort. --- tools/testtool/freeport.go | 9 ++++++--- tools/testtool/freeport_test.go | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/tools/testtool/freeport.go b/tools/testtool/freeport.go index 197b3bf..437e0fd 100644 --- a/tools/testtool/freeport.go +++ b/tools/testtool/freeport.go @@ -3,7 +3,6 @@ package testtool import ( "fmt" "net" - "os" "strconv" "time" ) @@ -26,10 +25,14 @@ func GetFreePort() (string, error) { return strconv.Itoa(p), nil } +type logger interface { + Logf(format string, args ...interface{}) +} + // WaitForPort tries to connect to given local port with constant backoff. // // Returns error if port is not ready after timeout. -func WaitForPort(timeout time.Duration, port string) error { +func WaitForPort(l logger, timeout time.Duration, port string) error { stopTimer := time.NewTimer(timeout) defer stopTimer.Stop() @@ -42,7 +45,7 @@ func WaitForPort(timeout time.Duration, port string) error { return fmt.Errorf("no server started listening on port %s after timeout %d", port, timeout) case <-t.C: if err := portIsReady(port); err != nil { - _, _ = fmt.Fprintf(os.Stderr, "waiting for port: %s\n", err) + l.Logf("waiting for port: %s\n", err) break } return nil diff --git a/tools/testtool/freeport_test.go b/tools/testtool/freeport_test.go index f9cbbbb..486fb44 100644 --- a/tools/testtool/freeport_test.go +++ b/tools/testtool/freeport_test.go @@ -26,12 +26,12 @@ func TestWaitForPort(t *testing.T) { _, port, err := net.SplitHostPort(u.Host) require.Nil(t, err) - require.NoError(t, WaitForPort(time.Second, port)) + require.NoError(t, WaitForPort(t, time.Second, port)) } func TestWaitForPort_timeout(t *testing.T) { p, err := GetFreePort() require.NoError(t, err) - require.Error(t, WaitForPort(time.Second, p)) + require.Error(t, WaitForPort(t, time.Second, p)) } From daff2e5c7b926437a4426b9aa3a2a2444e7a7f56 Mon Sep 17 00:00:00 2001 From: Arseny Balobanov Date: Wed, 19 Feb 2020 00:26:48 +0300 Subject: [PATCH 9/9] Use %s for duration; log timeout error in test. --- tools/testtool/freeport.go | 2 +- tools/testtool/freeport_test.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/testtool/freeport.go b/tools/testtool/freeport.go index 437e0fd..84a9fde 100644 --- a/tools/testtool/freeport.go +++ b/tools/testtool/freeport.go @@ -42,7 +42,7 @@ func WaitForPort(l logger, timeout time.Duration, port string) error { for { select { case <-stopTimer.C: - return fmt.Errorf("no server started listening on port %s after timeout %d", port, timeout) + return fmt.Errorf("no server started listening on port %s after timeout %s", port, timeout) case <-t.C: if err := portIsReady(port); err != nil { l.Logf("waiting for port: %s\n", err) diff --git a/tools/testtool/freeport_test.go b/tools/testtool/freeport_test.go index 486fb44..2189fb7 100644 --- a/tools/testtool/freeport_test.go +++ b/tools/testtool/freeport_test.go @@ -33,5 +33,7 @@ func TestWaitForPort_timeout(t *testing.T) { p, err := GetFreePort() require.NoError(t, err) - require.Error(t, WaitForPort(t, time.Second, p)) + err = WaitForPort(t, time.Second, p) + require.Error(t, err) + t.Log(err.Error()) }