Resolved test suite run in separated process executed one by one in separated process. #6063
Conversation
src/Framework/TestSuite.php
Outdated
| * | ||
| * @throws Exception | ||
| */ | ||
| protected function addTestMethod(ReflectionClass $class, ReflectionMethod $method, array $groups): void |
There was a problem hiding this comment.
why have this methods been moved in this file? this makes the actual diff hard to read (in case this has been done by CS-fixer, it might make sense todo the private->public transformation in a separate PR, so the rest gets readable)
There was a problem hiding this comment.
It was done by CS-fixer. Only changed the access type.
There was a problem hiding this comment.
I see, thx. I have this CS-fixer rule :)
There was a problem hiding this comment.
@sebastianbergmann wdyt about this CS fixer rule? :)
(most of the diff in this file of the PR is generated by the CS fixer sorting methods arround, because they changed the visibility. we don't see the actual changes anymore)
There was a problem hiding this comment.
saying just that, maybe @akotulu could use separate commits for the CS fixing changes and the rest, so we could review commit by commit
There was a problem hiding this comment.
I agree that there is a conflict of objectives at play here: one the one hand, I want methods to be ordered in a certain way. On the other hand, the proposed changes should be readable. In this case the method ordering gets in the way of patch readability because the visitbility of a method is changed.
There was a problem hiding this comment.
Ok, how can I revert it or close this branch and add the modifications without CS fixer?
Hmm, atm I will revert the function back and add the modifications. Rebase and commit to this pull request. After successful review I will commit again with CS fixer?
|
please rebase |
|
Rebased to main, also reverted to CS-fixer changes. Do I need to make a new pull request from my main? |
|
Force-pushing the current state to this PR should be enough. |
akotulu
force-pushed
the
separated-process-improvement
branch
from
1f79abe to
f4728e7
Compare
|
Hope I did it right. |
|
Hey, any progress on the review? |
|
I am currently only able to work for very short periods of time (and should not even do that), see https://phpc.social/deck/@sebastian/113487079241563305. I will get to this as soon as I am able to and kindly ask for your patience. |
sebastianbergmann
force-pushed
the
separated-process-improvement
branch
from
f4728e7 to
387c163
Compare
Second pass of resolving
RunClassInSeparateProcessattribute issues runningTestSuitetests one by one in a separated process and calling before and after class methods for each test in the suite. Also test suite in separated process called primary process before and after class methods.Created separated branch, it contains first pass modifications as well.