zenstruck / messenger-test

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

Check stamps and final message #5

Open CharloMez opened 3 years ago

CharloMez commented 3 years ago

Is there a way to check stamps and the final message with this library ? I explain myself. I use messenger with rabbitmq, and between different microservices. To perform that, I use a specific stamp to give the routing_key dynamically when i dispatch the message

    $envelope = new Envelope(
            $fooMessage,
            [
                new AmqpStamp("myexchange.{$bar}")
            ]
    );

And secondly I use symfony serializer to not pair the message with specific class.

    messenger:
        serializer:
            default_serializer: messenger.transport.symfony_serializer
            symfony_serializer:
                format: json
                context: { }

and the normalizer:

<?php

namespace App\Serializer\Normalizer;

use Symfony\Component\Messenger\Envelope;
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;

class FooNormalizer implements NormalizerInterface
{
    /**
     * @param Envelope $object
     * @param string|null $format
     * @param array $context
     *
     * @return array
     */
    public function normalize($object, string $format = null, array $context = [])
    {
        /** @var FooMessage $fooMessage */
        $fooMessage = $object->getMessage();

        return [
            'body' => [
                'foo' => $fooMessage->bar(),
            ],
            'headers' => [
                'stamps' => $object->all()
            ]
        ];
    }

    public function supportsNormalization($data, string $format = null)
    {
        return $data instanceof Envelope
            && $data->getMessage() instanceof FooMessage
        ;
    }
}

I would like to perform fonctional testing on it, and for me it's very important to check that my stamp is good and that my final message too (I mean the final json which will really be in my queue).

I saw that you have the serializer in your transport class, is it possible to provide a function to get the serialized message ? And I don't know how yet, but do you think it is possible to have a function to verify stamps instance and content ?

I don't have time to watch now, but if it is a good idea for you, i can look at it later and make a PR.

thanks !

kbond commented 3 years ago

but do you think it is possible to have a function to verify stamps instance and content

In your case, would the stamp be available on the envelope?

foreach ($this->messenger()->queue() as $envelope) {
    $envelope->all(); // is the listed stamp here?
}
CharloMez commented 3 years ago

Just check it, yes this works fine for stamps my bad i didn't see

        $stamps = [];
        foreach ($this->transport('foo_transport')->queue() as $envelope) {
            $stamps = $envelope->all();
        }

        $this->assertArrayHasKey(AmqpStamp::class, $stamps);
        $this->assertSame('myexchange.bar_type', $stamps[AmqpStamp::class][0]->getRoutingKey());

But it could be more friendly i think. It would be really great to provide some assert and accessors like for messages:

public function assertStampsContains | notContains(string $stampClass)

public function stamps(string $Class)

And for the serialized message I thought about a property serializedMessages in your TestTransport.php

and in your EnvelopeCollection.php a

public function getSerializedMessage(?string $class = null)

which get the serialized message which represent the $class message

What do you think about it ?

kbond commented 3 years ago

But it could be more friendly i think.

Absolutely! I'm thinking about how the API for this should work though. I'm trying to think where the assertStamps(Not)Contains() assertions should live.

public function getSerializedMessage(?string $class = null)

Still trying to understand this one - you want this method to return the serialized string for the message?

kbond commented 3 years ago

Currently, it's difficult to get the exact envelope/message you want if there are multiple messages sent with the same message class. Perhaps #3 could help but instead of returning Envelope it returns a wrapped TestEnvelope with the stamp assertions on it?

CharloMez commented 3 years ago

public function getSerializedMessage(?string $class = null)

Still trying to understand this one - you want this method to return the serialized string for the message?

Yes thats what I want, but serialized by the right serializer, the one I configure in messenger

    messenger:
        serializer:
            default_serializer: messenger.transport.symfony_serializer
            symfony_serializer:
                format: json
                context: { }

It's possible to configure a specific serializer per transport like this:

external_messages:
    dsn: '%env(MESSENGER_TRANSPORT_DSN)%'
    serializer: App\Messenger\ExternalJsonMessageSerializer

Like explain here https://symfonycasts.com/screencast/messenger/transport-serializer

So it's not a complete functionnal test since we can't check the output of these serialisation

CharloMez commented 3 years ago

Absolutely! I'm thinking about how the API for this should work though. I'm trying to think where the assertStamps(Not)Contains() assertions should live.

Maybe creating a StampCollection with these function and in TestTransport.php a function

    public function stamp(string $queueName): StampCollection

I feel we loose the queue name in your lib, but a function like this would be great.
Like that in test we could do

$this->transport('my_transport')->stamp('queue_name')->assertContains(AmqpStamp::class);

Would be the mirror of this conf:

        transports:
            my_transport:
                dsn: '%env(MESSENGER_DEFAULT_DSN)%'
                options:
                    ...
                    queues:
                        queue_name:
                            binding_keys:
                                ....
kbond commented 3 years ago

The "queue_name" is an option that not all transports have, isn't it? The AmqpStamp is specific to the AmpqTransport so it just wouldn't be there when using the test transport.

kbond commented 3 years ago

For the stamp assertions, I was thinking these would only be applicable for "user-defined" stamps (ie DelayStamp).

CharloMez commented 3 years ago

The "queue_name" is an option that not all transports have, isn't it? The AmqpStamp is specific to the AmpqTransport so it just wouldn't be there when using the test transport.

hmm yeah right, amqb have specific conf, cf. doc:

    messenger:
        transports:
            async_priority_high:
                dsn: '%env(MESSENGER_TRANSPORT_DSN)%'
                options:
                    # queue_name is specific to the doctrine transport
                    queue_name: high

                    # for AMQP send to a separate exchange then queue
                    #exchange:
                    #    name: high
                    #queues:
                    #    messages_high: ~
                    # or redis try "group"
            async_priority_low:
                dsn: '%env(MESSENGER_TRANSPORT_DSN)%'
                options:
                    queue_name: low

Complicate to handle everything

kbond commented 3 years ago

Just check it, yes this works fine for stamps my bad i didn't see

I'm not seeing how this code snipped works? How is the AmpqStamp being added when using this lib's TestTransport?

CharloMez commented 3 years ago

For the stamp assertions, I was thinking these would only be applicable for "user-defined" stamps (ie DelayStamp).

Mostly yes, but I think not only. Some library/bundles can apply specific stamps based on annotations in the application (I mainly think about api-platform, but I don't have example). In that case, you want to check tier library/bundles stamps to be sure you configure in the right way

kbond commented 3 years ago

I'm not seeing how this code snipped works?

Forget this - I see now you're adding it (it's user defined). I thought it was something the AmpqTransport added....

kbond commented 3 years ago

So to summarize my understanding here, I'm seeing two separate features:

  1. Easy access to and assertions on envelope stamps. Could an api like this work?
    $this->messenger('my_transport')->queue()->first(MyMessage::class)->assertHasStamp(AmpqStamp::class);
  2. Access the raw serialized string for a message. Could an api like this work?
    $serializedString = $this->messenger('my_transport')->queue()->first(MyMessage::class)->serialized();

    I'm still trying to find the value of this one. Are you going to be making assertions on the raw string?

kbond commented 3 years ago

Some library/bundles can apply specific stamps based on annotations in the application

What I meant is we can't make assertions on the stamps different "transports" add.

CharloMez commented 3 years ago

So to summarize my understanding here, I'm seeing two separate features:

  1. Easy access to and assertions on envelope stamps. Could an api like this work?
    $this->messenger('my_transport')->queue()->first(MyMessage::class)->assertHasStamp(AmpqStamp::class);
  2. Access the raw serialized string for a message. Could an api like this work?

    $serializedString = $this->messenger('my_transport')->queue()->first(MyMessage::class)->serialized();

    I'm still trying to find the value of this one. Are you going to be making assertions on the raw string?

Yes that would be great, but with your other comment I understand that if we configure specific serializer per transport we can't have them threw your transport.

kbond commented 3 years ago

Can you not do?

# config/packages/test/messenger.yaml

framework:
    messenger:
        transports:
            async:
                dsn: test://
                serializer: App\Messenger\ExternalJsonMessageSerializer

The TestTransportFactory should create the TestTransport with your configured serializer. Admittedly, this advanced messenger stuff is a little out of my depth.

CharloMez commented 3 years ago

