Eloquent model events are not triggered when testing

[PascalZajac]

The various Eloquent model events ending with ing are not being triggered during tests.

I discovered this a while ago and assumed it was intended functionality (forcing you to test your event handlers separately, much as route filters are not run in test mode).

However today I discovered that the post-action event handler created is triggered when run in test mode. Is there a reason for the discrepancy?

[PascalZajac]

Upon further investigation, the behaviour gets weirder. It seems like the model event only executes the very first time the action occurs within the test suite.

To explain my particular scenario: I have an Image model, and an associated ImageService which handles created and deleted events. When I first wrote the unit tests for the service, the model events were triggered correctly. I then added an ImageController, with tests covering creation of an Image via the controller. Suddenly, my ImageService tests began to fail. Disabling the tests for the ImageController would fix the ImageService tests, as would manually invoking the ImageService event methods within the ImageService test.

[taylorotwell]

Paste test.

On May 5, 2013, at 2:58 AM, Pascal Zajac notifications@github.com wrote:

Upon further investigation, the behaviour gets weirder. It seems like the model event only executes the very first time the action occurs within the test suite.

To explain my particular scenario: I have an Image model, and an associated ImageService which handles created and deleted events. When I first wrote the unit tests for the service, the model events were trigger correctly. I then added an ImageController, with tests covering creation of an Image via the controller. Suddenly, my ImageService tests began to fail. Disabling the tests for the ImageController would fix the ImageService tests, as would manually invoking the ImageService event methods within the ImageService test.

— Reply to this email directly or view it on GitHub.

[PascalZajac]

Ok, this is the most succinct example I can produce:

    public function testFolderCreationAndDeletion()
    {
        // Create a new image.
        $image = Image::create(array());

        // Make sure a new folder was created.
        $path = ImageService::getPath($image);
        $this->assertTrue(is_dir($path)); // Passes.

        // Cleanup.
        $image->delete();
        $this->assertFalse(is_dir($path));
    }

    public function testAgain()
    {
        // Create a new image.
        $image = Image::create(array());

        // Make sure a new folder was created.
        $path = ImageService::getPath($image);
        $this->assertTrue(is_dir($path)); // Fails

        // Cleanup.
        $image->delete();
        $this->assertFalse(is_dir($path));
    }

The exact same test, if run again as a separate test, will fail where noted.

[taylorotwell]

Can you just paste the entire test class that fails?

On May 5, 2013, at 6:31 PM, Pascal Zajac notifications@github.com wrote:

Ok, this is the most succinct example I can produce:

public function testFolderCreationAndDeletion()
{
    // Create a new image.
    $image = Image::create(array());

    // Make sure a new folder was created.
    $path = ImageService::getPath($image);
    $this->assertTrue(is_dir($path)); // Passes.

    // Cleanup.
    $image->delete();
    $this->assertFalse(is_dir($path));
}

public function testAgain()
{
    // Create a new image.
    $image = Image::create(array());

    // Make sure a new folder was created.
    $path = ImageService::getPath($image);
    $this->assertTrue(is_dir($path)); // Fails

    // Cleanup.
    $image->delete();
    $this->assertFalse(is_dir($path));
}

The exact same test, if run again as a separate test, will fail where noted.

— Reply to this email directly or view it on GitHub.

[PascalZajac]

use Instinct\Image;
use Instinct\User;

class ImageServiceTest extends TestCase {

    public function testFolderCreationAndDeletion()
    {
        // Create a new image.
        $image = Image::create(array());
        ImageService::created($image);

        // Make sure a new folder was created.
        $path = ImageService::getPath($image);
        $this->assertTrue(is_dir($path));

        // Cleanup.
        $image->delete();
        ImageService::deleted($image);
        $this->assertFalse(is_dir($path));
    }

    public function testEntityCleanup()
    {
        // Create a new image.
        $image = Image::create(array());
        ImageService::created($image);

        // Create a new user.
        $user = User::create(array(
            'image_id' => $image->id
        ));

        // Now delete the image.
        $image->delete();
        ImageService::deleted($image);

        // Reload the user and make sure the image was removed from their profile.
        $user = User::find($user->id);
        $this->assertEquals(0, $user->image_id);

        // Cleanup.
        $user->delete();
    }

}

