Make bus queriable

[flohw]

Hi @kbond,

As discussed in #41 and may help #47 as it was a similar question.

I tried to implement it this way. Tell me if I am in the right direction.

If yes, I don't know how to use the EnvelopeCollection object as it requires the TransportInterface. Maybe it only require to create an AbstractEnvelopeCollection which will contains all the assert* methods and a TransportEnvelopeCollection which is the existing EnveloppeCollection (I may need help for deprecation) and a MessageEnveloppeCollection which will only contains the envelopes or maybe the bus which dispatched to be able to call back method as yet.

I certainly will need some help about testing. At the moment there is only one messenger.bus.default bus. But I didn't look much into it for now as you can see. :slightly_smiling_face:

Thank you!

[nikophil]

Hello,

I'd like to propose another approach for the given problem:

After reading #41 I understand the problem is to specifically track messages which have no transport, (they are automagically synchronously by Messenger). What if we force messenger to use TestTransport anyway in this case? This would just require to decorate SendersLocator and yield our transport if it has not found any. Maybe we could opt-in this behavior by a config of the bundle?

I cannot find out if it is clever or a terrible hack :joy:

(After reading #47 it seems the problem is solved by getting the "real" bus. maybe this could be closed?)

[flohw]

After reading #41 I understand the problem is to specifically track messages which have no transport, (they are automagically synchronously by Messenger). What if we force messenger to use TestTransport anyway in this case? This would just require to decorate SendersLocator and yield our transport if it has not found any. Maybe we could opt-in this behavior by a config of the bundle?

I cannot find out if it is clever or a terrible hack joy I am not sure to understand the whole idea. I am still not very confident with these service locators and didn't dive a lot in Messenger. :grimacing: Can you detail a bit please?

(After reading #47 it seems the problem is solved by getting the "real" bus. maybe this could be closed?)

I am not sure because of autowiring. If I want the real bus I have to know the parameter to pass to self::getContainer()->get() and if I remember correcly, passing MessageBusInterface::class.' $queryBus' doesn't work. But maybe I don't use the right parameter... :thinking:

Thank you again both of you for your help and advices :slightly_smiling_face:

[flohw]

Test fails because of self deprecation. Not sure how to fix this yet. :upside_down_face:

[kbond]

Test fails because of self deprecation. Not sure how to fix this yet. upside_down_face

You can add the @group legacy annotation above the test to indicate you "expect a deprecation". Leave just 1 test to use ->messenger() and change the others to use ->transport().

[nikophil]

I am not sure to understand the whole idea. I am still not very confident with these service locators and didn't dive a lot in Messenger. grimacing Can you detail a bit please?

My idea is something like: we can have a workaround to force messenger use our TestTransport, then, there would be no need to decorate the message bus. But I'm not 100% certain it would be a good idea

I am not sure because of autowiring. If I want the real bus I have to know the parameter to pass to self::getContainer()->get() and if I remember correcly, passing MessageBusInterface::class.' $queryBus' doesn't work. But maybe I don't use the right parameter...

Indeed $container->get(MessageBusInterface::class.' $queryBus' ) won't work, as it is an "autowiring target" (not sure of the naming). But any declared bus has an id with the same name, something like command.bus, query.bus...

[flohw]

Hello there!

I updated with your feedbacks. One test is still failing due to an indirect deprecation and the @group legacy can't be added to this test.

I don't know why some of the tests are marked cacelled btw. :thinking:

I hope that what I've done is correct and can add missing test. :slightly_smiling_face:

Thanks!

[nikophil]

Hi @flohw

I don't know why some of the tests are marked cacelled btw.

this is because the tests are declared in a GitHub's "matrix" and by default, the fail-fast option is activated, which cancels other running jobs in the matrix once one fails: https://docs.github.com/fr/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstrategyfail-fast

One test is still failing due to an indirect deprecation

Deprecation helper is configured this way <server name="SYMFONY_DEPRECATIONS_HELPER" value="max[self]=0&amp;max[direct]=0"/>, so the indirect deprecation is not the culprit.

But we also have failOnRisky="true" in phpunit.xml.dist, and your test can_detect_buses does not perform any assertions, so it is considered as risky and the CI fails

[nikophil]

about the tests, you'll need to add some new messenger configurations in tests/Fixtures/config. Then you can enable this config in a test by calling: self::bootKernel(['environment' => 'your_conf_name']); you'll find plenty of examples in InteractsWithMessengerTest

You can then define the messenger's configuration you want to test. I'd say:

• basic config with no routing
• some more complex config, with two buses, and assert that $this->bus($busName) only returns its own dispatched message

[flohw]

Thank you for your quick review @nikophil

Before adding some tests based on your recommendations, I would like to validate a bit more what I've done. I completed the test I created to assert that this first part works as expected... And it does! :partying_face:

[flohw]

Nice. Thanks a lot for all these tips and reminder.

Tests now. :-)

[flohw]

Hi @kbond and @nikophil,

I completed the readme. Let me know if I need to update something else. :slightly_smiling_face:

I completed the tests but I have some doubt on some of them. Mainly on retried_messages_are_handled because I am not sure to understand the configuration I created completely. And the second strange thing: when the test is executed on its own (php vendor/bin/phpunit -c phpunit.xml.dist --filter retried_messages_are_handled) with this error Expected to find "Zenstruck\Messenger\Test\Tests\Fixture\Messenger\MessageA" 1 times but found 4 times. (line 118) but executed in the suite the test pass.

I hope the unit test on TestBus and TestBusRegistry are find, it was easier and a bit more logic for me to test these cases in dedicated unit tests instead of in the InteractsWithMessengerTest.

Thank you.

[flohw]

Hopefully latest review :-)

Tests are failing due to deprecation on phpunit... Let me know if I have something else to do about it.

[kbond]

Tests are failing due to deprecation on phpunit... Let me know if I have something else to do about it.

I fixed this in #57, rebasing this PR will fix.

[kbond]

We'll also need to add "symfony/deprecation-contracts": "^2.2|^3.0", to the require in composer.json.

[flohw]

:partying_face:

I hope I didn't forget anything. :slightly_smiling_face:

[flohw]

Hi @kbond

Any idea if the release could be available soon? It may help us a bit. :slightly_smiling_face:

Thank you!

[kbond]

Just released as 1.7.0 now! Thanks for the reminder.

[flohw]

Yeah! :grinning:

[flohw]

First usage of this feature in the project. Proud. :smiling_face_with_three_hearts: