From f6433934d48e479695c39b1c1c15a79fd3d2b856 Mon Sep 17 00:00:00 2001
From: Wei-Cheng Lin <lwc@meta.com>
Date: Thu, 21 Nov 2024 21:11:40 -0800
Subject: [PATCH] move node type check to its own test case

Summary: as title. a dedicated test case for node type check

Differential Revision: D66272495

fbshipit-source-id: 2c6f6adeffc4e84ce1d8a8a9e4131f9f52dd61a8
---
 .../nodes/tests/ThriftStructNodeTests.cpp     | 83 ++++++++++++-------
 1 file changed, 51 insertions(+), 32 deletions(-)

diff --git a/fboss/thrift_cow/nodes/tests/ThriftStructNodeTests.cpp b/fboss/thrift_cow/nodes/tests/ThriftStructNodeTests.cpp
index 9dd54a135064d..bff0053187cd1 100644
--- a/fboss/thrift_cow/nodes/tests/ThriftStructNodeTests.cpp
+++ b/fboss/thrift_cow/nodes/tests/ThriftStructNodeTests.cpp
@@ -360,7 +360,7 @@ class ThriftStructNodeTestSuite : public ::testing::Test {
 
 TYPED_TEST_SUITE(ThriftStructNodeTestSuite, StorageTestTypes);
 
-TYPED_TEST(ThriftStructNodeTestSuite, ThriftStructNodeModifyTest) {
+TYPED_TEST(ThriftStructNodeTestSuite, NodeTypeTest) {
   using Param = typename TestFixture::T;
   constexpr bool enableHybridStorage = Param::hybridStorage;
 
@@ -372,42 +372,73 @@ TYPED_TEST(ThriftStructNodeTestSuite, ThriftStructNodeModifyTest) {
   data.inlineString() = "HelloThere";
   data.inlineStruct() = std::move(portRange);
   data.mapOfStringToI32() = {{"test1", 1}};
+  data.hybridStruct() = ChildStruct();
 
   auto node = this->initNode(data);
+  node->publish();
+
+  // root is not hybrid node
   static_assert(!std::is_same_v<
                 typename decltype(node)::element_type::CowType,
                 HybridNodeType>);
-  ASSERT_FALSE(node->isPublished());
-  if (!this->isHybridStorage()) {
-    ASSERT_FALSE(node->template cref<k::inlineStruct>()->isPublished());
-  }
-
-  node->publish();
-  ASSERT_TRUE(node->isPublished());
-  if constexpr (!enableHybridStorage) {
-    ASSERT_TRUE(node->template cref<k::inlineStruct>()->isPublished());
-  }
-
-  ThriftStructNode<
-      TestStruct,
-      ThriftStructResolver<TestStruct, enableHybridStorage>,
-      enableHybridStorage>::modify(&node, "inlineStruct");
 
+  // check fields that are not annotated, should never be hybrid
   static_assert(!std::is_same_v<
                 typename decltype(node->template get<
                                   k::inlineStruct>())::element_type::CowType,
                 HybridNodeType>);
+  static_assert(
+      !std::is_same_v<
+          typename folly::remove_cvref_t<
+              decltype(node->template get<k::inlineInt>().value())>::CowType,
+          HybridNodeType>);
+  static_assert(
+      !std::is_same_v<
+          typename folly::remove_cvref_t<
+              decltype(node->template get<k::inlineString>().value())>::CowType,
+          HybridNodeType>);
 
+  // check fields that are annotated, should be hybrid if enabled
   static_assert(
       std::is_same_v<
           typename decltype(node->template get<
                             k::mapOfStringToI32>())::element_type::CowType,
           HybridNodeType> == enableHybridStorage);
+  static_assert(
+      std::is_same_v<
+          typename decltype(node->template get<
+                            k::hybridStruct>())::element_type::CowType,
+          HybridNodeType> == enableHybridStorage);
+}
 
-  if constexpr (!enableHybridStorage) {
-    ASSERT_FALSE(node->isPublished());
-    ASSERT_FALSE(node->template cref<k::inlineStruct>()->isPublished());
-  }
+TYPED_TEST(ThriftStructNodeTestSuite, ThriftStructNodeModifyTest) {
+  using Param = typename TestFixture::T;
+  constexpr bool enableHybridStorage = Param::hybridStorage;
+
+  auto portRange = buildPortRange(100, 999);
+
+  TestStruct data;
+  data.inlineBool() = true;
+  data.inlineInt() = 123;
+  data.inlineString() = "HelloThere";
+  data.inlineStruct() = std::move(portRange);
+
+  auto node = this->initNode(data);
+
+  ASSERT_FALSE(node->isPublished());
+  ASSERT_FALSE(node->template cref<k::inlineStruct>()->isPublished());
+
+  node->publish();
+  ASSERT_TRUE(node->isPublished());
+  ASSERT_TRUE(node->template cref<k::inlineStruct>()->isPublished());
+
+  ThriftStructNode<
+      TestStruct,
+      ThriftStructResolver<TestStruct, enableHybridStorage>,
+      enableHybridStorage>::modify(&node, "inlineStruct");
+
+  ASSERT_FALSE(node->isPublished());
+  ASSERT_FALSE(node->template cref<k::inlineStruct>()->isPublished());
 
   // now try modifying a missing optional field
   ASSERT_FALSE(node->template isSet<k::optionalString>());
@@ -417,12 +448,6 @@ TYPED_TEST(ThriftStructNodeTestSuite, ThriftStructNodeModifyTest) {
       enableHybridStorage>::modify(&node, "optionalString");
   ASSERT_TRUE(node->template isSet<k::optionalString>());
 
-  static_assert(!std::is_same_v<
-                typename folly::remove_cvref_t<
-                    decltype(node->template get<k::optionalString>()
-                                 .value())>::CowType,
-                HybridNodeType>);
-
   node = this->initNode(data);
   node->publish();
   auto& inLineBool = ThriftStructNode<
@@ -439,12 +464,6 @@ TYPED_TEST(ThriftStructNodeTestSuite, ThriftStructNodeModifyTest) {
   EXPECT_FALSE(optionalStruct1->isPublished());
   optionalStruct1->fromThrift(buildPortRange(1, 10));
 
-  static_assert(!std::is_same_v<
-                typename folly::remove_cvref_t<
-                    decltype(node->template get<k::optionalStruct>())>::
-                    element_type::CowType,
-                HybridNodeType>);
-
   node->publish();
   EXPECT_TRUE(node->isPublished());