Skip to content
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

Code factoring request : \PHPUnit\Framework\TestCase::runTest() #3551

Closed
exteon opened this issue Mar 6, 2019 · 7 comments
Closed

Code factoring request : \PHPUnit\Framework\TestCase::runTest() #3551

exteon opened this issue Mar 6, 2019 · 7 comments

Comments

@exteon
Copy link

exteon commented Mar 6, 2019

Q A
PHPUnit version head
PHP version all
Installation Method all

I'm working on a TestCase wrapper for PHPUnit and Codeception that does a different kind of process isolation run, using pcntl_fork. You can see it here: https://github.com/exteon/WithIsolation

In order for this to work, we need to wrap around the actual test running as it is performed by PHPUnit, in \PHPUnit\Framework\TestCase::runTest(), line $testResult = $this->{$this->name}(...\array_values($testArguments)); . At the moment we are achieving this by using a hack: overriding runTest() and hijacking $this->name to trick it into running a wrapper. Since this is obviously wrong in the long run, it would help us to have the $testResult = $this->{$this->name}(...\array_values($testArguments)); factored out in a method that can be overridden.

@exteon exteon changed the title Code factoring request Code factoring request : \PHPUnit\Framework\TestCase::runTest() Mar 6, 2019
@sebastianbergmann
Copy link
Owner

Please send a pull request against master for me to look at.

@epdenouden
Copy link
Contributor

epdenouden commented Mar 6, 2019

@exteon I find your ideas intriguing and would like to subscribe to your newsletter. Thanks for the work you have already put into this.

I did some experiments around this on at a hackathon and had something working with containers that worked like the above. It was fast and worked like a charm as long as the tests were simple. Also have a branch with 99% working lazy-loading @dataprovider which is also impacted by this and vice-versa.

There is a lot to going on around process isolation, just to name a few things:

@exteon
Copy link
Author

exteon commented Mar 6, 2019

@epdenouden Well, that is hy we started this, because the fork preserves the state and doesn't need to be reinitialized. Which is all the more important to run with Codeception, which adds a whole deal of wrapping around PHPUnit that seems pretty difficult to reinitialize from zero. But with the downside that some libraries, even if threadsafe, may not be fork-safe (i.e. some DB clients, we are exploring this). But this is a general setback that will impede any form of shared resource, hence lazy loading, no matter the process initialisation mode; some resources are just not "serializable" so they can't be IPC'ed. It works for what we set out to do, but for all intents and purposes it seems it remains still a lot more work to do.

@sebastianbergmann
Copy link
Owner

@exteon Still waiting for the pull request.

@stale
Copy link

stale bot commented Jun 21, 2019

This issue has been automatically marked as stale because it has not had activity within the last 60 days. It will be closed after 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 21, 2019
@stale
Copy link

stale bot commented Jun 28, 2019

This issue has been automatically closed because it has not had activity since it was marked as stale. Thank you for your contributions.

@stale stale bot closed this as completed Jun 28, 2019
@exteon
Copy link
Author

exteon commented Jul 7, 2019

Hi @sebastianbergmann , sorry for the late reply: we've patched PHPUnit with https://github.com/exteon/WithIsolation/blob/master/Codeception/codeception.patch
The changes are pretty small; but I see two patches don't run against the PHPUnit head, as i.e. TestCase::setUseErrorHandlerFromAnnotation() seems to be gone now.

Unfortunately I can't find the time ATM to upgrade everything (Codecept/PHPUnit/WithIsolation) and make everything work again; however, as a principle (since I see you're rewriting the code anyway): all we need is method/property/readers visible such that the AbstractProcess::processChildResult() and its wrapping calling code can be replicated in a class derived from TestCase (as you can see in https://github.com/exteon/WithIsolation/blob/master/Codeception/tests/_support/Test/WithIsolation.php method run())

I'm sorry I can't find the time to provide a more precise pull request at this point, but I hope it helps for the code rewrites I see advertised everywhere in the current code as coming for V9.

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

No branches or pull requests

3 participants