Paths

Table of Contentst

-
services/blob/test/
-
blob/
-
test/
1/1
DatabaseManagerTest.cpp

[services] Blob - Switch from memcmp to a simple EQ in database tests
ClosedPublic
Actions

Authored by • max on Apr 11 2022, 7:16 AM.

Details

Reviewers

• karol
tomek

Commits

rCOMMfa38e5944b38: [services] Blob - Switch from memcmp to a simple EQ in database tests

Summary

@palys-swm revealed a bug related to the use of memcmp in this case:
The test would not fail if the test item is a prefix of the found from database item, because we're comparing only
item.*someItem.size() bytes. We don't need to compare the sizes, because the switch to the simple comparison of strings solves the
problem.

Related diff comment https://phabricator.ashoat.com/D3686#101268

Test Plan

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

Diff Detail

Repository

rCOMM Comm

Branch

blob-remove-tests-memcmp

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• max created this revision.Apr 11 2022, 7:16 AM

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

• max added reviewers: • karol, tomek.Apr 11 2022, 7:17 AM

Harbormaster completed remote builds in B8074: Diff 11293.Apr 11 2022, 7:21 AM

• max requested review of this revision.Apr 11 2022, 7:21 AM

• karol accepted this revision.Apr 12 2022, 1:23 AM

Thanks for fixing it!

This revision is now accepted and ready to land.Apr 12 2022, 3:12 AM

Rebase on master.

In D3696#101938, @palys-swm wrote:

Thanks for fixing it!

Thank you for revealing this bug!

This revision was landed with ongoing or failed builds.Apr 12 2022, 6:46 AM

Closed by commit rCOMMfa38e5944b38: [services] Blob - Switch from memcmp to a simple EQ in database tests (authored by • max). · Explain Why

This revision was automatically updated to reflect the committed changes.

• max added a commit: rCOMMfa38e5944b38: [services] Blob - Switch from memcmp to a simple EQ in database tests.

Harbormaster completed remote builds in B8116: Diff 11343.Apr 12 2022, 6:47 AM

Revision Contents
Changeset List

Path

Size

services/

blob/

test/

DatabaseManagerTest.cpp

24 lines

Diff 11293

View Options

services/blob/test/DatabaseManagerTest.cpp

Show First 20 Lines • Show All 76 Lines • ▼ Show 20 Lines	"dependencies": {
"sharp": "^0.30.5",		"sharp": "^0.30.5",
"siwe": "^1.1.6",		"siwe": "^1.1.6",
"sql-template-strings": "^2.2.2",		"sql-template-strings": "^2.2.2",
"stream-combiner": "^0.2.2",		"stream-combiner": "^0.2.2",
"tcomb": "^3.2.29",		"tcomb": "^3.2.29",
"twin-bcrypt": "^2.1.1",		"twin-bcrypt": "^2.1.1",
"uuid": "^3.3.3",		"uuid": "^3.3.3",
"web": "0.0.1",		"web": "0.0.1",
"web-push": "^3.5.0"		"web-push": "^3.5.0",
		"ws": "^8.13.0"
		ashoatUnsubmitted Done Inline Actions Realizing this will change the version of `ws` that is imported here. In fact we're already importing a newer version (7.5.9) than our version of `express-ws` uses (5.2.3), but it looks like we only use it for Flow types so it's okay ashoat: Realizing this will change the version of `ws` that is imported [here](https://github.
},		},
"optionalDependencies": {		"optionalDependencies": {
"bufferutil": "^4.0.5",		"bufferutil": "^4.0.5",
"utf-8-validate": "^5.0.7"		"utf-8-validate": "^5.0.7"
},		},
"nodemonConfig": {		"nodemonConfig": {
"delay": "200"		"delay": "200"
},		},
Show All 15 Lines

[services] Blob - Switch from memcmp to a simple EQ in database testsClosedPublicActions