search

zenstruck / messenger-test

Assertions and helpers for testing your symfony/messenger queues.
MIT License
223 stars 15 forks source link

Adding listeners to TestTransport event dispatcher #23

Closed dbrumann closed 2 years ago

dbrumann commented 2 years ago

I am using EventSubscribers in my application that listen to some of the worker events. When using the messenger tests those subscribers are not called because the TestTransport will create its own EventDispatcher and there is no way to access it to register new listeners/subscribers.

Simply injecting the event dispatcher in the constructor is probably not a good idea, because it might mess up the priority necessary for the test listeners added there, but maybe there is another way to make it possible to add custom logic to the EventDispatcher?

kbond commented 2 years ago

Good idea!

because it might mess up the priority necessary for the test listeners added there

Do you think so? The listeners I add are all about stopping the worker - just like the ones added in the consume command (at the same priority).

dbrumann commented 2 years ago

I am not sure. It's definitely worth a try as that would probably be the easiest way to solve this. I will give a it a try and see if it works with the subscribers in my use case.

kbond commented 2 years ago

Ok, are you just going to hack it up in your app to test? I can create a PR for you to test if you'd like.

One thing to note: the event dispatcher would need to be cloned (probably by creating a new EventDispatcher and adding all the listeners from the service) before adding the event listeners in TestTransport::process().

kbond commented 2 years ago

PR #26. Test suite passes at least.. If you can, test in your app and let me know if it works as you expect.

dbrumann commented 2 years ago

Thanks for the quick response. I will have a look at it in the next few days and try it out.

dbrumann commented 2 years ago

I just tried it in my project and it works like a charm. I don't even have to manually register my subscribers, because autowiring kicks in and provides the default event dispatcher. :+1:

dbrumann commented 2 years ago

Let me know if I can help with the PR (other than reviewing it, once it leaves the draft state) and thanks again.

kbond commented 2 years ago

Thanks for testing @dbrumann. Think I'm good with that PR, I'll merge once CI is happy, then create a release.

It's possible there could be some circumstances where you'd want to disable using the app's event dispatcher. I can add this feature when/if someone asks for it. I think being enabled by default is best as it more accurately mimics your production setup. WDYT?

dbrumann commented 2 years ago

I think being enabled by default is best as it more accurately mimics your production setup. WDYT?

Yes, absolutely. The PR's behavior is exactly what I initially expected when writing my tests.

kbond commented 2 years ago

Released in 1.1.