Skip to content

Commit

Permalink
testsuite.StartDevServer: Respect timeout during dial
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
abhinav committed May 31, 2024
1 parent 486c233 commit 6afc684
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 4 deletions.
13 changes: 9 additions & 4 deletions testsuite/devserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -351,7 +351,7 @@ 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")
Expand All @@ -363,7 +363,12 @@ func retryFor(maxAttempts int, interval time.Duration, cond func() error) error
} else {
lastErr = curE
}
time.Sleep(interval)
select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(interval):
// Try again.
}
}
return lastErr
}
Expand Down
62 changes: 62 additions & 0 deletions testsuite/devserver_internal_test.go
Original file line number Diff line number Diff line change
@@ -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.
)
}

0 comments on commit 6afc684

Please sign in to comment.