Page MenuHomePhabricator

[services] Tunnelbroker - Tests for `validateUUIDv4` function
ClosedPublic

Authored by max on Jul 7 2022, 4:55 AM.
Tags
None
Referenced Files
F3508841: D4468.diff
Fri, Dec 20, 11:59 PM
F3508771: D4468.id.diff
Fri, Dec 20, 11:54 PM
Unknown Object (File)
Mon, Dec 16, 12:09 AM
Unknown Object (File)
Mon, Dec 16, 12:09 AM
Unknown Object (File)
Mon, Dec 16, 12:09 AM
Unknown Object (File)
Mon, Dec 16, 12:09 AM
Unknown Object (File)
Thu, Dec 12, 8:06 AM
Unknown Object (File)
Thu, Dec 5, 2:21 PM

Details

Summary

This diff introduces tests for a validateUUIDv4 function from D4467 which validate generated UUIDs.

Test Plan

Expected test results on run yarn test-tunnelbroker-service-in-sandbox.

Diff Detail

Repository
rCOMM Comm
Branch
add-validateuuid-tests
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

max held this revision as a draft.
karol added inline comments.
services/tunnelbroker/test/ToolsTest.cpp
44 ↗(On Diff #14250)

nit: maybe we could reuse the string from the previous case and just use a substring here?

We can also add some tests with characters greater than f.
One really important test to add is a failing test with uppercase letters. We're implicitly assuming that all the clients will use lowercase letters for this id, so we should document it somewhere (as a test) to make it more explicit.

This revision is now accepted and ready to land.Jul 8 2022, 2:21 AM

Rebase on stack changes.
Test on invalid format with character greater than F was added.
Test on an uppercase UUID format was added.

max retitled this revision from [services] Tunnelbroker - Tests for `validateUUID` function to [services] Tunnelbroker - Tests for `validateUUIDv4` function.Jul 12 2022, 5:05 PM
max edited the summary of this revision. (Show Details)
In D4468#127460, @palys-swm wrote:

We can also add some tests with characters greater than f.

I've changed the invalid UUID to use a character greater than F instead of the invalid length. That makes more sense.

One really important test to add is a failing test with uppercase letters. We're implicitly assuming that all the clients will use lowercase letters for this id, so we should document it somewhere (as a test) to make it more explicit.

That makes sense to me. I've changed the regex in D4467 to match only the lowercase UUIDs and added the failing test for the uppercase UUID.

services/tunnelbroker/test/ToolsTest.cpp
44 ↗(On Diff #14250)

nit: maybe we could reuse the string from the previous case and just use a substring here?

On the updated version it was changed to use the wrong format with the character greater than F.

This revision was automatically updated to reflect the committed changes.
max marked an inline comment as done.