Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MQ best practices #126

Open
bodinsamuel opened this issue Aug 7, 2021 · 2 comments
Open

MQ best practices #126

bodinsamuel opened this issue Aug 7, 2021 · 2 comments

Comments

@bodinsamuel
Copy link

Hey,
First and foremost, congrats on shipping this, the content is very great 💪🏻
I wanted to discuss not sure issue are the best place but there is no alternative.

I mostly agree with everything but Use a fake MQ for the majority of testing.
In my experience this is something you do because at the beginning of a project it reduces the cost of CI and helps tracking messages, however it does not translate to real use case. Like the advice to not query DB and always use REST API, I believe MQ should be treated the same way. In production you send something to a black box and maybe get it sometimes, it's important to test this too.

My main point, is that mocking a MQ is simple at first but then you need Fanout, TTL, dead-lettering, ack/nack and your mock is now a mess because you just rewrote your MQ.

Purging a queue is slow and not deterministic. When the purge/delete command arrives, some messages are in transit and the queue will not delete those until it get acknowledgment.

This main argument is understandable but easily defeated by using a prefixed queue name per test with a TTL. No need to purge, no need to wait for delete, tests can be run in parallel, etc...

but at the same time will kill the performance

I don't agree with this, a typical MQ software can hold thousand of queues with a very minimal amount of CPU/RAM.
If your MQ is a bottleneck there is something wrong (unless you are sending millions of jobs per second).
For example, rabbitmq with default configuration will hold the load of a normal CI without any issue and deliver messages in a few ms guaranteed. Even in production with high load you should expect < 10ms for delivery.

Given all of this background, a recommended MQ testing strategy is to use simplistic-fake for the majority of the tests, mostly the tests that cover the app flows

I would say, MQ should only be needed on E2E otherwise that means the code is not well decoupled. That's why it's not a problem to not mock it.


I would also add that in memory mock can be dangerous with language like JS that hold pointer ref. If your tests are running in parallel in the same process, you might end up sharing pointer ref or ending up with the same issue about queue name.

Anyway, it's not a dramatic thing, just as a person who did this mistake multiple times I felt like sharing my POV on this topic.

@goldbergyoni
Copy link
Contributor

@bodinsamuel Welcome aboard ✨. This is not just the place to have this discussion, we created this repo mostly to have these discussions:)

Overall, we're totally in agreement. For the majority of the testing, fake the MQ. But you also need E2E to test advanced scenarios as you mentioned. I believe that this is what we tried to communicate, maybe we emphasized the "mocked" testing too much?

Now to the smaller details:

This main argument is understandable but easily defeated by using a prefixed queue name per test with a TTL

For E2E tests - Sure. For component tests (with fake MQ) - Not sure. Performance and developer experience (DX) are critical, we aim to run all the tests in less than 10 seconds, maybe less than 5. Creating 50 queues in Rabbit will last a few seconds by itself (e.g. going by a lame benchmark that I've conducted, have to measure again).

If your tests are running in parallel in the same process

Is this a valid option? When doing so, you open the door for so many other issues like not able to use test-doubles, can share nothing between tests and many other pains. I also run tests in parallel but cross-process (default for Jest)

Fanout, TTL, dead-lettering, ack/nack

Did you read the code example by chance? If not, I'll be honored to get your feedback. We show there testing of ack/nack with fake MQ that is ~15 lines long. We don't need to test the nack/ack were received and processed correctly - This is the responsibility of the MQ product. We only aim to test that OUR app sent the right flag to the MQ. DLQ testing indeed need a real MQ.

Like the advice to not query DB and always use REST API, I believe MQ should be treated the same way

Yes, and no. I suggest that these decisions should balance TWO factors: Developer-experience AND realism. With DB, it's pretty easy to isolate tests. DBs were built to serve multiple consumers at once. With real MQ, we will achieve realism but sacrifice DX due the difficulty in isolating queues. This is why we suggested writing just a few E2E using real MQ

Thoughts?

@mikicho
Copy link
Contributor

mikicho commented Apr 1, 2022

I really enjoyed this conversation and I think it adds A LOT to the practice.
@goldbergyoni maybe we should tie such discussion to the point itself? in the README
Maybe we can even leverage the new discussions feature. WDYT guys?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants