Page MenuHomePhabricator

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

Authored by max on Jul 7 2022, 4:55 AM.
Tags
None
Referenced Files
F2205679: D4468.id14250.diff
Sat, Jul 6, 11:15 PM
F2204204: D4468.id14526.diff
Sat, Jul 6, 3:34 PM
F2204203: D4468.id14418.diff
Sat, Jul 6, 3:34 PM
F2204201: D4468.diff
Sat, Jul 6, 3:33 PM
Unknown Object (File)
Sat, Jul 6, 10:58 AM
Unknown Object (File)
Sat, Jul 6, 1:13 AM
Unknown Object (File)
Fri, Jul 5, 7:52 PM
Unknown Object (File)
Fri, Jul 5, 5:20 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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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.