Details

Reviewers

varun
tomek
• karol

Commits

rCOMM3819740fbf82: [services] Tunnelbroker - Database SessionItem tests

Summary

Database (DynamoDB) item tests. For the SessionItem database entity we perform:

Check if the table is available,
Create and fulfill database item entity,
Put into the database table,
Find the record using the primary key,
Compare record fields of created item and item from database record,
Remove the testing record from the database.

Related linear task: ENG-686

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 11 2022, 3:34 AM

Herald added subscribers: • abosh, • benschac, • karol and 4 others. · View Herald TranscriptApr 11 2022, 3:34 AM

• max retitled this revision from [services] Tunnelbroker - Database SessionItem tests. to [services] Tunnelbroker - Database SessionItem tests.Apr 11 2022, 3:35 AM

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

• max added reviewers: varun, tomek, • karol.

• max added a parent revision: D3643: [services] Tunnelbroker - Database item tests initialization.

Harbormaster completed remote builds in B8054: Diff 11273.Apr 11 2022, 3:39 AM

• max requested review of this revision.Apr 11 2022, 3:39 AM

• max mentioned this in D3643: [services] Tunnelbroker - Database item tests initialization.Apr 11 2022, 3:55 AM

We also need to clarify our approach to randomness in D3654

services/tunnelbroker/test/DatabaseManagerTest.cpp
44–49 ↗	(On Diff #11273)	It's always better to use functions provided by the testing framework, as in case of failure, they provide a lot more useful messages. So in this case, if we wanted to compare char* it would be better to use a combination of `ASSERT_THAT` and `ElementsAre` matcher... but in this case there's no need for that. We want to compare `item.getDeviceID` so why would we want to extract bytes from it? Can't we just compare strings? BTW, your solution would not fail if `item.getDeviceID()` was a prefix of `foundItem->getDeviceID()` because we're comparing only `item.getDeviceID().size()` bytes.

This revision now requires changes to proceed.Apr 11 2022, 5:01 AM

tomek mentioned this in D3687: [services] Tunnelbroker - Database SessionSignatureItem tests.Apr 11 2022, 5:14 AM

Switch from memcmp to a simple EQ.

services/tunnelbroker/test/DatabaseManagerTest.cpp
44–49 ↗	(On Diff #11273)	It's always better to use functions provided by the testing framework, as in case of failure, they provide a lot more useful messages. So in this case, if we wanted to compare char* it would be better to use a combination of `ASSERT_THAT` and `ElementsAre` matcher... but in this case there's no need for that. We want to compare `item.getDeviceID` so why would we want to extract bytes from it? Can't we just compare strings? BTW, your solution would not fail if `item.getDeviceID()` was a prefix of `foundItem->getDeviceID()` because we're comparing only `item.getDeviceID().size()` bytes. This is a good catch! Switched to simple EQ.