Skip to content

Commit 656040b

Browse files
motiz88meta-codesync[bot]
authored andcommitted
Extract test fixtures to separate header (facebook#55056)
Summary: Pull Request resolved: facebook#55056 Refactors InspectorPackagerConnectionTest.cpp to extract the test fixtures into a separate header file. This allows us to split `InspectorPackagerConnectionTest.cpp` into multiple files as we add more test cases, which is particularly helpful for LLMs writing tests. Changelog: [Internal] Reviewed By: robhogan Differential Revision: D90174645
1 parent e6494eb commit 656040b

File tree

2 files changed

+141
-119
lines changed

2 files changed

+141
-119
lines changed

packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorPackagerConnectionTest.cpp

Lines changed: 1 addition & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,12 @@
66
*/
77

88
#include <fmt/format.h>
9-
#include <folly/dynamic.h>
10-
#include <folly/executors/ManualExecutor.h>
11-
#include <folly/executors/QueuedImmediateExecutor.h>
129
#include <folly/json.h>
1310
#include <gmock/gmock.h>
1411
#include <gtest/gtest.h>
1512

16-
#include <jsinspector-modern/InspectorInterfaces.h>
17-
#include <jsinspector-modern/InspectorPackagerConnection.h>
18-
19-
#include <memory>
20-
2113
#include "FollyDynamicMatchers.h"
22-
#include "InspectorMocks.h"
23-
#include "UniquePtrFactory.h"
14+
#include "InspectorPackagerConnectionTest.h"
2415

2516
using namespace ::testing;
2617
using namespace std::literals::chrono_literals;
@@ -29,115 +20,6 @@ using folly::dynamic, folly::toJson;
2920

3021
namespace facebook::react::jsinspector_modern {
3122

32-
namespace {
33-
34-
template <typename Executor>
35-
class InspectorPackagerConnectionTestBase : public testing::Test {
36-
protected:
37-
InspectorPackagerConnectionTestBase()
38-
: packagerConnection_(
39-
InspectorPackagerConnection{
40-
"ws://mock-host:12345",
41-
"my-device",
42-
"my-app",
43-
packagerConnectionDelegates_.make_unique(asyncExecutor_)}) {
44-
auto makeSocket = webSockets_.lazily_make_unique<
45-
const std::string&,
46-
std::weak_ptr<IWebSocketDelegate>>();
47-
ON_CALL(*packagerConnectionDelegate(), connectWebSocket(_, _))
48-
.WillByDefault([makeSocket](auto&&... args) {
49-
auto socket = makeSocket(std::forward<decltype(args)>(args)...);
50-
socket->getDelegate().didOpen();
51-
return std::move(socket);
52-
});
53-
EXPECT_CALL(*packagerConnectionDelegate(), connectWebSocket(_, _))
54-
.Times(AnyNumber());
55-
}
56-
57-
void TearDown() override {
58-
// Forcibly clean up all pages currently registered with the inspector in
59-
// order to isolate state between tests. NOTE: Using TearDown instead of a
60-
// destructor so that we can use FAIL() etc.
61-
std::vector<int> pagesToRemove;
62-
auto pages = getInspectorInstance().getPages();
63-
int liveConnectionCount = 0;
64-
for (size_t i = 0; i != localConnections_.objectsVended(); ++i) {
65-
if (localConnections_[i] != nullptr) {
66-
liveConnectionCount++;
67-
// localConnections_[i] is a strict mock and will complain when we
68-
// removePage if the call is unexpected.
69-
EXPECT_CALL(*localConnections_[i], disconnect());
70-
}
71-
}
72-
for (auto& page : pages) {
73-
getInspectorInstance().removePage(page.id);
74-
}
75-
if (!pages.empty() && (liveConnectionCount != 0)) {
76-
if (!::testing::Test::HasFailure()) {
77-
FAIL()
78-
<< "Test case ended with " << liveConnectionCount
79-
<< " open connection(s) and " << pages.size()
80-
<< " registered page(s). You must manually call removePage for each page.";
81-
}
82-
}
83-
::testing::Test::TearDown();
84-
}
85-
86-
MockInspectorPackagerConnectionDelegate* packagerConnectionDelegate() {
87-
// We only create one PackagerConnectionDelegate per test.
88-
EXPECT_EQ(packagerConnectionDelegates_.objectsVended(), 1);
89-
return packagerConnectionDelegates_[0];
90-
}
91-
92-
Executor asyncExecutor_;
93-
94-
UniquePtrFactory<MockInspectorPackagerConnectionDelegate>
95-
packagerConnectionDelegates_;
96-
/**
97-
* webSockets_ will hold the WebSocket instance(s) owned by
98-
* packagerConnection_ while also allowing us to access them during
99-
* the test. We can send messages *to* packagerConnection_ by
100-
* calling webSockets_[i]->getDelegate().didReceiveMessage(...). Messages
101-
* *from* packagerConnection_ will be found as calls to
102-
* webSockets_[i]->send, which is a mock method installed by gmock.
103-
* These are strict mocks, so method calls will fail if they are not
104-
* expected with a corresponding call to EXPECT_CALL(...) - for example
105-
* if unexpected WebSocket messages are sent.
106-
*/
107-
UniquePtrFactory<StrictMock<MockWebSocket>> webSockets_;
108-
/**
109-
* localConnections_ will hold the LocalConnection instances owned
110-
* by packagerConnection_ while also allowing us to access them
111-
* during the test.
112-
* These are strict mocks, so method calls will fail if they are not
113-
* expected with a corresponding call to EXPECT_CALL(...).
114-
*/
115-
UniquePtrFactory<StrictMock<MockLocalConnection>> localConnections_;
116-
std::optional<InspectorPackagerConnection> packagerConnection_;
117-
};
118-
119-
using InspectorPackagerConnectionTest =
120-
InspectorPackagerConnectionTestBase<folly::QueuedImmediateExecutor>;
121-
122-
/**
123-
* Fixture class for tests that run on a ManualExecutor. Work scheduled
124-
* on the executor is *not* run automatically; it must be manually advanced
125-
* in the body of the test.
126-
*/
127-
class InspectorPackagerConnectionTestAsync
128-
: public InspectorPackagerConnectionTestBase<folly::ManualExecutor> {
129-
public:
130-
virtual void TearDown() override {
131-
// Assert there are no pending tasks on the ManualExecutor.
132-
auto tasksCleared = asyncExecutor_.clear();
133-
EXPECT_EQ(tasksCleared, 0)
134-
<< "There were still pending tasks on asyncExecutor_ at the end of the test. Use advance() or run() as needed.";
135-
InspectorPackagerConnectionTestBase<folly::ManualExecutor>::TearDown();
136-
}
137-
};
138-
139-
} // namespace
140-
14123
TEST_F(InspectorPackagerConnectionTest, TestConnectThenDestroy) {
14224
packagerConnection_->connect();
14325

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
#pragma once
9+
10+
#include <fmt/format.h>
11+
#include <folly/dynamic.h>
12+
#include <folly/executors/ManualExecutor.h>
13+
#include <folly/executors/QueuedImmediateExecutor.h>
14+
#include <folly/json.h>
15+
#include <gmock/gmock.h>
16+
#include <gtest/gtest.h>
17+
18+
#include <jsinspector-modern/InspectorInterfaces.h>
19+
#include <jsinspector-modern/InspectorPackagerConnection.h>
20+
21+
#include <memory>
22+
23+
#include "InspectorMocks.h"
24+
#include "UniquePtrFactory.h"
25+
26+
namespace facebook::react::jsinspector_modern {
27+
28+
/**
29+
* Base test fixture for InspectorPackagerConnection tests.
30+
*
31+
* The Executor template parameter controls how asynchronous work is executed:
32+
* - folly::QueuedImmediateExecutor: Work is executed immediately inline.
33+
* - folly::ManualExecutor: Work must be manually advanced in the test.
34+
*/
35+
template <typename Executor>
36+
class InspectorPackagerConnectionTestBase : public testing::Test {
37+
protected:
38+
InspectorPackagerConnectionTestBase()
39+
: packagerConnection_(
40+
InspectorPackagerConnection{
41+
"ws://mock-host:12345",
42+
"my-device",
43+
"my-app",
44+
packagerConnectionDelegates_.make_unique(asyncExecutor_)})
45+
{
46+
auto makeSocket = webSockets_.lazily_make_unique<const std::string &, std::weak_ptr<IWebSocketDelegate>>();
47+
ON_CALL(*packagerConnectionDelegate(), connectWebSocket(testing::_, testing::_))
48+
.WillByDefault([makeSocket](auto &&...args) {
49+
auto socket = makeSocket(std::forward<decltype(args)>(args)...);
50+
socket->getDelegate().didOpen();
51+
return std::move(socket);
52+
});
53+
EXPECT_CALL(*packagerConnectionDelegate(), connectWebSocket(testing::_, testing::_)).Times(testing::AnyNumber());
54+
}
55+
56+
void TearDown() override
57+
{
58+
// Forcibly clean up all pages currently registered with the inspector in
59+
// order to isolate state between tests. NOTE: Using TearDown instead of a
60+
// destructor so that we can use FAIL() etc.
61+
std::vector<int> pagesToRemove;
62+
auto pages = getInspectorInstance().getPages();
63+
int liveConnectionCount = 0;
64+
for (size_t i = 0; i != localConnections_.objectsVended(); ++i) {
65+
if (localConnections_[i] != nullptr) {
66+
liveConnectionCount++;
67+
// localConnections_[i] is a strict mock and will complain when we
68+
// removePage if the call is unexpected.
69+
EXPECT_CALL(*localConnections_[i], disconnect());
70+
}
71+
}
72+
for (auto &page : pages) {
73+
getInspectorInstance().removePage(page.id);
74+
}
75+
if (!pages.empty() && (liveConnectionCount != 0)) {
76+
if (!::testing::Test::HasFailure()) {
77+
FAIL() << "Test case ended with " << liveConnectionCount << " open connection(s) and " << pages.size()
78+
<< " registered page(s). You must manually call removePage for each page.";
79+
}
80+
}
81+
::testing::Test::TearDown();
82+
}
83+
84+
MockInspectorPackagerConnectionDelegate *packagerConnectionDelegate()
85+
{
86+
// We only create one PackagerConnectionDelegate per test.
87+
EXPECT_EQ(packagerConnectionDelegates_.objectsVended(), 1);
88+
return packagerConnectionDelegates_[0];
89+
}
90+
91+
Executor asyncExecutor_;
92+
93+
UniquePtrFactory<MockInspectorPackagerConnectionDelegate> packagerConnectionDelegates_;
94+
/**
95+
* webSockets_ will hold the WebSocket instance(s) owned by
96+
* packagerConnection_ while also allowing us to access them during
97+
* the test. We can send messages *to* packagerConnection_ by
98+
* calling webSockets_[i]->getDelegate().didReceiveMessage(...). Messages
99+
* *from* packagerConnection_ will be found as calls to
100+
* webSockets_[i]->send, which is a mock method installed by gmock.
101+
* These are strict mocks, so method calls will fail if they are not
102+
* expected with a corresponding call to EXPECT_CALL(...) - for example
103+
* if unexpected WebSocket messages are sent.
104+
*/
105+
UniquePtrFactory<testing::StrictMock<MockWebSocket>> webSockets_;
106+
/**
107+
* localConnections_ will hold the LocalConnection instances owned
108+
* by packagerConnection_ while also allowing us to access them
109+
* during the test.
110+
* These are strict mocks, so method calls will fail if they are not
111+
* expected with a corresponding call to EXPECT_CALL(...).
112+
*/
113+
UniquePtrFactory<testing::StrictMock<MockLocalConnection>> localConnections_;
114+
std::optional<InspectorPackagerConnection> packagerConnection_;
115+
};
116+
117+
/**
118+
* Standard test fixture that uses QueuedImmediateExecutor.
119+
* Work scheduled on the executor is run immediately inline.
120+
*/
121+
using InspectorPackagerConnectionTest = InspectorPackagerConnectionTestBase<folly::QueuedImmediateExecutor>;
122+
123+
/**
124+
* Test fixture that uses ManualExecutor.
125+
* Work scheduled on the executor is *not* run automatically; it must be
126+
* manually advanced in the body of the test.
127+
*/
128+
class InspectorPackagerConnectionTestAsync : public InspectorPackagerConnectionTestBase<folly::ManualExecutor> {
129+
public:
130+
void TearDown() override
131+
{
132+
// Assert there are no pending tasks on the ManualExecutor.
133+
auto tasksCleared = asyncExecutor_.clear();
134+
EXPECT_EQ(tasksCleared, 0)
135+
<< "There were still pending tasks on asyncExecutor_ at the end of the test. Use advance() or run() as needed.";
136+
InspectorPackagerConnectionTestBase<folly::ManualExecutor>::TearDown();
137+
}
138+
};
139+
140+
} // namespace facebook::react::jsinspector_modern

0 commit comments

Comments
 (0)