Skip to content

Commit

Permalink
test.thrift: define hybrid node access patterns
Browse files Browse the repository at this point in the history
Summary:
Update test struct defintion to cover hybrid node access patterns mimicing prod
Thrift model.
This will be used in next diffs to update thrift_cow node and visitor UTs to
cover follwing access patterns:
* ThriftStructNode UT for EnableHybridStorage template parameter to cover paths
  traversing multiple TCs (map, list, struct) and path length 1 and 2 before
  reaching HybridNode

Visitor UTs to cover Thrift paths of following patterns, starting at RootTestStruct:
* path terminating at HybridNode (annotated member hybridMapOfI32ToStruct)
* path terminating at Thrift container (list, map, set, struct field in ChildStruct)
* path terminating at leaf primitive under HybridNode (primitive leaves
  under ChildStruct: optionalEnum, leafI32, childMap/<key>,
  structMap/<key>/<field>, listOfStruct<key>/<field>)

Reviewed By: wilsonwinhi

Differential Revision: D66525279

fbshipit-source-id: 87febe619c3da504ce5f22577cf9a679af7ea674
  • Loading branch information
Priyank Warkhede authored and facebook-github-bot committed Dec 3, 2024
1 parent 210ce5c commit 64dba13
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 7 deletions.
21 changes: 20 additions & 1 deletion fboss/thrift_cow/nodes/tests/test.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ struct ChildStruct {
1: map<i32, bool> childMap;
2: map<string, i32> strMap;
3: map<string, switch_config.L4PortRange> structMap;
4: list<switch_config.L4PortRange> listOfStruct;
5: i32 leafI32;
6: optional TestEnum optionalEnum;
}