I'm just thinking, but I feel that if we avoid all transport specific conf, it makes tests around it not totally functional. I mean that, for me a functional test should only mock when it go external.

I'm trying to think about how to do. Do you think it is possible to make a CompilerPass which get all configured transport, and proxify all of them with your transport threw decoration ?

You could override just the dsn to work in memory or something like that. You understand what I mean ? sorry for my english :)

kbond commented 3 years ago

The purpose of this library as I see it, is a hybrid of the in-memory/sync transports. It's use is to assert your app is sending the correct message to your async buses with an option to process them (if applicable). I think transport specific stuff is out of this scope.

kbond commented 3 years ago

I can definitely see the use case for asserting stamps you define are added to your messages but as for wiring up your production transports (even the serializer imo) should be left for unit tests/E2E tests.

kbond commented 3 years ago

Question, is your ExternalJsonMessageSerializer equipped to "unserialize" the message or is this done in a completely separate app? Basically, is this causing a problem?

CharloMez commented 3 years ago

In that case for me, with your lib we can perform integration tests but not really functional test. But I understand your point of view.

kbond commented 3 years ago

In that case for me, with your lib we can perform integration tests but not really functional test.

That's a good way to put it.

CharloMez commented 3 years ago

Question, is your ExternalJsonMessageSerializer equipped to "unserialize" the message or is this done in a completely separate app? Basically, is this causing a problem?

Yes the unserialize is in other app, but this line is not a problem it doesn't make error (probably because I use the symfony serializer system).

CharloMez commented 3 years ago

Even with custom serializer it can't makes error, thanks to the interface

CharloMez commented 3 years ago

But it could hide a real error if the serializer in your class is not the same as the one configured in the real transport

kbond commented 3 years ago

So, I'm thinking the stamp assertions are useful and well within the scope of this library. Would that be helpful for you?

CharloMez commented 3 years ago

I'm still trying to find the value of this one. Are you going to be making assertions on the raw string?

To answer to this question, yes that's what i planned to do. Like in behat when you check the output content like this:

And the JSON node "data.my_node" should be equal to "my data"

Actually I used to do that for rabbit message with behat (in previous project with swarrot), with my team we created Swarrot Context that allowed us to do things like

And I should have 1 message in "my_queue"
Then I process 1 message
And the JSON node "data.my_node" should be equal to "my data"

But now we I use messenger and phpunit for functional tests, and I try to do the same kind of assertion

CharloMez commented 3 years ago

So, I'm thinking the stamp assertions are useful and well within the scope of this library. Would that be helpful for you?

Yes that would still be helpful, and the serializedMessage too because your lib use the same serializer as my project

kbond commented 3 years ago

Yes that would still be helpful

Ok, I can work on this feature - it's related to #3.

and the serializedMessage too because your lib use the same serializer as my project

This is because you're setting your custom serializer on the test transport like here? I'm still a little unclear on how to implement this feature - I'll tackle the stamp assertions first and then attempt this with some additional feedback from you.

CharloMez commented 3 years ago

No I configured It globally on messenger like in my first message, so I think your transport got it too.

kbond commented 3 years ago

~Got it~

kbond commented 3 years ago

Forgive me, I'm still trying to understand the serialized string feature. If your serializer is configured globally, TestTransport::send() encodes/decodes, doesn't that tell you it's working correctly? In this context I don't understand the need to get the serialized string.

What I do understand is a transport that encodes a message for an external consumer with a proprietary format and the current app can't decode. I can see you'd want to test the encoded string to see it's in the correct format. Even here though, it makes more sense to me as a unit test on your custom serializer (or perhaps a real E2E test...)

CharloMez commented 3 years ago

The serialization process need to be tested, there is important logic inside.
Yes it is mandatory to test all serialization process by unit tests, but it is not enough. The serialization process can be a full chain of different class (normalizers and decoders) that can call themselves the serializer again, and all of that can be driven by the data. Of courses a large amount of E2E tests can cover it, but I can say same for unit tests "we don't need it because we have E2E". And having E2E tests with async micro services working with rabbit is really really not easy.
For me functional test is an important part of the overall test strategy.

kbond commented 3 years ago

@CharloMez, #6 adds the stamp assertions (here's the documentation for this). WDYT?