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

Eloquent model events are not triggered when testing #1181

Closed
PascalZajac opened this Issue May 5, 2013 · 55 comments

Comments

Projects
None yet
@PascalZajac

PascalZajac commented May 5, 2013

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

This comment has been minimized.

Show comment
Hide comment
@PascalZajac

PascalZajac May 5, 2013

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.

PascalZajac commented May 5, 2013

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

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell May 5, 2013

Member

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.

Member

taylorotwell commented May 5, 2013

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

This comment has been minimized.

Show comment
Hide comment
@PascalZajac

PascalZajac May 5, 2013

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.

PascalZajac commented May 5, 2013

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

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell May 5, 2013

Member

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.

Member

taylorotwell commented May 5, 2013

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

This comment has been minimized.

Show comment
Hide comment
@PascalZajac

PascalZajac May 6, 2013

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 commented May 6, 2013

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

This comment has been minimized.

Show comment
Hide comment
@PascalZajac

PascalZajac May 12, 2013

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.

PascalZajac commented May 12, 2013

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

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell May 12, 2013

Member

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

Member

taylorotwell commented May 12, 2013

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

@PascalZajac

This comment has been minimized.

Show comment
Hide comment
@PascalZajac

PascalZajac May 12, 2013

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 commented May 12, 2013

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

This comment has been minimized.

Show comment
Hide comment
@PascalZajac

PascalZajac May 13, 2013

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?

PascalZajac commented May 13, 2013

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

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell May 18, 2013

Member

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

Member

taylorotwell commented May 18, 2013

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

@PascalZajac

This comment has been minimized.

Show comment
Hide comment
@PascalZajac

PascalZajac May 18, 2013

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.

PascalZajac commented May 18, 2013

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

This comment has been minimized.

Show comment
Hide comment
@taylorotwell

taylorotwell May 18, 2013

Member

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.

Member

taylorotwell commented May 18, 2013

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

This comment has been minimized.

Show comment
Hide comment
@PascalZajac

PascalZajac May 19, 2013

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.

PascalZajac commented May 19, 2013

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

This comment has been minimized.

Show comment
Hide comment
@markalanevans

markalanevans Jan 16, 2014

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();

markalanevans commented Jan 16, 2014

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

This comment has been minimized.

Show comment
Hide comment
@PascalZajac

PascalZajac Jan 16, 2014

@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'));
    }
}

PascalZajac commented Jan 16, 2014

@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 comment has been minimized.

Show comment
Hide comment
@buzzware

buzzware Apr 4, 2014

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)

buzzware commented Apr 4, 2014

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

This comment has been minimized.

Show comment
Hide comment
@tomzx

tomzx Apr 5, 2014

Contributor

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();
    }
Contributor

tomzx commented Apr 5, 2014

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

This comment has been minimized.

Show comment
Hide comment
@acairns

acairns Apr 16, 2014

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?

acairns commented Apr 16, 2014

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

This comment has been minimized.

Show comment
Hide comment
@thsteinmetz

thsteinmetz Apr 18, 2014

Contributor

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.

Contributor

thsteinmetz commented Apr 18, 2014

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

This comment has been minimized.

Show comment
Hide comment
@aaronpeterson

aaronpeterson May 8, 2014

Same story here. Bummer.

aaronpeterson commented May 8, 2014

Same story here. Bummer.

@anlutro

This comment has been minimized.

Show comment
Hide comment
@anlutro

anlutro May 9, 2014

Contributor

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.

Contributor

anlutro commented May 9, 2014

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

This comment has been minimized.

Show comment
Hide comment
@PascalZajac

PascalZajac May 11, 2014

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.

PascalZajac commented May 11, 2014

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

This comment has been minimized.

Show comment
Hide comment
@wlepinski

wlepinski May 13, 2014

Same problem here. Any tips on this?

wlepinski commented May 13, 2014

Same problem here. Any tips on this?

@harshilmathur

This comment has been minimized.

Show comment
Hide comment
@harshilmathur

harshilmathur May 13, 2014

@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.

harshilmathur commented May 13, 2014

@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

This comment has been minimized.

Show comment
Hide comment
@PascalZajac

PascalZajac May 13, 2014

@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.

PascalZajac commented May 13, 2014

@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

This comment has been minimized.

Show comment
Hide comment
@eblanshey

eblanshey May 14, 2014

