[Thinkit] Add ControlDevice method to filter out interfaces that cause a collateral link down on admin down.Add SCOPED_TRACE for interface names. Move helper functions in factory_reset_test from header into anonymous namespace in source.Add ConfigureSwitchPair function that returns status instead of P4RT sessions.Update to clear all entities instead of just table entries. Generalize APIs to dual-switch testbeds with different configs for each switch.

Author: kishanps

tests/gnoi/BUILD.bazel

@@ -67,15 +67,17 @@ cc_library(
     ],
     deps = [
         "//gutil:status_matchers",
-        "//lib/gnmi:gnmi_helper",
         "//lib/validator:validator_lib",
-        "//thinkit:mirror_testbed_fixture",
         "//thinkit:ssh_client",
+        "//thinkit:switch",
         "@com_github_gnoi//factory_reset:factory_reset_cc_proto",
         "@com_github_google_glog//:glog",
-        "@com_google_absl//absl/container:flat_hash_set",
+        "@com_github_grpc_grpc//:grpc++",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/time",
+        "@com_google_absl//absl/types:span",
         "@com_google_googletest//:gtest",
     ],
     alwayslink = True,

tests/gnoi/bert_tests.cc

@@ -541,6 +541,7 @@ void GetAndVerifyBertResultsWithAdminDownInterfaces(
         GetInterfaceNameFromOcInterfacePath(
             result_response.per_port_responses(idx).interface()));
     LOG(INFO) << "Verifying result for interface: " << interface_name;