That's the whole class, as it stands, including my manual invocations of ImageService methods (which should actually be triggered by event bindings, and are when run outside of a testing environment).

[PascalZajac]

I've done some further investigation on this. It seems that the Dispatcher class is re-instantiated between each test run. Some events, like artisan.start, are re-registered on each test run. However, Eloquent model events do not seem to be re-registered - they are registered once the first time the class is referenced anywhere in the test suite, and that's it.

[taylorotwell]

Honestly I don't get your test. You're passing model instances into the events methods. What is that even supposed to do?

[PascalZajac]

Ok, I think the class definition for the Image model may be relevant:

namespace Instinct;

class Image extends \Eloquent {

    /**
     * The database table used by the model.
     *
     * @var string
     */
    protected $table = 'images';

    /**
     * The fields that can be changed via mass assignment.
     * @var array
     */
    protected $fillable = array(
        'extension',
        'title',
        'description'
    );
}

// Register event handlers.
Image::created('ImageService@created');
Image::deleted('ImageService@deleted');

As you can see, in the model's class file I'm defining two event handlers. When an Image model is created, I want a folder to be created on the file system to handle the actual image which will soon be uploaded. When an Image is deleted, I want to remove the files from disk and delete the folder.

Now comes my assumption: since I create the event handlers in the model class file, they are not being reloaded between tests because composer does a require_once on the file. Thereafter it already has the class definition, so the file is never reloaded, and the event bindings are never recreated.

So it seems like the problem isn't with Laravel's event system but rather with where I'm defining my event bindings. Doing so in the model's class file seemed logical so they were clearly grouped together. Is there somewhere else I should put these sort of bindings?

[PascalZajac]

I read back over the documentation for Eloquent events and saw the static boot method (that I had somehow not noticed before). So I duly moved my event bindings into the boot method for each model, but the problem persists - the boot code is only run the first time the model is encountered in the test suite, but the event dispatcher is re-instantiated between each test.

Is there a reason for resetting the event dispatcher?

[taylorotwell]

I would move your event bindings into a separate method like Image::registerEvents(); and then you can call that from the setUp method.

[PascalZajac]

That creates a different set of problems though - the boot method is still called the first time the model is referenced in the test suite, so in the first unit test that uses the class, if you have a setUp method as well, two event listeners will be created.

I appreciate that we've travelled far from the original assertions of this ticket, so I'm happy to open a new one, but the underlying issues remain unresolved - either the Dispatcher class needs to discard attempts to register the same event handler twice, or the Dispatcher should not be re-instantiated between test executions. I'm not sure which is better/possible.

[taylorotwell]

Could also use Model::flushModelEvents to clear them out.

On May 17, 2013, at 11:20 PM, Pascal Zajac notifications@github.com wrote:

That creates a different set of problems though - the boot method is still called the first time the model is referenced in the test suite, so in the first unit test that uses the class, if you have a setUp method as well, two event listeners will be created.

I appreciate that we've travelled far from the original assertions of this ticket, so I'm happy to open a new one, but the underlying issues remain unresolved - either the Dispatcher class needs to discard attempts to register the same event handler twice, or the Dispatcher should not be re-instantiated between test executions. I'm not sure which is better/possible.

— Reply to this email directly or view it on GitHub.

[PascalZajac]

Ah ok, that method (which is actually Model::flushEventListeners) hadn't been added the last time I updated. That's neater, now I can just maintain an array of classes to flush and reboot between tests where I need to. Thank you.

[markalanevans]

So is the solution for unit tests to

Create Model::registerEvents();

Call that method in the boot method.

Then in unit tests, in the setup method call $modelsToUpdate = array('User', 'Group', ... etc)

And loop through the models to reset the events? Model::flushEvenListeners() Model::registerEvents();

[PascalZajac]

@markalanevans Laravel already looks for a static boot method on the models under normal circumstances, so I wound up with code in app/tests/TestCase.php along the lines of:

