Paths

Table of Contentst

-
services/tunnelbroker/test/
-
tunnelbroker/
-
test/
-
CryptoToolsTest.cpp

[services] Tunnelbroker - CryptoTools tests.
ClosedPublic
Actions

Authored by • max on Apr 7 2022, 10:45 PM.

Details

Reviewers

• karol
tomek
varun

Commits

rCOMM677b6ce9826f: [services] Tunnelbroker - CryptoTools tests.

Summary

CryptoTools - is an internal toolset of functions for cryptography.

We perform tests on a rsaVerifyString() function, that checks the verification message signature signed by the public key provided.

Test Plan

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

Diff Detail

Repository

rCOMM Comm

Branch

tunnelbroker-tests-cryptotools

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• max created this revision.Apr 7 2022, 10:45 PM

Herald added subscribers: • abosh, • benschac, • karol and 4 others. · View Herald TranscriptApr 7 2022, 10:45 PM

• max edited the summary of this revision. (Show Details)Apr 7 2022, 10:46 PM

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

Harbormaster completed remote builds in B8001: Diff 11206.Apr 7 2022, 10:50 PM

• max requested review of this revision.Apr 7 2022, 10:50 PM

• karol accepted this revision.Apr 8 2022, 12:11 AM

tomek requested changes to this revision.Apr 8 2022, 12:39 AM

tomek added inline comments.

services/tunnelbroker/test/CryptoToolsTest.cpp
13–36 ↗	(On Diff #11206)	It's usually a good practice to have only one assertion in a test. The benefit is that by looking at tests result you exactly know which assertions were violated. Also, when a test has multiple assertion, the execution stops on the first failure and we don't know what would be the result of following assertions. So please split this test into two.

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

Split the test into two.

• max marked an inline comment as done.Apr 11 2022, 5:24 AM

• max added inline comments.

services/tunnelbroker/test/CryptoToolsTest.cpp
13–36 ↗	(On Diff #11206)	It's usually a good practice to have only one assertion in a test. The benefit is that by looking at tests result you exactly know which assertions were violated. Also, when a test has multiple assertion, the execution stops on the first failure and we don't know what would be the result of following assertions. So please split this test into two. That makes sense to me. Split it into two.

Harbormaster completed remote builds in B8058: Diff 11277.Apr 11 2022, 5:28 AM

tomek accepted this revision.Apr 11 2022, 5:50 AM

tomek added inline comments.

services/tunnelbroker/test/CryptoToolsTest.cpp
13	We should be able to use simple tests https://google.github.io/googletest/primer.html#simple-tests instead of fixtures https://google.github.io/googletest/primer.html#same-data-multiple-tests. `TEST` is used when you don't need to share data between tests and `TEST_F` when you need that.

This revision is now accepted and ready to land.Apr 11 2022, 5:50 AM

• max marked 2 inline comments as done.Apr 18 2022, 3:19 AM

• max added inline comments.

services/tunnelbroker/test/CryptoToolsTest.cpp
13	We should be able to use simple tests https://google.github.io/googletest/primer.html#simple-tests instead of fixtures https://google.github.io/googletest/primer.html#same-data-multiple-tests. `TEST` is used when you don't need to share data between tests and `TEST_F` when you need that. Good catch! Thanks, @palys-swm.

Switched from TEST_F to TEST, because we don't use a fixtures here.

I think it can be landed without re-reviewing, because of such a tiny change.

Harbormaster completed remote builds in B8248: Diff 11537.Apr 18 2022, 3:28 AM

Closed by commit rCOMM677b6ce9826f: [services] Tunnelbroker - CryptoTools tests. (authored by • max). · Explain WhyApr 18 2022, 3:38 AM

This revision was automatically updated to reflect the committed changes.

• max added a commit: rCOMM677b6ce9826f: [services] Tunnelbroker - CryptoTools tests..

Revision Contents
Changeset List

Path

Size

services/

tunnelbroker/

test/

CryptoToolsTest.cpp

45 lines

Diff 11277

View Options

services/tunnelbroker/test/CryptoToolsTest.cpp

Show First 20 Lines • Show All 144 Lines • ▼ Show 20 Lines	render(): React.Node {
if (messageActionButtons) {		if (messageActionButtons) {
const actionLinksClassName = classNames({		const actionLinksClassName = classNames({
[css.messageActionActiveArea]: true,		[css.messageActionActiveArea]: true,
[css.viewerMessageActionActiveArea]: isViewer,		[css.viewerMessageActionActiveArea]: isViewer,
[css.nonViewerMessageActiveArea]: !isViewer,		[css.nonViewerMessageActiveArea]: !isViewer,
});		});

messageActionLinks = (		messageActionLinks = (
<div className={actionLinksClassName}>		<div className={actionLinksClassName}>{messageActionButtons}</div>
<div className={css.messageActionContainer}>
atulAuthorUnsubmitted Done Inline Actions We push this styling "down" to the actual `MessageActionButtons` component. atul: We push this styling "down" to the actual `MessageActionButtons` component.
{messageActionButtons}
</div>
</div>
);		);
}		}

const viewerActionLinks = isViewer ? messageActionLinks : null;		const viewerActionLinks = isViewer ? messageActionLinks : null;
const nonViewerActionLinks = !isViewer ? messageActionLinks : null;		const nonViewerActionLinks = !isViewer ? messageActionLinks : null;

let inlineSidebar = null;		let inlineSidebar = null;
if (item.threadCreatedFromMessage) {		if (item.threadCreatedFromMessage) {
▲ Show 20 Lines • Show All 74 Lines • Show Last 20 Lines

[services] Tunnelbroker - CryptoTools tests.ClosedPublicActions