Skip to content

Commit

Permalink
Fix minor flakiness in dsf subscription tests
Browse files Browse the repository at this point in the history
Summary:
- Connection state is updated in separate context from
switch state so check that with ASSERT_EVENTUALLY_EQ rather
than EXPECT_EQ
- Static rifs, sys ports are not marked stale, pruned. While
we don't create static sys ports, rifs here, update test
logic to allow for it.

Reviewed By: nivinl

Differential Revision: D62916407

fbshipit-source-id: 537fc495adc45e0aef10e20f92c9be3b4b0263aa
  • Loading branch information
Jasmeet Bagga authored and facebook-github-bot committed Sep 18, 2024
1 parent 761e120 commit eb2f7b8
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 10 deletions.
1 change: 1 addition & 0 deletions fboss/agent/test/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@ cpp_library(
"//fboss/agent/hw/switch_asics:switch_asics",
"//fboss/agent/hw/test:config_factory",
"//fboss/agent/hw/test:hw_copp_utils",
"//fboss/agent/test/utils:acl_test_utils",
"//fboss/agent/test/utils:agent_hw_test_constants",
"//fboss/agent/test/utils:stats_test_utils",
"//fboss/lib:common_utils",
Expand Down
30 changes: 20 additions & 10 deletions fboss/agent/test/DsfSubscriptionTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,8 @@ TYPED_TEST(DsfSubscriptionTest, Connect) {
this->getRemoteSystemPorts()->size(), this->kNumRemoteSwitchAsics);
ASSERT_EVENTUALLY_EQ(
this->getRemoteInterfaces()->size(), this->kNumRemoteSwitchAsics);
EXPECT_EQ(this->dsfSessionState(), DsfSessionState::WAIT_FOR_REMOTE);
ASSERT_EVENTUALLY_EQ(
this->dsfSessionState(), DsfSessionState::WAIT_FOR_REMOTE);
});

this->updateDsfSubscriberState(
Expand All @@ -293,11 +294,13 @@ TYPED_TEST(DsfSubscriptionTest, ConnectDisconnect) {
this->getRemoteSystemPorts()->size(), this->kNumRemoteSwitchAsics);
ASSERT_EVENTUALLY_EQ(
this->getRemoteInterfaces()->size(), this->kNumRemoteSwitchAsics);
EXPECT_EQ(this->dsfSessionState(), DsfSessionState::WAIT_FOR_REMOTE);
ASSERT_EVENTUALLY_EQ(
this->dsfSessionState(), DsfSessionState::WAIT_FOR_REMOTE);
});

this->stopPublisher();
EXPECT_EQ(this->dsfSessionState(), DsfSessionState::CONNECT);
WITH_RETRIES(
ASSERT_EVENTUALLY_EQ(this->dsfSessionState(), DsfSessionState::CONNECT));
}

TYPED_TEST(DsfSubscriptionTest, GR) {
Expand All @@ -312,7 +315,8 @@ TYPED_TEST(DsfSubscriptionTest, GR) {
this->getRemoteSystemPorts()->size(), this->kNumRemoteSwitchAsics);
ASSERT_EVENTUALLY_EQ(
this->getRemoteInterfaces()->size(), this->kNumRemoteSwitchAsics);
EXPECT_EQ(this->dsfSessionState(), DsfSessionState::WAIT_FOR_REMOTE);
ASSERT_EVENTUALLY_EQ(
this->dsfSessionState(), DsfSessionState::WAIT_FOR_REMOTE);
});

this->stopPublisher(true);
Expand All @@ -337,8 +341,10 @@ TYPED_TEST(DsfSubscriptionTest, GR) {
auto assertObjStatus = [expectedStatus](const auto& objs) {
std::for_each(
objs->begin(), objs->end(), [expectedStatus](const auto& idAndObj) {
EXPECT_EQ(
idAndObj.second->getRemoteLivenessStatus(), expectedStatus);
if (!idAndObj.second->isStatic()) {
EXPECT_EQ(
idAndObj.second->getRemoteLivenessStatus(), expectedStatus);
}
});
};
assertObjStatus(this->getRemoteSystemPorts());
Expand All @@ -358,8 +364,10 @@ TYPED_TEST(DsfSubscriptionTest, GR) {
auto remoteRifs = this->getRemoteInterfaces();
for (const auto [_, rif] : std::as_const(*remoteRifs)) {
// Neighbors should get pruned
ASSERT_EVENTUALLY_EQ(rif->getNdpTable()->size(), 0);
ASSERT_EVENTUALLY_EQ(rif->getArpTable()->size(), 0);
if (!rif->isStatic()) {
ASSERT_EVENTUALLY_EQ(rif->getNdpTable()->size(), 0);
ASSERT_EVENTUALLY_EQ(rif->getArpTable()->size(), 0);
}
}
});
// Should be STATLE after GR expire
Expand All @@ -381,7 +389,8 @@ TYPED_TEST(DsfSubscriptionTest, DataUpdate) {
this->getRemoteSystemPorts()->size(), this->kNumRemoteSwitchAsics);
ASSERT_EVENTUALLY_EQ(
this->getRemoteInterfaces()->size(), this->kNumRemoteSwitchAsics);
EXPECT_EQ(this->dsfSessionState(), DsfSessionState::WAIT_FOR_REMOTE);
ASSERT_EVENTUALLY_EQ(
this->dsfSessionState(), DsfSessionState::WAIT_FOR_REMOTE);
});

auto sysPort2 = makeSysPort(
Expand Down Expand Up @@ -410,7 +419,8 @@ TYPED_TEST(DsfSubscriptionTest, updateFailed) {
this->getRemoteSystemPorts()->size(), this->kNumRemoteSwitchAsics);
ASSERT_EVENTUALLY_EQ(
this->getRemoteInterfaces()->size(), this->kNumRemoteSwitchAsics);
EXPECT_EQ(this->dsfSessionState(), DsfSessionState::WAIT_FOR_REMOTE);
ASSERT_EVENTUALLY_EQ(
this->dsfSessionState(), DsfSessionState::WAIT_FOR_REMOTE);
});
waitForStateUpdates(this->sw_);

Expand Down

0 comments on commit eb2f7b8

Please sign in to comment.