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

Illuminate\Foundation\Testing\TestCase createApplication frequency #1798

Closed
assertchris opened this Issue Jul 2, 2013 · 19 comments

Comments

Projects
None yet
6 participants
@assertchris
Contributor

assertchris commented Jul 2, 2013

createApplication is called before every test (by virtue of it being called in setUp). This loads the Laravel environment multiple times per TestCase, which is over-kill.

I am developing a test-suite; in which test cases have 5 tests each. Each TestCase adds 17mb of memory usage to the overall memory usage, and I have a feeling it's because of the frequency of this method.

@assertchris

This comment has been minimized.

Show comment
Hide comment
@assertchris

assertchris Jul 2, 2013

Contributor

So, having dug deep in vendor; it appears that this is primarily caused by PHPUnit creating a class instance per test* method.

This also affects (read: breaks) running phpunit with process isolation...

Contributor

assertchris commented Jul 2, 2013

So, having dug deep in vendor; it appears that this is primarily caused by PHPUnit creating a class instance per test* method.

This also affects (read: breaks) running phpunit with process isolation...

@assertchris

This comment has been minimized.

Show comment
Hide comment
@assertchris

assertchris Jul 2, 2013

Contributor

Ok - so I have an idea.

In order to prevent new Laravel instances for every single test* method, why not create a bootstrap/testing.php file which has the following code:

<?php

require("autoload.php");
require("start.php");

Then; phpunit.xml can list bootstrap/testing.php (instead of bootstrap/autoload.php) and app/tests/TestCase.php can include the following code:

<?php

class TestCase extends Illuminate\Foundation\Testing\TestCase {

    /**
     * Creates the application.
     *
     * @return Symfony\Component\HttpKernel\HttpKernelInterface
     */
    public function createApplication()
    {
        global $app;

        $app['env'] = 'testing';

        return $app;
    }

}

This means Laravel is only loaded once, but the tests can continue to function.

EDIT:

Process isolation is still broken, but for a completely different reason...

Contributor

assertchris commented Jul 2, 2013

Ok - so I have an idea.

In order to prevent new Laravel instances for every single test* method, why not create a bootstrap/testing.php file which has the following code:

<?php

require("autoload.php");
require("start.php");

Then; phpunit.xml can list bootstrap/testing.php (instead of bootstrap/autoload.php) and app/tests/TestCase.php can include the following code:

<?php

class TestCase extends Illuminate\Foundation\Testing\TestCase {

    /**
     * Creates the application.
     *
     * @return Symfony\Component\HttpKernel\HttpKernelInterface
     */
    public function createApplication()
    {
        global $app;

        $app['env'] = 'testing';

        return $app;
    }

}

This means Laravel is only loaded once, but the tests can continue to function.

EDIT:

Process isolation is still broken, but for a completely different reason...

@assertchris

This comment has been minimized.

Show comment
Hide comment
@assertchris

assertchris Jul 2, 2013

Contributor

Here is a before and after:

before:

dev@dev:~/Source/chrispitt/beta.numbers.org.za/source$ phpunit
PHPUnit 3.7.21 by Sebastian Bergmann.

Configuration read from /home/dev/Source/chrispitt/beta.numbers.org.za/source/phpunit.xml

...............................................................  63 / 130 ( 48%)
............................................................... 126 / 130 ( 96%)
....

Time: 26 seconds, Memory: 475.25Mb

OK (130 tests, 286 assertions)
dev@dev:~/Source/chrispitt/beta.numbers.org.za/source$

after:

dev@dev:~/Source/chrispitt/beta.numbers.org.za/source$ phpunit
PHPUnit 3.7.21 by Sebastian Bergmann.

Configuration read from /home/dev/Source/chrispitt/beta.numbers.org.za/source/phpunit.xml

...............................................................  63 / 130 ( 48%)
............................................................... 126 / 130 ( 96%)
....

Time: 4 seconds, Memory: 18.00Mb

OK (130 tests, 286 assertions)
dev@dev:~/Source/chrispitt/beta.numbers.org.za/source$
Contributor

assertchris commented Jul 2, 2013

Here is a before and after:

before:

dev@dev:~/Source/chrispitt/beta.numbers.org.za/source$ phpunit
PHPUnit 3.7.21 by Sebastian Bergmann.

Configuration read from /home/dev/Source/chrispitt/beta.numbers.org.za/source/phpunit.xml

...............................................................  63 / 130 ( 48%)
............................................................... 126 / 130 ( 96%)
....

Time: 26 seconds, Memory: 475.25Mb

OK (130 tests, 286 assertions)
dev@dev:~/Source/chrispitt/beta.numbers.org.za/source$

after:

dev@dev:~/Source/chrispitt/beta.numbers.org.za/source$ phpunit
PHPUnit 3.7.21 by Sebastian Bergmann.

Configuration read from /home/dev/Source/chrispitt/beta.numbers.org.za/source/phpunit.xml