Contributor

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

Contributor

eblanshey commented May 14, 2014

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

@alexandre-butynski

This comment has been minimized.

Show comment
Hide comment
@alexandre-butynski

alexandre-butynski May 15, 2014

Contributor

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).

Contributor

alexandre-butynski commented May 15, 2014

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

This comment has been minimized.

Show comment
Hide comment
@Sorbing

Sorbing May 23, 2014

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.

Sorbing commented May 23, 2014

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

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jul 25, 2014

@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?

ghost commented Jul 25, 2014

@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

This comment has been minimized.

Show comment
Hide comment
@harshilmathur

harshilmathur Jul 26, 2014

@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.

harshilmathur commented Jul 26, 2014

@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

This comment has been minimized.

Show comment
Hide comment
@dwightwatson

dwightwatson Jul 29, 2014

Contributor

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.

Contributor

dwightwatson commented Jul 29, 2014

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

This comment has been minimized.

Show comment
Hide comment
@anlutro

anlutro Jul 29, 2014

Contributor

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

Problem being?

Contributor

anlutro commented Jul 29, 2014

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

Problem being?

@dwightwatson

This comment has been minimized.

Show comment
Hide comment
@dwightwatson

dwightwatson Jul 29, 2014

Contributor

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.

Contributor

dwightwatson commented Jul 29, 2014

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

This comment has been minimized.

Show comment
Hide comment
@anlutro

anlutro Jul 29, 2014

Contributor

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.

Contributor

anlutro commented Jul 29, 2014

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

This comment has been minimized.

Show comment
Hide comment
@dwightwatson

dwightwatson Jul 29, 2014

Contributor

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.

Contributor

dwightwatson commented Jul 29, 2014

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

This comment has been minimized.

Show comment
Hide comment
@Flightfreak

Flightfreak Aug 8, 2014

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?

Flightfreak commented Aug 8, 2014

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

This comment has been minimized.

Show comment
Hide comment
@davidwinter

davidwinter Aug 21, 2014

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.

davidwinter commented Aug 21, 2014

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.

@wppd

This comment has been minimized.

Show comment
Hide comment
@wppd

wppd Aug 25, 2014

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?!

wppd commented Aug 25, 2014

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

This comment has been minimized.

Show comment
Hide comment
@davidwinter

davidwinter Aug 25, 2014

@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?

davidwinter commented Aug 25, 2014

@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?

@wppd

This comment has been minimized.

Show comment
Hide comment
@wppd

wppd Aug 25, 2014

This is super crazy! Here is my hacky workaround:

    trait ModelEventOverride {
      public $events = [ 'saving'   => 'beforeSave',   'saved'   => 'afterSaved',
                         'creating' => 'beforeCreate', 'created' => 'afterCreated',
                         'updating' => 'beforeUpdate', 'updated' => 'afterUpdated',
                         'deleting' => 'beforeDelete', 'deleted' => 'afterDeleted',
                         'validating' => 'beforeValidate','validated' => 'afterValidated'
                       ];


      protected function fireModelEvent($event, $halt = true)
      {
          $_event = $event;

            if ( isset(static::$dispatcher) ) {

                $event = "eloquent.{$event}: ".get_class($this);

                if ( !empty(static::$dispatcher->getListeners($event)) )
                {
                    $method = $halt ? 'until' : 'fire';
                    return static::$dispatcher->$method($event, $this);
                }

            }

            $event = $_event;

            if ( ! isset($this->events[$event])) return true;

            $method = $this->events[$event];

            if(method_exists($this, $method))
            {
                return call_user_func(array($this, $method),$this);
            }

            return true;
       }
    }

lol

wppd commented Aug 25, 2014

This is super crazy! Here is my hacky workaround:

    trait ModelEventOverride {
      public $events = [ 'saving'   => 'beforeSave',   'saved'   => 'afterSaved',
                         'creating' => 'beforeCreate', 'created' => 'afterCreated',
                         'updating' => 'beforeUpdate', 'updated' => 'afterUpdated',
                         'deleting' => 'beforeDelete', 'deleted' => 'afterDeleted',
                         'validating' => 'beforeValidate','validated' => 'afterValidated'
                       ];


      protected function fireModelEvent($event, $halt = true)
      {
          $_event = $event;

            if ( isset(static::$dispatcher) ) {

                $event = "eloquent.{$event}: ".get_class($this);

                if ( !empty(static::$dispatcher->getListeners($event)) )
                {
                    $method = $halt ? 'until' : 'fire';
                    return static::$dispatcher->$method($event, $this);
                }

            }

            $event = $_event;

            if ( ! isset($this->events[$event])) return true;

            $method = $this->events[$event];

            if(method_exists($this, $method))
            {
                return call_user_func(array($this, $method),$this);
            }

            return true;
       }
    }