+    SCOPED_TRACE(absl::Substitute("Interface: $0", interface_name));
     // Check if interface is part of list where admin state was disabled.
     if (IsInterfaceInList(interface_name, admin_down_interfaces) ||
         IsInterfaceInList(interface_name, admin_down_on_peer_interfaces)) {
@@ -1192,16 +1193,27 @@ TEST_P(BertTest, RunBertOnMaximumAllowedPorts) {
   // Select SUT interfaces whose peer interfaces on control switch will be admin
   // disabled in the range
   // [sut_test_interfaces_/2..sut_test_interfaces_.size()).
-  std::vector<std::string> sut_interfaces_peer_admin_down;
-  std::sample(sut_test_interfaces_.begin() + sut_test_interfaces_.size() / 2,
-              sut_test_interfaces_.end(),
-              std::back_inserter(sut_interfaces_peer_admin_down),
-              num_interfaces_to_disable,
-              std::mt19937(absl::GetFlag(FLAGS_idx_seed)));
-  // Get control switch interfaces for admin disable.
+  std::vector<std::string> sut_interfaces_peer_admin_down_candidates(
+      sut_test_interfaces_.begin() + sut_test_interfaces_.size() / 2,
+      sut_test_interfaces_.end());
+  ASSERT_OK_AND_ASSIGN(std::vector<std::string>
+                           control_switch_interfaces_for_admin_down_candidates,
+                       GetPeerInterfacesForSutInterfaces(
+                           sut_interfaces_peer_admin_down_candidates));
   ASSERT_OK_AND_ASSIGN(
-      std::vector<std::string> control_switch_interfaces_for_admin_down,
-      GetPeerInterfacesForSutInterfaces(sut_interfaces_peer_admin_down));
+      std::vector<std::string>
+          control_switch_interfaces_for_admin_down_filtered_candidates,
+      control_device.FilterCollateralDownOnAdminDownInterfaces(
+          control_switch_interfaces_for_admin_down_candidates));
+  std::vector<std::string> control_switch_interfaces_for_admin_down;
+  std::sample(
+      control_switch_interfaces_for_admin_down_filtered_candidates.begin(),
+      control_switch_interfaces_for_admin_down_filtered_candidates.end(),
+      std::back_inserter(control_switch_interfaces_for_admin_down),
+      num_interfaces_to_disable, std::mt19937(absl::GetFlag(FLAGS_idx_seed)));
+  ASSERT_OK_AND_ASSIGN(std::vector<std::string> sut_interfaces_peer_admin_down,
+                       GetSutInterfacesForControlInterfaces(
+                           control_switch_interfaces_for_admin_down));
 
   LOG(INFO) << "Starting BERT on " << sut_test_interfaces_.size()
             << " {SUT, control_device} links: ";

tests/gnoi/bert_tests.h

@@ -129,6 +129,20 @@ class BertTest : public thinkit::GenericTestbedFixture<> {
     return peer_interfaces;
   }
 
+  absl::StatusOr<std::vector<std::string>> GetSutInterfacesForControlInterfaces(
+      const std::vector<std::string> &control_interfaces) {
+    std::vector<std::string> sut_interfaces;
+    sut_interfaces.reserve(control_interfaces.size());
+    for (const std::string &control_interface : control_interfaces) {
+      if (control_to_peer_interface_mapping_.count(control_interface) == 0) {
+        return absl::NotFoundError("Failed to find peer.");
+      }
+      sut_interfaces.push_back(
+          control_to_peer_interface_mapping_[control_interface]);
+    }
+    return sut_interfaces;
+  }
+
  protected:
   std::unique_ptr<thinkit::GenericTestbed> generic_testbed_;
   std::unique_ptr<gnmi::gNMI::StubInterface> sut_gnmi_stub_;

tests/gnoi/factory_reset_test.cc

@@ -1,4 +1,4 @@
-// Copyright 2024 Google LLC
+// Copyright 2025 Google LLC
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -14,28 +14,29 @@
 
 #include "tests/gnoi/factory_reset_test.h"
 
-#include <cstdint>
-#include <cstdlib>
-#include <map>
 #include <memory>
 #include <string>
-#include <utility>
-#include <valarray>
 
-#include "absl/container/flat_hash_set.h"
+#include "absl/status/status.h"
+#include "absl/status/statusor.h"
 #include "absl/strings/numbers.h"
-#include "absl/strings/str_cat.h"
-#include "absl/strings/string_view.h"
 #include "absl/time/clock.h"
 #include "absl/time/time.h"
+#include "absl/types/span.h"
+#include "factory_reset/factory_reset.pb.h"
 #include "glog/logging.h"
 #include "gmock/gmock.h"
+#include "grpcpp/client_context.h"
+#include "grpcpp/support/status.h"
 #include "gtest/gtest.h"
+#include "gutil/status.h"
 #include "gutil/status_matchers.h"
-#include "lib/gnmi/gnmi_helper.h"
 #include "lib/validator/validator_lib.h"
+#include "thinkit/ssh_client.h"
+#include "thinkit/switch.h"
 
 namespace factory_reset {
+namespace {
 
 // TODO: investigate why shutdown time is increasing and revert
 // to 60s if possible.
@@ -49,7 +50,7 @@ constexpr absl::Duration kSshSessionTimeout = absl::Seconds(5);
 void IssueGnoiFactoryResetAndValidateStatus(
     thinkit::Switch& sut, const gnoi::factory_reset::StartRequest& request,
     gnoi::factory_reset::StartResponse* response,
-    grpc::Status expected_status) {
+    grpc::Status expected_status = {}) {
   LOG(INFO) << "Issuing factory reset with parameters: "
             << request.DebugString();
   ASSERT_OK_AND_ASSIGN(auto sut_gnoi_factory_reset_stub,
@@ -104,6 +105,8 @@ void ValidateStackState(thinkit::Switch& sut,
       << "System did not come up in " << kFactoryResetWaitForUpTime;
 }
 
+}  // namespace
+
 void TestFactoryResetSuccess(thinkit::Switch& sut,
                              thinkit::SSHClient& ssh_client,
                              absl::Span<const std::string> interfaces) {
@@ -142,14 +145,15 @@ void TestGnoiFactoryResetGnoiServerUnreachableFail(
   while (consecutive_unreachable_count < kConsecutivePingsRequired) {
     if (pins_test::Pingable(sut, kPingTimeout).ok()) {
       consecutive_unreachable_count = 0;
+      LOG(INFO) << "System still reachable";
+      if (absl::Now() - start > kFactoryResetWaitForDownTime) {
+        FAIL() << "System did not go down in " << kFactoryResetWaitForDownTime;
+      }
     } else {
       consecutive_unreachable_count++;
+      LOG(INFO) << "System unreachable for " << consecutive_unreachable_count
+                << " consecutive pings";
     }
-    if (absl::Now() - start > kFactoryResetWaitForDownTime) {
-      FAIL() << "System did not go down in " << kFactoryResetWaitForDownTime;
-    }
-    LOG(INFO) << "System unreachable for " << consecutive_unreachable_count
-              << " consecutive pings";
     absl::SleepFor(kPingReachabilityInterval);
   }
   LOG(INFO) << "Device became unreachable after: " << absl::Now() - start;

tests/gnoi/factory_reset_test.h

@@ -1,4 +1,4 @@
-// Copyright 2024 Google LLC
+// Copyright 2025 Google LLC
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -15,32 +15,26 @@
 #ifndef PINS_TESTS_GNOI_FACTORY_RESET_TEST_H_
 #define PINS_TESTS_GNOI_FACTORY_RESET_TEST_H_
 
-#include "factory_reset/factory_reset.pb.h"
-#include "thinkit/mirror_testbed_fixture.h"
+#include <string>
+
+#include "absl/types/span.h"
 #include "thinkit/ssh_client.h"
+#include "thinkit/switch.h"
 
 namespace factory_reset {
 
-void IssueGnoiFactoryResetAndValidateStatus(
-    thinkit::Switch &sut, const gnoi::factory_reset::StartRequest &request,
-    gnoi::factory_reset::StartResponse *response,
-    grpc::Status expected_status = grpc::Status());
-
-void ValidateStackState(thinkit::Switch &sut,
-                        absl::Span<const std::string> interfaces);
-
-void TestFactoryResetSuccess(thinkit::Switch &sut,
-                             thinkit::SSHClient &ssh_client,
+void TestFactoryResetSuccess(thinkit::Switch& sut,
+                             thinkit::SSHClient& ssh_client,
                              absl::Span<const std::string> interfaces);
 
-void TestDuplicateFactoryResetFailure(thinkit::Switch &sut,
-                                      thinkit::SSHClient &ssh_client,
+void TestDuplicateFactoryResetFailure(thinkit::Switch& sut,
+                                      thinkit::SSHClient& ssh_client,
                                       absl::Span<const std::string> interfaces);
 
 void TestGnoiFactoryResetGnoiServerUnreachableFail(
-    thinkit::Switch &sut, thinkit::SSHClient &ssh_client,
+    thinkit::Switch& sut, thinkit::SSHClient& ssh_client,
     absl::Span<const std::string> interfaces);
 
-} // namespace factory_reset
+}  // namespace factory_reset
 
-#endif // PINS_TESTS_GNOI_FACTORY_RESET_TEST_H_
+#endif  // PINS_TESTS_GNOI_FACTORY_RESET_TEST_H_

tests/lib/BUILD.bazel

@@ -113,6 +113,7 @@ cc_library(
         "@com_github_p4lang_p4runtime//:p4info_cc_proto",
         "@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
         "@com_github_p4lang_p4runtime//:p4types_cc_proto",
+        "@com_google_absl//absl/base:core_headers",
         "@com_google_absl//absl/container:btree",
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/status",

tests/lib/p4info_helper_test.cc

@@ -17,7 +17,7 @@
 #include "gtest/gtest.h"
 #include "p4_pdpi/ir.pb.h"
 
-namespace gpins {
+namespace pins {
 namespace {
 
 TEST(P4InfoHelperTest, TableHasMatchField) {
@@ -36,4 +36,4 @@ TEST(P4InfoHelperTest, TableHasMatchField) {
 }
 
 }  // namespace
-}  // namespace gpins
+}  // namespace pins