Revert D90290258

Summary:
Reverts D90290258 which added a buffer constructor to FbossEepromInterface.

The file-based testing approach using folly::test::TemporaryDirectory is more valuable for catching issues at the unit test level rather than relying on hardware tests.

This also removes the boost dependency in favor of std::transform for uppercase string conversion.

Reviewed By: somasun

Differential Revision: D90834969

fbshipit-source-id: 0539c4dc2f336a15721d3ce9f6991686440ce1d4

Author: binhuang00

cmake/PlatformWeutil.cmake

@@ -29,10 +29,9 @@ add_library(weutil_fboss_eeprom_interface
 )
 
 target_link_libraries(weutil_fboss_eeprom_interface
-    fmt::fmt
-    Folly::folly
-    weutil_crc16_ccitt_aug
-    weutil_eeprom_contents_cpp2
+  Folly::folly
+  weutil_crc16_ccitt_aug
+  weutil_eeprom_contents_cpp2
 )
 
 add_library(weutil_lib

fboss/platform/weutil/BUCK

@@ -15,7 +15,6 @@ cpp_library(
     headers = [
         "WeutilInterface.h",
     ],
-    preferred_linkage = "static",
     exported_deps = [
         ":config_utils_lib",
         ":eeprom_content_validator",
@@ -31,7 +30,6 @@ cpp_library(
     ],
     exported_external_deps = [
         "gflags",
-        ("boost", None, "boost_algorithm"),
     ],
 )
 

fboss/platform/weutil/FbossEepromInterface.cpp

@@ -83,15 +83,6 @@ FbossEepromInterface::FbossEepromInterface(
     const std::string& eepromPath,
     const uint16_t offset) {
   auto buffer = ParserUtils::loadEeprom(eepromPath, offset);
-  initFromBuffer(buffer);
-}
-
-FbossEepromInterface::FbossEepromInterface(
-    const std::vector<uint8_t>& eepromBuffer) {
-  initFromBuffer(eepromBuffer);
-}
-
-void FbossEepromInterface::initFromBuffer(const std::vector<uint8_t>& buffer) {
   if (buffer.size() < kHeaderSize) {
     throw std::runtime_error("Invalid EEPROM size");
   }

fboss/platform/weutil/FbossEepromInterface.h

@@ -31,8 +31,6 @@ class FbossEepromInterface {
 
   FbossEepromInterface(const std::string& eepromPath, const uint16_t offset);
 
-  explicit FbossEepromInterface(const std::vector<uint8_t>& eepromBuffer);
-
   // TODO: Get rid of getContents() in the future.
   std::vector<std::pair<std::string, std::string>> getContents() const;
 
@@ -48,7 +46,6 @@ class FbossEepromInterface {
 
  private:
   FbossEepromInterface() = default;
-  void initFromBuffer(const std::vector<uint8_t>& buffer);
   void parseEepromBlobTLV(const std::vector<uint8_t>& buffer);
 
   std::map<int, EepromFieldEntry> fieldMap_{};

fboss/platform/weutil/Weutil.cpp

@@ -1,7 +1,6 @@
 // (c) Facebook, Inc. and its affiliates. Confidential and proprietary.
 #include "fboss/platform/weutil/Weutil.h"
 
-#include <boost/algorithm/string.hpp>
 #include <folly/logging/xlog.h>
 #include <thrift/lib/cpp2/protocol/Serializer.h>
 
@@ -33,7 +32,12 @@ std::unique_ptr<WeutilInterface> createWeUtilIntf(
   }
   weutil::ConfigUtils configUtils(platformName);
   weutil::FruEeprom fruEeprom;
-  auto upperEepromName = boost::to_upper_copy(eepromName);
+  std::string upperEepromName = eepromName;
+  std::transform(
+      upperEepromName.begin(),
+      upperEepromName.end(),
+      upperEepromName.begin(),
+      [](unsigned char c) { return std::toupper(c); });
   if (upperEepromName == "CHASSIS" || upperEepromName.empty()) {
     fruEeprom = configUtils.getFruEeprom(configUtils.getChassisEepromName());
   } else {

fboss/platform/weutil/test/BUCK

@@ -38,9 +38,10 @@ cpp_unittest(
         "FbossEepromInterfaceTest.cpp",
     ],
     deps = [
-        "fbsource//third-party/fmt:fmt",
         "//fboss/platform/weutil:fboss_eeprom_lib",
         "//fboss/platform/weutil/if:eeprom_contents-cpp2-types",
+        "//folly:file_util",
+        "//folly/testing:test_util",
     ],
 )
 

fboss/platform/weutil/test/FbossEepromInterfaceTest.cpp

@@ -1,8 +1,10 @@
 // (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary.
 
-#include <fmt/format.h>
 #include <gtest/gtest.h>
 
+#include <folly/FileUtil.h>
+#include <folly/testing/TestUtil.h>
+
 #include "fboss/platform/weutil/FbossEepromInterface.h"
 #include "fboss/platform/weutil/if/gen-cpp2/eeprom_contents_types.h"
 
@@ -13,7 +15,7 @@ using EepromData = std::vector<uint8_t>;
 
 // Based on the Spec for V5 EEPROM:
 // https://github.com/facebook/fboss/blob/main/fboss/docs/meta_eeprom_format_v5.md
-EepromData kEepromV5 = {
+const EepromData kEepromV5 = {
     0xfb, 0xfb, 0x05, 0xff, 0x01, 0x0d, 0x46, 0x49, 0x52, 0x53, 0x54, 0x5f,
     0x53, 0x51, 0x55, 0x45, 0x45, 0x5a, 0x45, 0x02, 0x08, 0x32, 0x30, 0x31,
     0x32, 0x33, 0x34, 0x35, 0x36, 0x03, 0x08, 0x53, 0x59, 0x53, 0x41, 0x31,
@@ -36,7 +38,7 @@ EepromData kEepromV5 = {
 
 // EEPROM V5 with wrong CRC Programmed (same as the one above, but last 2
 // bytes have wrong CRC value programmed.)
-EepromData kEepromV5WrongCrc = {
+const EepromData kEepromV5WrongCrc = {
     0xfb, 0xfb, 0x05, 0xff, 0x01, 0x0d, 0x46, 0x49, 0x52, 0x53, 0x54, 0x5f,
     0x53, 0x51, 0x55, 0x45, 0x45, 0x5a, 0x45, 0x02, 0x08, 0x32, 0x30, 0x31,
     0x32, 0x33, 0x34, 0x35, 0x36, 0x03, 0x08, 0x53, 0x59, 0x53, 0x41, 0x31,
@@ -59,7 +61,7 @@ EepromData kEepromV5WrongCrc = {
 
 // Based on the Spec for V6 EEPROM:
 // https://github.com/facebook/fboss/blob/main/fboss/docs/meta_eeprom_format_v6.md
-EepromData kEepromV6 = {
+const EepromData kEepromV6 = {
     0xfb, 0xfb, 0x06, 0xff, 0x01, 0x0d, 0x46, 0x49, 0x52, 0x53, 0x54, 0x5f,
     0x53, 0x51, 0x55, 0x45, 0x45, 0x5a, 0x45, 0x02, 0x08, 0x32, 0x30, 0x31,
     0x32, 0x33, 0x34, 0x35, 0x36, 0x03, 0x08, 0x53, 0x59, 0x53, 0x41, 0x31,
@@ -82,6 +84,14 @@ EepromData kEepromV6 = {
     0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
     0xff};
 
+FbossEepromInterface createFbossEepromInterface(const EepromData& data) {
+  folly::test::TemporaryDirectory tmpDir = folly::test::TemporaryDirectory();
+  std::string fileName = tmpDir.path().string() + "/eepromContent";
+  folly::writeFile(data, fileName.c_str());
+
+  return FbossEepromInterface(fileName, 0);
+}
+
 constexpr auto kProductName = "FIRST_SQUEEZE";
 constexpr auto kProductPartNumber = "20123456";
 constexpr auto kSystemAssemblyPartNumber = "SYSA1234";
@@ -106,10 +116,9 @@ constexpr auto kRma = "1";
 constexpr auto kVendorDefinedField1 = "0x0101010101";
 constexpr auto kVendorDefinedField2 = "0x48656c6c6f";
 constexpr auto kVendorDefinedField3 = "";
-constexpr auto kCrc16V5 = "0xd5c6";
-constexpr auto kCrc16V6 = "0x4a05";
-constexpr auto kCrcCorrectTemplate = "{} (CRC Matched)";
-constexpr auto kCrc16WrongTemplate = "0xa6b7 (CRC Mismatch. Expected {})";
+constexpr auto kCrc16V5 = "0xd5c6 (CRC Matched)";
+constexpr auto kCrc16V6 = "0x4a05 (CRC Matched)";
+constexpr auto kCrc16V5Wrong = "0xa6b7 (CRC Mismatch. Expected 0xd5c6)";
 
 EepromContents createEepromContents(int version, bool crcMatched = true) {
   EepromContents result;
@@ -134,15 +143,13 @@ EepromContents createEepromContents(int version, bool crcMatched = true) {
   result.bmcMac() = kBmcMac;
   result.switchAsicMac() = kSwitchAsicMac;
   result.metaReservedMac() = kMetaReservedMac;
-  const std::string crc16 = version == 5 ? kCrc16V5 : kCrc16V6;
 
-  if (crcMatched) {
-    result.crc16() = fmt::format(kCrcCorrectTemplate, crc16);
-  } else {
-    result.crc16() = fmt::format(kCrc16WrongTemplate, crc16);
+  if (version == 5) {
+    result.crc16() = crcMatched ? kCrc16V5 : kCrc16V5Wrong;
+  } else if (version == 6) {
+    result.crc16() = kCrc16V6;
   }
 
-  // V6 unique fields
   if (version == 6) {
     result.rma() = kRma;
     result.vendorDefinedField1() = kVendorDefinedField1;
@@ -153,94 +160,62 @@ EepromContents createEepromContents(int version, bool crcMatched = true) {
   return result;
 };
 
-struct CommonEepromFields {
-  std::string productName;
-  std::string productPartNumber;
-  std::string productionState;
-  std::string productionSubState;
-  std::string variantIndicator;
-  std::string productSerialNumber;
-
-  bool operator==(const CommonEepromFields& other) const = default;
-};
-
-CommonEepromFields getCommonFields(const FbossEepromInterface& eeprom) {
-  return CommonEepromFields{
-      .productName = eeprom.getProductName(),
-      .productPartNumber = eeprom.getProductPartNumber(),
-      .productionState = eeprom.getProductionState(),
-      .productionSubState = eeprom.getProductionSubState(),
-      .variantIndicator = eeprom.getVariantVersion(),
-      .productSerialNumber = eeprom.getProductSerialNumber(),
-  };
-}
-
-const CommonEepromFields kExpectedCommonFields{
-    .productName = kProductName,
-    .productPartNumber = kProductPartNumber,
-    .productionState = kProductionState,
-    .productionSubState = kProductionSubState,
-    .variantIndicator = kVariantIndicator,
-    .productSerialNumber = kProductSerialNumber,
-};
-
-// Helper to verify common EEPROM fields parsed correctly
-void verifyCommonEepromFields(const FbossEepromInterface& eeprom) {
-  EXPECT_EQ(getCommonFields(eeprom), kExpectedCommonFields);
-}
-
 } // namespace
 
-TEST(FbossEepromInterfaceTest, V5WithBufferConstructor) {
-  FbossEepromInterface eeprom(kEepromV5);
-  EXPECT_EQ(eeprom.getVersion(), 5);
-  verifyCommonEepromFields(eeprom);
+TEST(FbossEepromInterfaceTest, V5) {
+  auto eeprom = createFbossEepromInterface(kEepromV5);
+
+  EXPECT_EQ(eeprom.getProductName(), kProductName);
+  EXPECT_EQ(eeprom.getProductPartNumber(), kProductPartNumber);
+  EXPECT_EQ(eeprom.getProductionState(), kProductionState);
+  EXPECT_EQ(eeprom.getProductionSubState(), kProductionSubState);
+  EXPECT_EQ(eeprom.getVariantVersion(), kVariantIndicator);
+  EXPECT_EQ(eeprom.getProductSerialNumber(), kProductSerialNumber);
 }
 
 TEST(FbossEepromInterfaceTest, V5WrongCRC) {
-  FbossEepromInterface eeprom(kEepromV5WrongCrc);
-  EXPECT_EQ(eeprom.getVersion(), 5);
-  verifyCommonEepromFields(eeprom);
+  auto eeprom = createFbossEepromInterface(kEepromV5WrongCrc);
+  EXPECT_EQ(eeprom.getProductName(), kProductName);
+  EXPECT_EQ(eeprom.getProductPartNumber(), kProductPartNumber);
+  EXPECT_EQ(eeprom.getProductionState(), kProductionState);
+  EXPECT_EQ(eeprom.getProductionSubState(), kProductionSubState);
+  EXPECT_EQ(eeprom.getVariantVersion(), kVariantIndicator);
+  EXPECT_EQ(eeprom.getProductSerialNumber(), kProductSerialNumber);
 }
 
-TEST(FbossEepromInterfaceTest, V6WithBufferConstructor) {
-  FbossEepromInterface eeprom(kEepromV6);
-  EXPECT_EQ(eeprom.getVersion(), 6);
-  verifyCommonEepromFields(eeprom);
+TEST(FbossEepromInterfaceTest, V6) {
+  auto eeprom = createFbossEepromInterface(kEepromV6);
+  EXPECT_EQ(eeprom.getProductName(), kProductName);
+  EXPECT_EQ(eeprom.getProductPartNumber(), kProductPartNumber);
+  EXPECT_EQ(eeprom.getProductionState(), kProductionState);
+  EXPECT_EQ(eeprom.getProductionSubState(), kProductionSubState);
+  EXPECT_EQ(eeprom.getVariantVersion(), kVariantIndicator);
+  EXPECT_EQ(eeprom.getProductSerialNumber(), kProductSerialNumber);
 }
 
 TEST(FbossEepromInterfaceTest, V5Object) {
-  FbossEepromInterface eepromInterface(kEepromV5);
-  auto actualObj = eepromInterface.getEepromContents();
+  auto eepromInterace = createFbossEepromInterface(kEepromV5);
+  auto actualObj = eepromInterace.getEepromContents();
+
   EepromContents expectedObj = createEepromContents(5);
 
   EXPECT_EQ(actualObj, expectedObj);
 }
 
 TEST(FbossEepromInterfaceTest, V6Object) {
-  FbossEepromInterface eepromInterface(kEepromV6);
-  auto actualObj = eepromInterface.getEepromContents();
+  auto eepromInterace = createFbossEepromInterface(kEepromV6);
+  auto actualObj = eepromInterace.getEepromContents();
   EepromContents expectedObj = createEepromContents(6);
 
   EXPECT_EQ(actualObj, expectedObj);
 }
 
 TEST(FbossEepromInterfaceTest, V5ObjWrongCrc) {
-  FbossEepromInterface eeprom(kEepromV5WrongCrc);
+  auto eeprom = createFbossEepromInterface(kEepromV5WrongCrc);
   auto actualObj = eeprom.getEepromContents();
   EepromContents expectedObj = createEepromContents(5, false);
 
   EXPECT_EQ(actualObj, expectedObj);
 }
 
-TEST(FbossEepromInterfaceTest, InvalidEepromSize) {
-  std::vector<uint8_t> tooSmall = {0xfb, 0xfb};
-  EXPECT_THROW((FbossEepromInterface{tooSmall}), std::runtime_error);
-}
-
-TEST(FbossEepromInterfaceTest, InvalidVersion) {
-  std::vector<uint8_t> badVersion = {0xfb, 0xfb, 0x99, 0xff};
-  EXPECT_THROW((FbossEepromInterface{badVersion}), std::runtime_error);
-}
-
 } // namespace facebook::fboss::platform