lol

@igorpan

This comment has been minimized.

Show comment
Hide comment
@igorpan

igorpan Aug 28, 2014

Just bumped into this one myself in Codeception's acceptance tests. Spent whole morning debugging just to discover that events aren't fired while in test......

igorpan commented Aug 28, 2014

Just bumped into this one myself in Codeception's acceptance tests. Spent whole morning debugging just to discover that events aren't fired while in test......

@wppd

This comment has been minimized.

Show comment
Hide comment
@wppd

wppd Aug 28, 2014

@igorpan yep, I can understand how you feel. been there...

I don't know why, if it can't be fixed, adding a note on the doc is okay too. This could save us some time.

wppd commented Aug 28, 2014

@igorpan yep, I can understand how you feel. been there...

I don't know why, if it can't be fixed, adding a note on the doc is okay too. This could save us some time.

@mk-relax

This comment has been minimized.

Show comment
Hide comment
@mk-relax

mk-relax Sep 4, 2014

I've been using Laravel for three months now and have been very impressed with it so far, but running into this issue after writing only two small testcases for a small Model was an unpleasant (and unexpected) surprise. It took me a couple of hours to find out that it was not my own class, but the test framework itself causing some of my tests to fail. And to pass when ran individually... I came up with the following "solution" which I didn't see in this thread, so I thought I'd share it with you:

As an introduction: I'm using the models boot() method to implement record-level authorization: users may only update a model (in this case a Boat) if they own it. For example:

class Boat extends Eloquent {

    protected static function boot()
    {
        parent::boot();

        static::updating(
            function ($boat) {
                if (Authority::cannot('update', 'Boat', $boat)) {
                    return false;
                }
            }
        );
    }

Some test failed failed because the boot() method on my Model was only called once. The model events like 'update' were only registered with the event dispatcher of the first test, but not with the (new) event dispatchers of following tests because the Model class was already "$booted".

Since you can't "unload" a class in PHP, I decided to add an unboot() method to my model. By calling it from TestCase::tearDown() I tell the model it its no longer "$booted". It will then boot() again during the next test (whenever it instantiates a model). In short:

First, I created the following trait to implement two methods, unbootIfBooted and unboot(). Maybe a bit of overhead, but basically the counterparts of bootIfNotBooted and boot() from the Model. Note that unbootIfBooted() was made public static to be able to call it from a TestCase.

trait UnbootTrait {

    public static function unbootIfBooted()
    {
        $class = get_called_class();

        if (isset(static::$booted[$class])) {
            static::$booted[$class] = null;

            // fireModelEvent('unbooting', false);

            static::unboot();

            // fireModelEvent('unbooted', false);
        }
    }