struct TestStruct {
Expand Down Expand Up @@ -75,7 +78,10 @@ struct TestStruct {
29: set<i32> hybridSet (allow_skip_thrift_cow = true);
30: TestUnion hybridUnion (allow_skip_thrift_cow = true);
31: ChildStruct hybridStruct (allow_skip_thrift_cow = true);
32: map<i32, switch_config.L4PortRange> hybridMapOfI32ToStruct (
// hybridMapOfI32ToStruct is crafted to cover deeper accesses inside HybridNode, with
// paths that terminate at primitive leaves, intermediate containers (list, map, struct)
// in UTs for various visitors: PathVisitor, RecurseVisitor, DeltaVisitor
32: map<i32, ChildStruct> hybridMapOfI32ToStruct (
allow_skip_thrift_cow = true,
);
33: map<i32, map<i32, i32>> hybridMapOfMap (allow_skip_thrift_cow = true);
Expand All @@ -84,8 +90,21 @@ struct TestStruct {
);
}

// structs declared to mimic deeper Thrift path accesses
struct ParentTestStruct {
1: TestStruct childStruct;
2: map<i32, map<string, TestStruct>> mapOfI32ToMapOfStruct;
}

struct RootTestStruct {
1: map<i32, map<string, ParentTestStruct>> mapOfI32ToMapOfStruct;
2: list<ParentTestStruct> listOfStruct;
3: ParentTestStruct inlineStruct;
}

// structs declared exclusively for testing Thrift annotations.
struct TestStructForAnnotation1 {
1: TestStruct childStruct;
} (random_annotation)

struct TestStruct2 {
Expand Down
26 changes: 22 additions & 4 deletions fboss/thrift_cow/visitors/tests/RecurseVisitorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ folly::dynamic createTestDynamic() {
"hybridMap", dynamic::object())("hybridList", dynamic::array())(
"hybridSet", dynamic::array())("hybridUnion", dynamic::object())(
"hybridStruct",
dynamic::object("childMap", dynamic::object())(
"strMap", dynamic::object())("structMap", dynamic::object()))(
dynamic::object("childMap", dynamic::object())("leafI32", 0)(
"listOfStruct", dynamic::array())("strMap", dynamic::object())(
"structMap", dynamic::object()))(
"hybridMapOfI32ToStruct", dynamic::object())(
"hybridMapOfMap", dynamic::object());
}
Expand Down Expand Up @@ -146,6 +147,9 @@ TYPED_TEST(RecurseVisitorTests, TestFullRecurse) {
{{"hybridStruct", "childMap"}, testDyn["hybridStruct"]["childMap"]},
{{"hybridStruct", "strMap"}, testDyn["hybridStruct"]["strMap"]},
{{"hybridStruct", "structMap"}, testDyn["hybridStruct"]["structMap"]},
{{"hybridStruct", "leafI32"}, testDyn["hybridStruct"]["leafI32"]},
{{"hybridStruct", "listOfStruct"},
testDyn["hybridStruct"]["listOfStruct"]},
{{"mapOfEnumToStruct"}, testDyn["mapOfEnumToStruct"]},
{{"mapOfEnumToStruct", "3"}, testDyn["mapOfEnumToStruct"][3]},
{{"mapOfEnumToStruct", "3", "min"},
Expand All @@ -166,6 +170,10 @@ TYPED_TEST(RecurseVisitorTests, TestFullRecurse) {
}

EXPECT_EQ(visited.size(), expected.size());
for (auto& [path, dyn] : visited) {
EXPECT_EQ(dyn, visited[path])
<< "Path /" << folly::join('/', path) << " does not match visited";
}
for (auto& [path, dyn] : expected) {
EXPECT_EQ(dyn, visited[path])
<< "Path /" << folly::join('/', path) << " does not match expected";
Expand Down Expand Up @@ -209,7 +217,8 @@ TYPED_TEST(RecurseVisitorTests, TestLeafRecurse) {
{{"mapOfEnumToStruct", "3", "max"},
testDyn["mapOfEnumToStruct"][3]["max"]},
{{"mapOfEnumToStruct", "3", "invert"},
testDyn["mapOfEnumToStruct"][3]["invert"]}};
testDyn["mapOfEnumToStruct"][3]["invert"]},
{{"hybridStruct", "leafI32"}, testDyn["hybridStruct"]["leafI32"]}};

std::map<std::vector<std::string>, folly::dynamic> hybridNodes = {
{{"mapOfEnumToStruct", "3"}, testDyn["mapOfEnumToStruct"][3]},
Expand All @@ -228,6 +237,10 @@ TYPED_TEST(RecurseVisitorTests, TestLeafRecurse) {
}

EXPECT_EQ(visited.size(), expected.size());
for (auto& [path, dyn] : visited) {
EXPECT_EQ(dyn, expected[path])
<< "Path /" << folly::join('/', path) << " does not match visited";
}
for (auto& [path, dyn] : expected) {
EXPECT_EQ(dyn, visited[path])
<< "Path /" << folly::join('/', path) << " does not match expected";
Expand All @@ -254,7 +267,8 @@ TYPED_TEST(RecurseVisitorTests, TestLeafRecurse) {
hybridDeepLeaves = {
{{"15", "3", "1"}, testDyn["mapOfEnumToStruct"][3]["min"]},
{{"15", "3", "2"}, testDyn["mapOfEnumToStruct"][3]["max"]},
{{"15", "3", "3"}, testDyn["mapOfEnumToStruct"][3]["invert"]}};
{{"15", "3", "3"}, testDyn["mapOfEnumToStruct"][3]["invert"]},
{{"31", "5"}, testDyn["hybridStruct"]["leafI32"]}};

hybridNodes = {
{{"15", "3"}, testDyn["mapOfEnumToStruct"][3]},
Expand All @@ -273,6 +287,10 @@ TYPED_TEST(RecurseVisitorTests, TestLeafRecurse) {
}

EXPECT_EQ(visited.size(), expected.size());
for (auto& [path, dyn] : visited) {
EXPECT_EQ(dyn, expected[path])
<< "Path /" << folly::join('/', path) << " does not match visited";
}
for (auto& [path, dyn] : expected) {
EXPECT_EQ(dyn, visited[path])
<< "Path /" << folly::join('/', path) << " does not match expected";
Expand Down
10 changes: 8 additions & 2 deletions fboss/thrift_cow/visitors/tests/VisitorTestUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,17 @@ TestStruct createSimpleTestStruct() {
"cowMap", dynamic::object(1, true))(
"hybridMap", dynamic::object(1, true))(
"hybridMapOfI32ToStruct",
dynamic::object(20, dynamic::object("min", 400)("max", 600)))(
dynamic::object(
20,
dynamic::object("childMap", dynamic::object())("leafI32", 0)(
"listOfStruct", dynamic::array())("strMap", dynamic::object())(
"structMap", dynamic::object())))(
"hybridMapOfMap", dynamic::object(10, dynamic::object(20, 30)))(
"hybridStruct",
dynamic::object(
"childMap", dynamic::object(10, true)(20, false)(50, false)))(
"childMap", dynamic::object(10, true)(20, false)(50, false))(
"leafI32", 0)("listOfStruct", dynamic::array())(
"strMap", dynamic::object())("structMap", dynamic::object()))(
"mapOfStringToI32", dynamic::object("test1", 1)("test2", 2))(
"mapOfI32ToStruct",
dynamic::object(20, dynamic::object("min", 400)("max", 600)))(
Expand Down

0 comments on commit 64dba13

Please sign in to comment.