public function setUp()
{
    parent::setUp();
    $this->resetEvents();
}
private function resetEvents()
{
    // Define the models that have event listeners.
    $models = array('Calendar', 'Event', ...);

    // Reset their event listeners.
    foreach ($models as $model) {

        // Flush any existing listeners.
        call_user_func(array($model, 'flushEventListeners'));

        // Reregister them.
        call_user_func(array($model, 'boot'));
    }
}

[buzzware]

This is a hack or a work-around, not a solution to a problem that still remains in the framework (as far as I know)

[tomzx]

I've face this similar problem recently (see #4036), and I've investigated quite a bit. Sadly, most of the problem is related to static/non-static behavior (the Model class having a static $booted relying on a static $dispatcher which changes on a new application).

A potential solution (which is far from the best) would be to reset the booted models when the dispatcher used is changed in Model::setEventDispatcher.

    public static function setEventDispatcher(Dispatcher $dispatcher)
    {
        static::$dispatcher = $dispatcher;
        static::$booted = array();
    }

[acairns]

I agree with @buzzware that this is a workaround and not a fix for the issue. Events being run once then cleared for subsequent tests creates an inconsistent testing environment. Maintaining a list of models which contain observers and reattaching their events isn't a fix - but a workaround.

Maybe models with events can be detected somehow and this functionality can be pushed down into a core TestCase class?

[thsteinmetz]

We just ran into this exact issue setting up our tests after migrating from CodeIgniter. The above-mentioned method works but it is far from ideal.

[aaronpeterson]

Same story here. Bummer.

[anlutro]

The solution to this is to register your model events in app/start/global.php, not the model's boot() method as recommended in the documentation, and especially not in the same file that the model class is defined! That's mixing procedural code with non-procedural and is just a huge no-no.

[PascalZajac]

Can you elaborate @anlutro? I would much prefer to define these event bindings outside the model class, but when I moved them to app/start/global.php they didn't run (according to my unit test suite), even after I wrapped the lines in an App::before() block.

[wlepinski]

Same problem here. Any tips on this?

[harshilmathur]

@PascalZajac: Your solution didn't help as well. It recreates listeners, but still the event isn't triggered on second call from unit tests. I am trying to use the creating event.

[PascalZajac]

@harshilmathur the hack I outlined above last year still works for me in my test suite. Feel free to paste the code you're using in a gist and I'll comment on it.

[eblanshey]

@PascalZajac FYI I have event listeners in the global file and my tests run fine without any hacks.

[PascalZajac]

Hrm I wonder what I did wrong then.

[laurencei]

I have same issue. Event is not triggered on second call from unit tests when using "protected static function boot()" in my model. I had to use @PascalZajac resetEvents() code in my TestCase.php file to fix the issue

[laurencei]

I've modified @PascalZajac code to automatically register and reset every model in the application - rather than having to manually update the array each time:

private function resetEvents()
{
        // Get all models in the Model directory
        $pathToModels = '/vagrant/app/models';
        $files = File::files($pathToModels);

        // Remove the directory name and the .php from the filename
        $files = str_replace($pathToModels.'/', '', $files);
        $files = str_replace('.php', '', $files);

        // Remove "BaseModel" as we dont want to boot that moodel
        if(($key = array_search('BaseModel', $files)) !== false) {
            unset($files[$key]);
        }

        // Reset each model event listeners.
        foreach ($files as $model) {

            // Flush any existing listeners.
            call_user_func(array($model, 'flushEventListeners'));

            // Reregister them.
            call_user_func(array($model, 'boot'));
        }
    }

Edit: this is just a temp workaround until a proper Laravel solution is done.

[harshilmathur]

All hacks & bypasses aside, but why isn't the issue fixed in laravel itself? or is it the intended behavior because it seems the issue is closed since long?

[wlepinski]

I agree with @harshilmathur I think a solution for this kind of issue should be provided by Laravel and should be part of the framework.

[slava-vishnyakov]

I'm not sure whether the comparison would be apt, but in Rails (which is ideologically very close to Laravel) this works out-of-box. You define an watcher ('before_save' for ex.) and it just works. In tests, in production, everywhere, without the need for manual binding.

[crynobone]

All hacks & bypasses aside, but why isn't the issue fixed in laravel itself? or is it the intended behavior because it seems the issue is closed since long?

https://github.com/laravel/framework/issues/1181#issuecomment-42636346

[slava-vishnyakov]

But, the next response to https://github.com/laravel/framework/issues/1181#issuecomment-42636346 says that it does not work, isn't it?

[crynobone]

@slava-vishnyakov have you tried it? I been testing model observer class without any issue all this while using unit testing approach (no hack needed). Also has anyone tried running the test using phpunit --process-isolation flag?

[slava-vishnyakov]

@crynobone You're right. I should try it.

Yes, actually @anlutro's way works!

https://github.com/slava-vishnyakov/fake-model-laravel/compare/4d72ab985091bc748d2816144e4846de202b2b80...8487ec9c20bc68d2aac8187a490feae264f7c3ab

Maybe we should get this added to Laravel Docs? http://laravel.com/docs/eloquent#model-events

[harshilmathur]

@crynobone : @anlutro's comment has other sets of issues. The method doesn't work if you bind the event to the parent class and expect it to work for child class's event. The boot method works for that. This question explains what I mean: http://stackoverflow.com/questions/21517258/laravel-eloquent-event-inheritance Currently, I have to declare events using global.php for all child classes which isn't very nice way to do it if you have lot of child classes requiring same event binding.

[alexandre-butynski]

I agree with @harshilmathur, model event binding inheritance is a very great feature and it needs to declare bindings in the boot() method.

And if mixing procedural with non-procedural code is a problem for somebody here, we have to remove PHP right now ! There is a lot of code in models that is close to configuration code ($fillable attribute, relationship declaration...) and, for me, event binding is in the same category (they are just one line functions).

[sorbing]

I think a solution for this kind of issue should be provided by Laravel and should be part of the framework.

+1, Will be work Model Events out of the box? Thanks.

[ghost]

@harshilmathur I found your comment when searching for a way around that exact issue today. I ended up doing the following:

Event::listen('eloquent.saving: *', function($model) {
  if ($model instance of \My\Base\Model) {
    // event code here
  }
});

This allowed all of my models to inherit the event handling code without binding to any Eloquent models in 3rd party packages. Did you happen to come up with a better way?

[harshilmathur]

@travis-rei I ended up overriding the save function of the eloquent model.php in my eloquent model class to do what I wanted and then call the parent's save function. This solved all my issues because it can now be unit tested without issues as well as inherited saving me from writing same function in multiple child classes.

[dwightwatson]

Throwing some support in for a solution here. When using third party packages model events have to be registered in the boot() method, registering them in global.php isn't really an option. Would be great to see something baked in instead of having to hack it together.

[anlutro]

When using third party packages model events have to be registered in the boot() method

Problem being?

[dwightwatson]

The discussion above seemed to indicate that this problem could be resolved if the model events were registered in global.php instead of in the boot() method.

[anlutro]

Ah sorry, let me clarify. In a package, the equivalent of app/start/global.php is its service provider's boot() method, which is where you would place event listeners - not in a model's boot() method.

[dwightwatson]

Ah, gotcha. This still wouldn't work in my instance (referring to watson/validating where it's a trait that registers an observer when the trait is booted on a model.

[Flightfreak]

I ran in the same issue today and seem to have fixed it by adding Model::flushEventListeners(); and Model::boot(); before each subsequent test. (sugested by PascalZajac) Should we not have some mention of this in the docs?

[davidwinter]

Very frustrating. Have just spent quite some time debugging this and then stumbled across this issue.

I'm using @dwightwatson great watson/validating package, and it seems really hacky in my tests to flush and reboot before each. And if it is the road we should be using, then I think this should be mentioned somewhere in the unit testing docs.

[ghost]

oh no, I've struggled with this problem for more than 3 hours and then I just now find now there are a lot of related issues in here. Can't believe it already exists for so long and Laravel doesn't want solve it at all?!

[davidwinter]

@taylorotwell any new thoughts on this? Do you consider this not to be an issue with Laravel, and we should just use the fixes above? If not, maybe re-open the issue so that other people can find it more easily?