    protected static function unboot()
    {
    }

}

Then you can simply add this trait to the models you want to test, without the need to change its inheritance:

class Boat extends Eloquent {
    use UnbootTrait;

And finally, tell the TestCase->teardown() method to "unboot" the models it used (usually only the model that's being tested):

class BoatTest extends TestCase {

    protected function tearDown()
    {
       parent::tearDown();
       Boat::unbootIfBooted();
    }

    public function testCanUpdateMyOwnBoat() {}

    public function testCannotUpdateSomeoneElsesBoat() {}

It's just a quick fix, but it works. Comments and suggestions are welcome. I hope it'll be a contribution to a definitive solution to this problem.

mk-relax commented Sep 4, 2014

I've been using Laravel for three months now and have been very impressed with it so far, but running into this issue after writing only two small testcases for a small Model was an unpleasant (and unexpected) surprise. It took me a couple of hours to find out that it was not my own class, but the test framework itself causing some of my tests to fail. And to pass when ran individually... I came up with the following "solution" which I didn't see in this thread, so I thought I'd share it with you:

As an introduction: I'm using the models boot() method to implement record-level authorization: users may only update a model (in this case a Boat) if they own it. For example:

class Boat extends Eloquent {

    protected static function boot()
    {
        parent::boot();

        static::updating(
            function ($boat) {
                if (Authority::cannot('update', 'Boat', $boat)) {
                    return false;
                }
            }
        );
    }

Some test failed failed because the boot() method on my Model was only called once. The model events like 'update' were only registered with the event dispatcher of the first test, but not with the (new) event dispatchers of following tests because the Model class was already "$booted".

Since you can't "unload" a class in PHP, I decided to add an unboot() method to my model. By calling it from TestCase::tearDown() I tell the model it its no longer "$booted". It will then boot() again during the next test (whenever it instantiates a model). In short:

First, I created the following trait to implement two methods, unbootIfBooted and unboot(). Maybe a bit of overhead, but basically the counterparts of bootIfNotBooted and boot() from the Model. Note that unbootIfBooted() was made public static to be able to call it from a TestCase.

trait UnbootTrait {

    public static function unbootIfBooted()
    {
        $class = get_called_class();

        if (isset(static::$booted[$class])) {
            static::$booted[$class] = null;

            // fireModelEvent('unbooting', false);

            static::unboot();

            // fireModelEvent('unbooted', false);
        }
    }

    protected static function unboot()
    {
    }

}

Then you can simply add this trait to the models you want to test, without the need to change its inheritance:

class Boat extends Eloquent {
    use UnbootTrait;

And finally, tell the TestCase->teardown() method to "unboot" the models it used (usually only the model that's being tested):

class BoatTest extends TestCase {

    protected function tearDown()
    {
       parent::tearDown();
       Boat::unbootIfBooted();
    }

    public function testCanUpdateMyOwnBoat() {}

    public function testCannotUpdateSomeoneElsesBoat() {}

It's just a quick fix, but it works. Comments and suggestions are welcome. I hope it'll be a contribution to a definitive solution to this problem.

@Sorbing

This comment has been minimized.

Show comment
Hide comment
@Sorbing

Sorbing Sep 4, 2014

Interesting solution, thanks) Although, I prefer not to think about the need unbooting models when writing Unit tests. Therefore, I prefer the solution based on EloquentEventsMechanic class.
Still, I'm looking forward to that this "feature" will fixed and everything will work out of the box)

Sorbing commented Sep 4, 2014

Interesting solution, thanks) Although, I prefer not to think about the need unbooting models when writing Unit tests. Therefore, I prefer the solution based on EloquentEventsMechanic class.
Still, I'm looking forward to that this "feature" will fixed and everything will work out of the box)

@laravel laravel locked and limited conversation to collaborators Sep 26, 2014

dimsav added a commit to dimsav/baum that referenced this issue Dec 17, 2014

Add the possibility to flush events for phpunit testing
After spending some hours, I discovered that it is required to redefine model events after each single test with phpunit.

[Here](laravel/framework#1181) is a description of the issue.

By putting the events definition to a separate function we allow writing tests for Node instances.

EspadaV8 added a commit to EspadaV8/eloquent-versioned that referenced this issue Jun 9, 2015

Require testbench so PHPUnit tests can make use of Laravel model events
There are a number of issues related to Laravel not firing model events
when using PHPUnit

laravel/framework#1181
laravel/framework#4975

This package seems to help with that

https://github.com/orchestral/testbench

EspadaV8 added a commit to EspadaV8/eloquent-versioned that referenced this issue Jun 9, 2015

Require testbench so PHPUnit tests can make use of Laravel model events
There are a number of issues related to Laravel not firing model events
when using PHPUnit

laravel/framework#1181
laravel/framework#4975

This package seems to help with that

https://github.com/orchestral/testbench

EspadaV8 added a commit to EspadaV8/eloquent-versioned that referenced this issue Jun 9, 2015

Require testbench so PHPUnit tests can make use of Laravel model events
There are a number of issues related to Laravel not firing model events
when using PHPUnit

laravel/framework#1181
laravel/framework#4975

This package seems to help with that

https://github.com/orchestral/testbench

EspadaV8 added a commit to EspadaV8/eloquent-versioned that referenced this issue Jun 10, 2015

Require testbench so PHPUnit tests can make use of Laravel model events
There are a number of issues related to Laravel not firing model events
when using PHPUnit

laravel/framework#1181
laravel/framework#4975

This package seems to help with that

https://github.com/orchestral/testbench
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.