testsuite.StartDevServer: Respect timeout during dial (#1498)

testsuite.StartDevServer accepts a context.Context
meant to indicate how long the caller is willing to wait for the result.
This context is used when downloading the Temporal CLI,
but is not considered when tryign to dial to the server in
`waitServerReady`.

Without this, if an attempt to start the server failed
(e.g. because of an invalid `--log-format` argument),
`waitServerReady` will wait for `600 * 100ms = 1 minute`
before returning the failure to the caller.

This changes `waitServerReady` to respect the timeout configured in the
context--checking in on it after each attempt.

This also changes the actual `Dial` attempt to use `DialContext`
with the same context so that if the context expires,
the dial operation also returns early if possible.

testsuite/devserver.go

@@ -339,9 +339,9 @@ func extractZip(r io.Reader, toExtract string, w io.Writer) error {
 // Returns a connected client created using the provided options.
 func waitServerReady(ctx context.Context, options client.Options) (client.Client, error) {
 	var returnedClient client.Client
-	lastErr := retryFor(600, 100*time.Millisecond, func() error {
+	lastErr := retryFor(ctx, 600, 100*time.Millisecond, func() error {
 		var err error
-		returnedClient, err = client.Dial(options)
+		returnedClient, err = client.DialContext(ctx, options)
 		return err
 	})
 	if lastErr != nil {
@@ -351,19 +351,28 @@ func waitServerReady(ctx context.Context, options client.Options) (client.Client
 }
 
 // retryFor retries some function until it returns nil or runs out of attempts. Wait interval between attempts.
-func retryFor(maxAttempts int, interval time.Duration, cond func() error) error {
+func retryFor(ctx context.Context, maxAttempts int, interval time.Duration, cond func() error) error {
 	if maxAttempts < 1 {
 		// this is used internally, okay to panic
 		panic("maxAttempts should be at least 1")
 	}
+
+	ticker := time.NewTicker(interval)
+	defer ticker.Stop()
+
 	var lastErr error
 	for i := 0; i < maxAttempts; i++ {
 		if curE := cond(); curE == nil {
 			return nil
 		} else {
 			lastErr = curE
 		}
-		time.Sleep(interval)
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		case <-ticker.C:
+			// Try again after waiting up to interval.
+		}
 	}
 	return lastErr
 }

testsuite/devserver_internal_test.go

@@ -0,0 +1,62 @@
+// The MIT License
+//
+// Copyright (c) 2020 Temporal Technologies Inc.  All rights reserved.
+//
+// Copyright (c) 2020 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package testsuite
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.temporal.io/sdk/client"
+	"go.temporal.io/sdk/internal/log"
+)
+
+func TestWaitServerReady_respectsTimeout(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond)
+	defer cancel()
+
+	hostPort, err := getFreeHostPort()
+	require.NoError(t, err, "get free host port")
+
+	startTime := time.Now()
+	_, err = waitServerReady(ctx, client.Options{
+		HostPort:  hostPort,
+		Namespace: "default",
+		Logger:    log.NewNopLogger(),
+	})
+	require.Error(t, err, "Dial should fail")
+	assert.ErrorIs(t, err, context.DeadlineExceeded)
+	assert.WithinDuration(t,
+		startTime.Add(time.Millisecond),
+		time.Now(),
+		5*time.Millisecond,
+		// Even though the timeout is only a millisecond,
+		// we'll allow for a slack of up to 5 milliseconds
+		// to account for slow CI machines.
+		// Anything smaller than 1 second is fine to use here.
+	)
+}