New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add parallel limit #225
Add parallel limit #225
Conversation
853956e
to
1d0bf74
Compare
|
||
protected function parallelLimitReached(): bool | ||
{ | ||
return count($this->processSet->get(ProcessWrapper::PROCESS_STATUS_PREPARED)) >= $this->parallelLimit; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is strange, isn`t it?
How current number of processes be greater than limit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, this is parallelism, who knows what could happen 🤣 . I prefer being a bit defensive there.
12227c8
to
2df6ad9
Compare
…n log and never finished tests This is caused by sebastianbergmann/phpunit#3379
ffc04cc
to
a2939c0
Compare
Ref. #210, #153
Allow to define a parallel limit (by default 50) - the actual number of parallel processes started by Steward.
Current theory of operation was:
But this started causing issues on large testsuites (with hundreds of tests), because, obviously, there has been running system process (consuming RAM, CPU) for each testcase, even if most of them were just queued and were just waiting for free slot. And with Selenium farm, which is capable of running many tests in a parallel, this could easily lead to uncontrolled resources exhausting.
The new theory operation is:
Note the PR is still kind-of proof of concept, I expect some beta-testing of this branch (and maybe some code cleanup) before this will be merged to master.