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

Data providers retention eases memory leaks #3715

Closed
nesk opened this issue Jun 6, 2019 · 7 comments
Closed

Data providers retention eases memory leaks #3715

nesk opened this issue Jun 6, 2019 · 7 comments
Assignees
Labels
type/enhancement A new idea that should be implemented

Comments

@nesk
Copy link

nesk commented Jun 6, 2019

Q A
PHPUnit version 7.5.12
PHP version 7.3.5
Installation Method Composer

I have some routes to test functionally, to achieve this I have a test which depends on a data provider. This data provider contains datasets with, for each, a URL and some URL parameters to fill.

Since those URL parameters generally accept an ID to fetch an entity inside the database, I did something like this to ease the writing of my tests (I'm not writing all the required code, so we can focus on what's important):

/** @dataProvider routeProvider */
public function testRouteReturnsOkCode(string $url, array $urlParameters): void
{
    // Save the entities to the database and replace them by their ID
    $urlParameters = array_map(function ($entity) {
        $this->saveToDatabase($entity);
        return $entity->getId();
    }, $urlParameters);

    // Replace the URL placeholders by the provided parameters
    $url = $this->generateUrl($url, $urlParameters);

    // Make the request and ensure 200 is the returned status code
    $this->client->request('GET', $url);
    static::assertSame(200, $this->client->getResponse()->getStatusCode());
}

public function routeProvider(): Generator
{
    yield [
        '/invoices/{invoiceId}', // URL
        ['invoiceId' => new Invoice], // Entity to persist, whose ID will be used as the parameter
    ];
}

This code works well, the entities are persisted and the URLs are properly generated. However, with my real codebase, I have 80 datasets in my provider with multiple entities (not just one like in the example).

The memory consumption is fine once PHPUnit generates all the data to estimate the number of tests to run, but, once the tests start running, the memory consumption quickly explodes because the entities are being persisted by Doctrine, which makes them use more memory. Since PHPUnit keeps all the data generated by providers in memory, those heavy entities (and probably many other things created by Doctrine) stay in memory.

I do understand why PHPUnit works that way, it needs to keep the data in memory to be able to display it if a test fails. However, maybe we could have an option to disable this data cache, avoiding to much memory consumption? Or maybe I shouldn't write my tests like this?

@epdenouden epdenouden self-assigned this Jun 6, 2019
@epdenouden epdenouden added the type/enhancement A new idea that should be implemented label Jun 6, 2019
@epdenouden
Copy link
Contributor

@nesk Goodmorning and thanks for the detailed use case you presented re: resource usage of dataproviders. I have been doing some work on this already on the PHPUnit side, which indeed does save (quite a lot) of memory and improves performance.

However that work is still experimental and targetted to version 8. Is your project and test suite open to the public?

I'd be great if you could hit me up per email or on LinkedIn as this is my current area of research.

@nesk
Copy link
Author

nesk commented Jun 6, 2019

Unfortunately, no, my project is not open to the public. However, maybe I could try to create a showcase to help you see what's really happening?

@epdenouden
Copy link
Contributor

Hello again @nesk!

Unfortunately, no, my project is not open to the public. However, maybe I could try to create a showcase to help you see what's really happening?

That'd be great, I've poked you on LinkedIn so we can discuss this outside the Github issues.

@nesk
Copy link
Author

nesk commented Jun 20, 2019

Here's the showcase: https://gist.github.com/nesk/343d78c13eee91355e999877d3518eb6

Clone the repo, install the dependencies, then run phpunit:

git clone https://gist.github.com/343d78c13eee91355e999877d3518eb6.git showcase
cd showcase
composer install
php -d memory_limit=128M ./vendor/bin/phpunit

The command should fail with the following error:

PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted (tried to allocate 266240 bytes) in /Users/johann/Desktop/phpunit-data-providers-memory-showcase/vendor/phpunit/phpunit/src/Framework/TestSuite.php on line 237

To avoid the memory limit error, use:

php -d memory_limit=256M ./vendor/bin/phpunit

@epdenouden
Copy link
Contributor

@nesk thank you! I will have a look tonight.

@epdenouden
Copy link
Contributor

@nesk I am working on #3736 to make PHPUnit more memory efficient re: data providers. Any feedback on that ticket is very much welcome.

Thank you for spending time on creating the showcase, its structure was a good idea for the testing the prototype.

@epdenouden
Copy link
Contributor

Superseded by #3736

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement A new idea that should be implemented
Projects
None yet
Development

No branches or pull requests

2 participants