zenstruck / messenger-test

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

Add a way to handle transport specific interfaces #83

Closed aegypius closed 4 weeks ago

aegypius commented 2 months ago

One of my tests fails when using this package if i use a test:// configuration.

To be more precise, I am writing a prometheus collector that iterates against transports and expose the number of message to be processed. For that purpose, I use a similar code as :


$stats = new \ArrayObject([]);

// $transports is autowired with  #[AutowireIterator(tag: 'messenger.receiver', indexAttribute: 'name')]
foreach ($transports as $name => $transport) {
    if (!($transport instanceof \Symfony\Component\Messenger\Transport\Receiver\MessageCountAwareInterface)) {
        continue;
    }

    try {
        [$shortName] = \array_slice(\explode('.', $name), -1);
        $stats->append(new Transport(
            name: $shortName,
            count: $transport->getMessageCount(),
        ));
    } catch (\Symfony\Component\Messenger\Exception\TransportException) {
        continue;
    }
}

If I don't use test:// scheme, the native transport (which hold a MessageCountAwareInterface interface) is properly listed but when I start using this package to tests queues the transport is no longer listed and my integration test fails.

How would you solve this issue ? Do we need to add some sorte of option to the test transport to had these kind of interface ?

nikophil commented 2 months ago

I'm wondering if we should just make TestTransport implement the interface MessageCountAwareInterface? WDYT?

aegypius commented 2 months ago

I'm wondering if we should just make TestTransport implement the interface MessageCountAwareInterface? WDYT?

It would definitely solve this test but would fail if the test does the opposite

nikophil commented 2 months ago

seems like our hands are tied...

nikophil commented 2 months ago

seen directly with @aegypius: a solution would be to decorate with an anonymous class:

new class extends TestTransport implements MessageCountAwareInterface {
   use MessageCountAwareTrait;
}

but this would cause problem with interface composition (ListableReceiverInterface, QueueReceiverInterface, ...).

Let's use a pragmatic solution: since our TestTransport is listable and countable, let's make it implement these interfaces. And we'll think about another complex solution when/if someone complains about the fact it cannot be tested as a "not listable or countable"

any thoughts @kbond ?

aegypius commented 2 months ago

And we'll think about another complex solution when/if someone complains about the fact it cannot be tested as a "not listable or countable"

I promise I will not :rofl: !

kbond commented 2 months ago

Let's use a pragmatic solution: since our TestTransport is listable and countable, let's make it implement these interfaces.

I agree!

It would definitely solve this test but would fail if the test does the opposite

@aegypius, you're fine with not being able to test the opposite (or using another solution for this test)?

aegypius commented 2 months ago

@kbond yes I am fine with this solution