Skip to content

fix: testing environment config file ignored and needlessly wait for timeout#735

Open
mtorromeo wants to merge 3 commits intotemporalio:masterfrom
mtorromeo:fix-testing-environment
Open

fix: testing environment config file ignored and needlessly wait for timeout#735
mtorromeo wants to merge 3 commits intotemporalio:masterfrom
mtorromeo:fix-testing-environment

Conversation

@mtorromeo
Copy link
Copy Markdown

What was changed

Changed Temporal\Testing\Environment::startRoadRunner to actually use the $configFile parameter and to not wait for timeout if the process is not running.

Why?

  1. In Temporal\Testing\Environment the startRoadRunner has a $configFile parameter, which is also passed through the start method's $roadRunnerConfigFile parameter but that is completely ignored.

  2. In startRoadRunner if the executed process crashes/exists before $commandTimeout triggers (which is 10 seconds by default but could also be much longer) then the function will wait needlessly for the timeout while it could return right away.

Checklist

Confirmed by testing it on my own project, also with a misconfigured command that does not start correctly.

@mtorromeo mtorromeo requested review from a team, roxblnfk and wolfy-j as code owners March 25, 2026 10:37
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 25, 2026

@mtorromeo is attempting to deploy a commit to the Temporal Team on Vercel.

A member of the Team first needs to authorize it.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 25, 2026

CLA assistant check
All committers have signed the CLA.

@mtorromeo mtorromeo changed the title fix: broken testing environment start due to changes in startTemporal… fix: testing environment config file ignored and needlessly wait for timeout Mar 25, 2026
@mtorromeo mtorromeo force-pushed the fix-testing-environment branch from 54ebcb7 to f260474 Compare March 31, 2026 08:14
@mtorromeo
Copy link
Copy Markdown
Author

I forgot to add that there was a third fix in the PR.

If startRoadrunner is passed a custom $rrCommand this is currently used correctly to start the serve process but the "check" loop still calls [$this->systemInfo->rrExecutable, 'workers', ...].

The proposed solution is to use $rrCommand by replacing the serve argument with workers.

I just separated the commit for easier review.

@mtorromeo
Copy link
Copy Markdown
Author

I would also like to point out that in my opinion the classes in this SDK seem to be declared as final a bit too often, and while sometimes it is indeed a good call, other times I don't see any reason to do so.

Temporal\Testing\Environment is one such case where I don't understand why this needs to be final, as I could have easily extended the class to implement my own startRoadRunner logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants