refactor: simplify test processes run#753
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22e176e16e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $output = $process->getOutput(); | ||
| if (!\str_contains($output, 'RoadRunner server started')) { | ||
| return false; |
There was a problem hiding this comment.
The new readiness callback now returns early unless getOutput() contains RoadRunner server started, so environments that intentionally silence logs (e.g. tests/Functional/.rr.silent.yaml sets logs.mode: none) can never be marked ready even when workers are actually up. In that case startRoadRunner() will time out and exit, which regresses functional test startup compared to the previous worker-probing-only logic.
Useful? React with 👍 / 👎.
| * @param array<string, mixed> $envs | ||
| */ | ||
| public function startRoadRunner(?array $rrCommand = null, int $commandTimeout = 10, array $envs = [], string $configFile = '.rr.yaml'): void | ||
| public function startRoadRunner(array $rrCommand, int $commandTimeout = 10, array $envs = [], string $configFile = '.rr.yaml'): void |
There was a problem hiding this comment.
Changing startRoadRunner to require a non-optional $rrCommand removes the previous default ([$this->systemInfo->rrExecutable, 'serve']) and introduces a runtime BC break for existing consumers of the public Temporal\Testing\Environment API that called startRoadRunner() with no arguments. Given this class is autoloaded as part of the library, this should remain optional or be shimmed to avoid unexpected ArgumentCountError in downstream test harnesses.
Useful? React with 👍 / 👎.
1 participant
What was changed
Why?
Checklist
Closes
How was this tested: