Details

Reviewers

• karol
tomek
varun

Commits

rCOMMecbd02a33956: [services] Tunnelbroker - AMQP tests

Summary

AMQPManager - is an internal class for use with the RabbitMQ message broker. It performs connect and re-connect, sends and receive messages from the queue.
When the AMQPManager receives the message it pushes it to the DeliveryBroker local queue.

AMQP tests that we perform:

Send by AMQPManager and Listen -> Receive message by DeliveryBroker.

Test Plan

Run yarn test-tunnelbroker-service all tests are run and succeed.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• max created this revision.Apr 7 2022, 9:05 AM

Herald added subscribers: • abosh, • benschac, • karol and 4 others. · View Herald TranscriptApr 7 2022, 9:05 AM

• max added reviewers: • karol, tomek, varun.Apr 7 2022, 9:06 AM

• max added a parent revision: D3279: [services] Tunnelbroker - Fix delivering messages one by one and remove mutex..

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 7 2022, 9:08 AM

Harbormaster failed remote builds in B7980: Diff 11184!

• max requested review of this revision.Apr 7 2022, 9:22 AM

Why do you put a dot . at the end of your diffs' titles? I haven't noticed anybody else on the team doing this (but maybe I missed something). Can we be consistent about this?

services/tunnelbroker/test/AmqpManagerTest.cpp
24–27 ↗	(On Diff #11184)	We probably shouldn't use randomly generated data for tests.

This revision now requires changes to proceed.Apr 8 2022, 12:19 AM

Harbormaster completed remote builds in B7980: Diff 11184.Apr 8 2022, 8:12 AM

• max retitled this revision from [services] Tunnelbroker - AMQP tests. to [services] Tunnelbroker - AMQP tests.Apr 18 2022, 5:21 AM

Added tests with static variables along with generated ones following D3654#102087

In D3653#100486, @karol-bisztyga wrote:

Why do you put a dot . at the end of your diffs' titles? I haven't noticed anybody else on the team doing this (but maybe I missed something). Can we be consistent about this?

Ok, let's do it. Changed it.

Harbormaster completed remote builds in B8252: Diff 11543.Apr 18 2022, 5:52 AM

Verbose output for generated values in case of failure was added.

Harbormaster completed remote builds in B8288: Diff 11584.Apr 19 2022, 12:33 AM

Remove == equality to EQ only.

Harbormaster completed remote builds in B8291: Diff 11587.Apr 19 2022, 1:03 AM

tomek accepted this revision.Apr 19 2022, 2:22 AM

tomek added inline comments.

services/tunnelbroker/test/AmqpManagerTest.cpp
22 ↗	(On Diff #11587)	Please find test names that explain what is actually being tested, e.g. `TestIfPopReturnsTheSameMassageThatWasSend` - probably a little too long but clearly explains what is being tested. It is important, because we should be able to know what would be a reason of failed test without spending too much time reading it.