[services] Tunnelbroker - Fix message `from` and `to` swap in AMQP tests
ClosedPublic
Actions

Authored by • max on Aug 4 2022, 11:40 AM.

Details

Reviewers

• karol
tomek

Commits

rCOMMcdfcba6468e2: [services] Tunnelbroker - Fix message `from` and `to` swap in AMQP tests

Summary

This diff is a fix.

The constructor for the MessageItem entity was changed (fromDeviceID and `toDeviceID was swapped in places), but the tests have not reflected these changes. This diff fixes tests for the message items to reflect these changes.

Related Linear task: ENG-1491

Test Plan

Tests in DatabaseManagerTest.cpp was successfully built and passed in a sandbox.

Diff Detail

Repository

rCOMM Comm

Branch

fix-amqp-tests-from-to-swap

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• max created this revision.Aug 4 2022, 11:40 AM

• max held this revision as a draft.

Herald added subscribers: • abosh, • karol, atul and 3 others. · View Herald TranscriptAug 4 2022, 11:40 AM

Harbormaster completed remote builds in B11134: Diff 15331.Aug 4 2022, 11:50 AM

• max published this revision for review.Aug 4 2022, 12:33 PM

• max edited the summary of this revision. (Show Details)

• max edited the test plan for this revision. (Show Details)

• max added reviewers: • karol, tomek.

• max edited the summary of this revision. (Show Details)

• max edited the test plan for this revision. (Show Details)

• max added a child revision: D4746: [services] Tunnelbroker - Changes in Amqp messages testing to send and listen in different threads.Aug 4 2022, 4:28 PM

• max added a child revision: D4749: [services] Tunnelbroker - Add timeout for a pop message waiting in AmqpManager tests.Aug 4 2022, 4:45 PM

tomek accepted this revision.Aug 5 2022, 5:35 AM

tomek added inline comments.

services/tunnelbroker/test/AmqpManagerTest.cpp
38–39	Is there a way to avoid this confusion in the future? I guess we could use https://en.cppreference.com/w/cpp/language/aggregate_initialization#Designated_initializers but that requires C++20. We can also consider using a builder pattern, where every property needs to be set by using a method with meaningful name.

This revision is now accepted and ready to land.Aug 5 2022, 5:35 AM

• karol accepted this revision.Aug 5 2022, 5:59 AM

• max marked an inline comment as done.Aug 5 2022, 6:22 AM

• max added inline comments.

services/tunnelbroker/test/AmqpManagerTest.cpp
38–39	Is there a way to avoid this confusion in the future? I guess we could use https://en.cppreference.com/w/cpp/language/aggregate_initialization#Designated_initializers but that requires C++20. I think there is some problem with switching to C++20 related to React Native ? Is there a way to avoid this confusion in the future? Running unit tests in CI will solve such sort of problems because it will just fail.