From 64dba138c1ca626f42145b1ae871eb5ae3061690 Mon Sep 17 00:00:00 2001 From: Priyank Warkhede Date: Tue, 3 Dec 2024 11:34:37 -0800 Subject: [PATCH] test.thrift: define hybrid node access patterns 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/, structMap//, listOfStruct/) Reviewed By: wilsonwinhi Differential Revision: D66525279 fbshipit-source-id: 87febe619c3da504ce5f22577cf9a679af7ea674 --- fboss/thrift_cow/nodes/tests/test.thrift | 21 ++++++++++++++- .../visitors/tests/RecurseVisitorTests.cpp | 26 ++++++++++++++++--- .../visitors/tests/VisitorTestUtils.cpp | 10 +++++-- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/fboss/thrift_cow/nodes/tests/test.thrift b/fboss/thrift_cow/nodes/tests/test.thrift index babed4fb68f35..5667f1ba9b371 100644 --- a/fboss/thrift_cow/nodes/tests/test.thrift +++ b/fboss/thrift_cow/nodes/tests/test.thrift @@ -34,6 +34,9 @@ struct ChildStruct { 1: map childMap; 2: map strMap; 3: map structMap; + 4: list listOfStruct; + 5: i32 leafI32; + 6: optional TestEnum optionalEnum; } struct TestStruct { @@ -75,7 +78,10 @@ struct TestStruct { 29: set hybridSet (allow_skip_thrift_cow = true); 30: TestUnion hybridUnion (allow_skip_thrift_cow = true); 31: ChildStruct hybridStruct (allow_skip_thrift_cow = true); - 32: map 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 hybridMapOfI32ToStruct ( allow_skip_thrift_cow = true, ); 33: map> hybridMapOfMap (allow_skip_thrift_cow = true); @@ -84,8 +90,21 @@ struct TestStruct { ); } +// structs declared to mimic deeper Thrift path accesses struct ParentTestStruct { 1: TestStruct childStruct; + 2: map> mapOfI32ToMapOfStruct; +} + +struct RootTestStruct { + 1: map> mapOfI32ToMapOfStruct; + 2: list listOfStruct; + 3: ParentTestStruct inlineStruct; +} + +// structs declared exclusively for testing Thrift annotations. +struct TestStructForAnnotation1 { + 1: TestStruct childStruct; } (random_annotation) struct TestStruct2 { diff --git a/fboss/thrift_cow/visitors/tests/RecurseVisitorTests.cpp b/fboss/thrift_cow/visitors/tests/RecurseVisitorTests.cpp index d2e65a8047d3e..705621bc3c20a 100644 --- a/fboss/thrift_cow/visitors/tests/RecurseVisitorTests.cpp +++ b/fboss/thrift_cow/visitors/tests/RecurseVisitorTests.cpp @@ -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()); } @@ -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"}, @@ -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"; @@ -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, folly::dynamic> hybridNodes = { {{"mapOfEnumToStruct", "3"}, testDyn["mapOfEnumToStruct"][3]}, @@ -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"; @@ -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]}, @@ -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"; diff --git a/fboss/thrift_cow/visitors/tests/VisitorTestUtils.cpp b/fboss/thrift_cow/visitors/tests/VisitorTestUtils.cpp index 08b6db853d1b9..0dda4da759ae8 100644 --- a/fboss/thrift_cow/visitors/tests/VisitorTestUtils.cpp +++ b/fboss/thrift_cow/visitors/tests/VisitorTestUtils.cpp @@ -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)))(