...............................................................  63 / 130 ( 48%)
............................................................... 126 / 130 ( 96%)
....

Time: 4 seconds, Memory: 18.00Mb

OK (130 tests, 286 assertions)
dev@dev:~/Source/chrispitt/beta.numbers.org.za/source$
@wayneashleyberry

This comment has been minimized.

Show comment
Hide comment
@wayneashleyberry

wayneashleyberry commented Jul 2, 2013

👍

@wayneashleyberry

This comment has been minimized.

Show comment
Hide comment
@wayneashleyberry

wayneashleyberry Jul 2, 2013

Those memory differences are massive

wayneashleyberry commented Jul 2, 2013

Those memory differences are massive

@crynobone

This comment has been minimized.

Show comment
Hide comment
@crynobone

crynobone Jul 2, 2013

Contributor

While this is good, having all test case sharing the same $app instance may cause a false assertion result (highly depends on scenario) where certain value is now shared between test (Config value, Session etc).

It as if using static method all over again.

Contributor

crynobone commented Jul 2, 2013

While this is good, having all test case sharing the same $app instance may cause a false assertion result (highly depends on scenario) where certain value is now shared between test (Config value, Session etc).

It as if using static method all over again.

@assertchris

This comment has been minimized.

Show comment
Hide comment
@assertchris

assertchris Jul 2, 2013

Contributor

Valid concern. Perhaps there should be a way to reset the framework; which can be called on a case-by-case basis. 475mb for a test run is horrible. :)

Contributor

assertchris commented Jul 2, 2013

Valid concern. Perhaps there should be a way to reset the framework; which can be called on a case-by-case basis. 475mb for a test run is horrible. :)

@taylorotwell

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell Jul 2, 2013

Member

Yeah all tests should definitely not share the same $app. You need a new application instance to work with in each of your tests so they are truly isolated, so we'll need a better way to decrease the RAM than that.

Member

taylorotwell commented Jul 2, 2013

Yeah all tests should definitely not share the same $app. You need a new application instance to work with in each of your tests so they are truly isolated, so we'll need a better way to decrease the RAM than that.

@taylorotwell

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell Jul 9, 2013

Member

You could also override setUp and just call createApplication whenever you truly needed to reset the test environment.

Member

taylorotwell commented Jul 9, 2013

You could also override setUp and just call createApplication whenever you truly needed to reset the test environment.

@assertchris

This comment has been minimized.

Show comment
Hide comment
@assertchris

assertchris Jul 9, 2013

Contributor

I tried that but it seems $this->app is not maintained between tests which
is why it is being called despite your checks...

On Tuesday, 9 July 2013, Taylor Otwell wrote:

You could also override setUp and just call createApplication whenever you
truly needed to reset the test environment.


Reply to this email directly or view it on GitHubhttps://github.com/laravel/framework/issues/1798#issuecomment-20651118
.

Contributor

assertchris commented Jul 9, 2013

I tried that but it seems $this->app is not maintained between tests which
is why it is being called despite your checks...

On Tuesday, 9 July 2013, Taylor Otwell wrote:

You could also override setUp and just call createApplication whenever you
truly needed to reset the test environment.


Reply to this email directly or view it on GitHubhttps://github.com/laravel/framework/issues/1798#issuecomment-20651118
.

@taylorotwell

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell Jul 9, 2013

Member

Sorry, it would really be:

public function setUp()
{
     // Just overriding the base setup.
}

public function testFoo()
{
     $this->app = $this->refreshApplication();

     //
}
Member

taylorotwell commented Jul 9, 2013

Sorry, it would really be:

public function setUp()
{
     // Just overriding the base setup.
}

public function testFoo()
{
     $this->app = $this->refreshApplication();

     //
}
@assertchris

This comment has been minimized.

Show comment
Hide comment
@assertchris

assertchris Jul 9, 2013

Contributor

So, if I understand you correctly, I should be running as many tests as I can without needing the application. While that would dramatically decrease the memory usage, it doesn't help me with model tests (which is what these are). In that case; I need the app to be bootstrapped before I can effectively test how the app's models work. Unless I am misunderstanding your suggestion... :)

Contributor

assertchris commented Jul 9, 2013

So, if I understand you correctly, I should be running as many tests as I can without needing the application. While that would dramatically decrease the memory usage, it doesn't help me with model tests (which is what these are). In that case; I need the app to be bootstrapped before I can effectively test how the app's models work. Unless I am misunderstanding your suggestion... :)

@taylorotwell

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell Jul 9, 2013

Member

Yeah, you could run tests without rebooting the app if you kinda are aware
how the IoC works and how Laravel works in general. It mainly is rebooted
everytime because most people won't have that knowledge and just want a
fresh start on each test. But, since you could override stuff in the IoC
with App::instance it will work fine.

On Tue, Jul 9, 2013 at 8:17 AM, Christopher Pitt
notifications@github.comwrote:

So, if I understand you correctly, I should be running as many tests as I
can without needing the application. While that would dramatically decrease
the memory usage, it doesn't help me with model tests (which is what these
are). In that case; I need the app to be bootstrapped before I can
effectively test how the app's models work. Unless I am misunderstanding
your suggestion... :)


Reply to this email directly or view it on GitHubhttps://github.com/laravel/framework/issues/1798#issuecomment-20672897
.

Member

taylorotwell commented Jul 9, 2013

Yeah, you could run tests without rebooting the app if you kinda are aware
how the IoC works and how Laravel works in general. It mainly is rebooted
everytime because most people won't have that knowledge and just want a
fresh start on each test. But, since you could override stuff in the IoC
with App::instance it will work fine.

On Tue, Jul 9, 2013 at 8:17 AM, Christopher Pitt
notifications@github.comwrote:

So, if I understand you correctly, I should be running as many tests as I
can without needing the application. While that would dramatically decrease
the memory usage, it doesn't help me with model tests (which is what these
are). In that case; I need the app to be bootstrapped before I can
effectively test how the app's models work. Unless I am misunderstanding
your suggestion... :)


Reply to this email directly or view it on GitHubhttps://github.com/laravel/framework/issues/1798#issuecomment-20672897
.

@assertchris

This comment has been minimized.

Show comment
Hide comment
@assertchris

assertchris Jul 9, 2013

Contributor

Ok - I think I understand. I did try this within the tests cases (app/tests/*) but PHPUnit appears to encapsulate each test method in it's own class instance, so none of them share instance variables. The only way I got around that was by changing how the bootstrapping happens (in the example code above). For my purposes; sharing 1 application instance is fine because I am aware of what is shared and have passable knowledge of L4 IoC, but new users with lots of tests will still suffer the problem. :(

Contributor

assertchris commented Jul 9, 2013

Ok - I think I understand. I did try this within the tests cases (app/tests/*) but PHPUnit appears to encapsulate each test method in it's own class instance, so none of them share instance variables. The only way I got around that was by changing how the bootstrapping happens (in the example code above). For my purposes; sharing 1 application instance is fine because I am aware of what is shared and have passable knowledge of L4 IoC, but new users with lots of tests will still suffer the problem. :(

@taylorotwell

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell Jul 9, 2013

Member

Have you modified your phpunit xml at all?

Member

taylorotwell commented Jul 9, 2013

Have you modified your phpunit xml at all?

@assertchris

This comment has been minimized.

Show comment
Hide comment
@assertchris

assertchris Jul 9, 2013

Contributor

Yeah - here it is:

<?xml version="1.0" encoding="UTF-8"?>
<phpunit
    backupGlobals="false"
    backupStaticAttributes="false"
    bootstrap="bootstrap/testing.php"
    colors="true"
    convertErrorsToExceptions="true"
    convertNoticesToExceptions="true"
    convertWarningsToExceptions="true"
    processIsolation="false"
    stopOnFailure="false"
    syntaxCheck="false"
    >
    <testsuites>
        <testsuite name="Application Test Suite">
            <directory>./app/tests/</directory>
        </testsuite>
    </testsuites>
</phpunit>
Contributor

assertchris commented Jul 9, 2013

Yeah - here it is:

<?xml version="1.0" encoding="UTF-8"?>
<phpunit
    backupGlobals="false"
    backupStaticAttributes="false"
    bootstrap="bootstrap/testing.php"
    colors="true"
    convertErrorsToExceptions="true"
    convertNoticesToExceptions="true"
    convertWarningsToExceptions="true"
    processIsolation="false"
    stopOnFailure="false"
    syntaxCheck="false"
    >
    <testsuites>
        <testsuite name="Application Test Suite">
            <directory>./app/tests/</directory>
        </testsuite>
    </testsuites>
</phpunit>
@assertchris

This comment has been minimized.

Show comment
Hide comment
@assertchris

assertchris Jul 9, 2013

Contributor

...here's bootstrap/testing.php

<?php

require ("autoload.php");
require ("start.php");
Contributor

assertchris commented Jul 9, 2013

...here's bootstrap/testing.php

<?php

require ("autoload.php");
require ("start.php");
@rizqidjamaluddin

This comment has been minimized.

Show comment
Hide comment
@rizqidjamaluddin

rizqidjamaluddin Jul 25, 2014

Dropping this gist for anyone arriving at this issue. Artisan command to run through multiple phpunit tests in sequence, so once can get around building memory use issues by splitting their tests through different suites.

rizqidjamaluddin commented Jul 25, 2014

Dropping this gist for anyone arriving at this issue. Artisan command to run through multiple phpunit tests in sequence, so once can get around building memory use issues by splitting their tests through different suites.

@LasseRafn

This comment has been minimized.

Show comment
Hide comment
@LasseRafn

LasseRafn Aug 13, 2017

Contributor

Anyone ever found a proper solution?

Contributor

LasseRafn commented Aug 13, 2017

Anyone ever found a proper solution?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment