search

temporalio / sdk-go

Temporal Go SDK
https://docs.temporal.io/application-development?lang=go
MIT License
481 stars 197 forks source link

testsuite.StartDevServer: Respect timeout during dial #1498

Closed abhinav closed 1 month ago

abhinav commented 1 month ago

What was changed

Background: 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 trying to dial to the server in waitServerReady.

Change: This changes waitServerReady to respect the timeout configured in the context--checking in on it after each attempt, and returning early if the context has expired. Similarly, the Dial attempt now uses DialContext so that if the context expires, the dial operation also returns early if it can.

Why?

Without this, if an attempt to start the server failed (e.g. because of an invalid --log-format argument), waitServerReady will block the test for 1 minute before giving up and returning an error. This is a pretty long time to wait for a test to fail, when the caller is expecting, say, 5 seconds at most.

Checklist

  1. How was this tested: A unit test was added for the new functionality, and run with go test -run WaitServerReady -count 1000 to verify that it's not flaky.

  2. Any docs updates needed? I don't think so.

CLAassistant commented 1 month ago

CLA assistant check
All committers have signed the CLA.

CLAassistant commented 1 month ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

cretz commented 1 month ago

LGTM, will let @Quinn-With-Two-Ns review

Quinn-With-Two-Ns commented 1 month ago

CI failing looks like an issue with the server v1.24.0 docker images. Will follow up with the server team.