Skip to content

Commit

Permalink
Fix unmatched Register/UnregisterReadHandlerAppCallback calls (projec…
Browse files Browse the repository at this point in the history 
…t-chip#36119)

* Fix unmatched Register/UnregisterReadHandlerAppCallback calls

By moving register/unregister calls to SetUp/TearDown we will ensure
that tests will not reuse stale global state from previous test case.

* Cleanup

* Restyled by clang-format

---------

Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
@arkq @restyled-commits
arkq and restyled-commits authored Oct 17, 2024
1 parent 7916808 commit 162b599
Showing 1 changed file with 4 additions and 129 deletions.
133 changes: 4 additions & 129 deletions src/controller/tests/data_model/TestRead.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ class TestRead : public chip::Test::AppContext, public app::ReadHandler::Applica
void SetUp() override
{
chip::Test::AppContext::SetUp();
// Register app callback, so we can test it as well to ensure we get the right
// number of SubscriptionEstablishment/Termination callbacks.
InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);
mOldProvider = InteractionModelEngine::GetInstance()->SetDataModelProvider(&CustomDataModel::Instance());
chip::Test::SetMockNodeConfig(TestMockNodeConfig());
}
Expand All @@ -127,6 +130,7 @@ class TestRead : public chip::Test::AppContext, public app::ReadHandler::Applica
{
chip::Test::ResetMockNodeConfig();
InteractionModelEngine::GetInstance()->SetDataModelProvider(mOldProvider);
InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback();
chip::Test::AppContext::TearDown();
}

Expand Down Expand Up @@ -270,11 +274,6 @@ TEST_F(TestRead, TestReadSubscribeAttributeResponseWithVersionOnlyCache)
chip::app::ClusterStateCache cache(delegate, Optional<EventNumber>::Missing(), false /*cachedData*/);

chip::app::ReadPrepareParams readPrepareParams(GetSessionBobToAlice());
//
// Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination
// callbacks.
//
app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

// read of E2C2A* and E3C2A2. Expect cache E2C2 version
{
Expand Down Expand Up @@ -352,11 +351,6 @@ TEST_F(TestRead, TestReadSubscribeAttributeResponseWithCache)
chip::app::ReadPrepareParams readPrepareParams(GetSessionBobToAlice());
readPrepareParams.mMinIntervalFloorSeconds = 0;
readPrepareParams.mMaxIntervalCeilingSeconds = 4;
//
// Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination
// callbacks.
//
app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

[[maybe_unused]] int testId = 0;

Expand Down Expand Up @@ -1691,12 +1685,6 @@ TEST_F(TestRead, TestReadHandler_MultipleSubscriptions)
numSubscriptionEstablishedCalls++;
};

//
// Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination
// callbacks.
//
app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

//
// Try to issue parallel subscriptions that will exceed the value for app::InteractionModelEngine::kReadHandlerPoolSize.
// If heap allocation is correctly setup, this should result in it successfully servicing more than the number
Expand Down Expand Up @@ -1727,7 +1715,6 @@ TEST_F(TestRead, TestReadHandler_MultipleSubscriptions)
EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u);

SetMRPMode(chip::Test::MessagingContext::MRPMode::kDefault);
app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback();
}

TEST_F(TestRead, TestReadHandler_SubscriptionAppRejection)
Expand Down Expand Up @@ -1756,12 +1743,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionAppRejection)
numSubscriptionEstablishedCalls++;
};

//
// Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination
// callbacks.
//
app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

//
// Test the application rejecting subscriptions.
//
Expand All @@ -1788,7 +1769,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionAppRejection)

EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u);

app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback();
mEmitSubscriptionError = false;
}

Expand Down Expand Up @@ -1831,17 +1811,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest1)
numSubscriptionEstablishedCalls++;
};

//
// Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination
// callbacks.
//
app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

//
// Test the server-side application altering the subscription intervals.
//
mAlterSubscriptionIntervals = false;

EXPECT_EQ(Controller::SubscribeAttribute<Clusters::UnitTesting::Attributes::ListStructOctetString::TypeInfo>(
&GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 5, 5,
onSubscriptionEstablishedCb, nullptr, true),
Expand All @@ -1863,9 +1832,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest1)
EXPECT_EQ(mNumActiveSubscriptions, 0);

EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u);

app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback();
mAlterSubscriptionIntervals = false;
}

// Subscriber sends the request with particular max-interval value:
Expand Down Expand Up @@ -1906,17 +1872,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest2)
numSubscriptionEstablishedCalls++;
};

//
// Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination
// callbacks.
//
app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

//
// Test the server-side application altering the subscription intervals.
//
mAlterSubscriptionIntervals = false;

EXPECT_EQ(Controller::SubscribeAttribute<Clusters::UnitTesting::Attributes::ListStructOctetString::TypeInfo>(
&GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 0, 10,
onSubscriptionEstablishedCb, nullptr, true),
Expand All @@ -1938,9 +1893,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest2)
EXPECT_EQ(mNumActiveSubscriptions, 0);

EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u);

app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback();
mAlterSubscriptionIntervals = false;
}

// Subscriber sends the request with particular max-interval value:
Expand Down Expand Up @@ -1981,12 +1933,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest3)
numSubscriptionEstablishedCalls++;
};

//
// Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination
// callbacks.
//
app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

