Using HasRelationshipObservables trait in multiple models causes double events to be fired in tests
BoyeMagnus opened this issue · 7 comments
When adding the HasRelationshipObservables trait to multiple models, the tests often fail, because of observers receiving the event twice.
For eg. a user and a group model with HasRelationshipObservables. With two observers that subscribe to belongsToManyAttached both.
trait HasRelationshipObservables
{
/**
* @var array
*/
protected static $relationshipObservables = [];
/**
* Initialize relationship observables.
*
* @return void
*/
public static function bootHasRelationshipObservables()
{
......gets the methods
static::mergeRelationshipObservables($methods);
}
Static properties will most of the time in tests never be reset, because of how PHPUnit works. When booting HasRelationshipObservables
, then it merges the $methods
with the static $relationshipObservables
. This causes the observer to be fire multiple times in tests where the static property is never reset.
A solution would be to set $relationshipObservables to an empty array in the bootHasRelationshipObservables. Anyone expiring the same issues?
Thanks, @BoyeMagnus
Could you please provide a test that fails?
Yes I'll provide one asap 😄
Thanks again, i'll look into it tomorrow.
ping @BoyeMagnus
Sorry for the delay, had a pretty rough sprint.
I've made a branch with a test job that is being run every time the event should be happening (In the UserObserver). As you can see the job is being fired more than one time if you have test cases in PHPUnit.
Adding static::$relationshipObservables = [];
to line 28 in Chelout\RelationshipEvents\Traits\HasRelationshipObservables
fixes the problem.
The problem is that the User model is booted multiple times during the tests, and as the package appends the "methods" in the static $relationshipObservables
variable, then it keeps growing for each test.
I've created a PR with the test here: #41
@chelout ping
@BoyeMagnus sorry for the huge delay, working on MVP project for days and nights 🤯
I would accept PR with described changes with provided tests if you have a little more time then me.