//
// Test the server-side application altering the subscription intervals.
//
Expand All @@ -2013,9 +1959,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest3)
EXPECT_EQ(mNumActiveSubscriptions, 0);

EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u);

app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback();
mAlterSubscriptionIntervals = false;
}

#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
Expand Down Expand Up @@ -2049,12 +1992,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest4)
numSubscriptionEstablishedCalls++;
};

//
// Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination
// callbacks.
//
app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

//
// Test the server-side application altering the subscription intervals.
//
Expand All @@ -2080,9 +2017,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest4)
EXPECT_EQ(mNumActiveSubscriptions, 0);

EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u);

app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback();
mAlterSubscriptionIntervals = false;
}

#if CHIP_CONFIG_ENABLE_ICD_SERVER != 1
Expand Down Expand Up @@ -2125,17 +2059,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest5)
numSubscriptionEstablishedCalls++;
};

//
// Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination
// callbacks.
//
app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

//
// Test the server-side application altering the subscription intervals.
//
mAlterSubscriptionIntervals = false;

EXPECT_EQ(Controller::SubscribeAttribute<Clusters::UnitTesting::Attributes::ListStructOctetString::TypeInfo>(
&GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 0, 4000,
onSubscriptionEstablishedCb, nullptr, true),
Expand All @@ -2157,9 +2080,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest5)
EXPECT_EQ(mNumActiveSubscriptions, 0);

EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u);

app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback();
mAlterSubscriptionIntervals = false;
}

// Subscriber sends the request with particular max-interval value:
Expand Down Expand Up @@ -2200,12 +2120,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest6)
numSubscriptionEstablishedCalls++;
};

//
// Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination
// callbacks.
//
app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

//
// Test the server-side application altering the subscription intervals.
//
Expand All @@ -2232,9 +2146,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest6)
EXPECT_EQ(mNumActiveSubscriptions, 0);

EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u);

app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback();
mAlterSubscriptionIntervals = false;
}

// Subscriber sends the request with particular max-interval value:
Expand Down Expand Up @@ -2274,11 +2185,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest7)

numSubscriptionEstablishedCalls++;
};
//
// Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination
// callbacks.
//
app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

//
// Test the server-side application altering the subscription intervals.
Expand Down Expand Up @@ -2306,9 +2212,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest7)
EXPECT_EQ(mNumActiveSubscriptions, 0);

EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u);

app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback();
mAlterSubscriptionIntervals = false;
}

#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
Expand Down Expand Up @@ -2341,11 +2244,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest8)
chip::SubscriptionId aSubscriptionId) {
numSubscriptionEstablishedCalls++;
};
//
// Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination
// callbacks.
//
app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

//
// Test the server-side application altering the subscription intervals.
Expand All @@ -2372,9 +2270,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest8)
EXPECT_EQ(mNumActiveSubscriptions, 0);

EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u);

app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback();
mAlterSubscriptionIntervals = false;
}

// Subscriber sends the request with particular max-interval value:
Expand Down Expand Up @@ -2405,17 +2300,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest9)
numSubscriptionEstablishedCalls++;
};

//
// Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination
// callbacks.
//
app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

//
// Test the server-side application altering the subscription intervals.
//
mAlterSubscriptionIntervals = false;

EXPECT_EQ(Controller::SubscribeAttribute<Clusters::UnitTesting::Attributes::ListStructOctetString::TypeInfo>(
&GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 5, 4,
onSubscriptionEstablishedCb, nullptr, true),
Expand All @@ -2434,9 +2318,6 @@ TEST_F(TestRead, TestReadHandler_SubscriptionReportingIntervalsTest9)
EXPECT_EQ(mNumActiveSubscriptions, 0);

EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u);

app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback();
mAlterSubscriptionIntervals = false;
}

/**
Expand Down Expand Up @@ -3291,8 +3172,6 @@ TEST_F(TestRead, TestReadHandler_KillOverQuotaSubscriptions)
app::InteractionModelEngine::kMinSupportedSubscriptionsPerFabric * GetFabricTable().FabricCount();
const auto kExpectedParallelPaths = kExpectedParallelSubs * app::InteractionModelEngine::kMinSupportedPathsPerSubscription;

app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

// Here, we set up two background perpetual read requests to simulate parallel Read + Subscriptions.
// We don't care about the data read, we only care about the existence of such read transactions.
TestReadCallback readCallback;
Expand Down Expand Up @@ -3502,8 +3381,6 @@ TEST_F(TestRead, TestReadHandler_KillOldestSubscriptions)
app::InteractionModelEngine::kMinSupportedSubscriptionsPerFabric * GetFabricTable().FabricCount();
const auto kExpectedParallelPaths = kExpectedParallelSubs * app::InteractionModelEngine::kMinSupportedPathsPerSubscription;

app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

TestReadCallback readCallback;
std::vector<std::unique_ptr<app::ReadClient>> readClients;

Expand Down Expand Up @@ -3621,8 +3498,6 @@ TEST_F(TestRead, TestReadHandler_ParallelReads)

auto sessionHandle = GetSessionBobToAlice();

app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(this);

auto TestCase = [&](const TestReadHandler_ParallelReads_TestCase_Parameters & params, std::function<void()> body) {
TestReadHandler_ParallelReads_TestCase(this, params, body);
};
Expand Down

0 comments on commit 162b599

Please sign in to comment.