Skip to content

Commit 183ecd5

Browse files
mszabo-wikiamszabo-wikia
committed
Fix tests
1 parent 8747563 commit 183ecd5

17 files changed

+142
-42
lines changed

‎mcrouter/lib/network/test/AsyncMcClientTestSync.cpp‎

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8+
#include <fstream>
9+
#include <stdexcept>
810
#include <string>
911

1012
#include <gtest/gtest.h>
@@ -869,6 +871,9 @@ TEST(AsyncMcClient, caretVersionUserSpecified) {
869871
}
870872

871873
TEST(AsyncMcClient, caretAdditionalFields) {
874+
#ifdef LIBMC_FBTRACE_DISABLE
875+
GTEST_SKIP() << "Tracing is disabled in the OSS build";
876+
#endif
872877
TestServer::Config config;
873878
config.useSsl = false;
874879
auto server = TestServer::create(std::move(config));
@@ -958,6 +963,26 @@ using TFOTestParams = std::tuple<bool, bool, bool, SecurityMech>;
958963

959964
class AsyncMcClientTFOTest : public TestWithParam<TFOTestParams> {};
960965

966+
/**
967+
* Check whether the current host system supports TCP fastopen.
968+
*/
969+
bool tfoSupportedOnHost() {
970+
try {
971+
constexpr int kClientSupport = 1;
972+
constexpr int kDataInOpeningSynWithoutCookie = 4;
973+
int flags;
974+
std::ifstream procFs("/proc/sys/net/ipv4/tcp_fastopen");
975+
procFs >> flags;
976+
977+
return
978+
(flags & kClientSupport) == kClientSupport &&
979+
(flags & kDataInOpeningSynWithoutCookie) == kDataInOpeningSynWithoutCookie;
980+
} catch (std::exception& e) {
981+
LOG(ERROR) << "Could not read TCP fastopen sysctl: " << e.what();
982+
return false;
983+
}
984+
}
985+
961986
TEST_P(AsyncMcClientTFOTest, testTfoWithSSL) {
962987
auto serverEnabled = std::get<0>(GetParam());
963988
auto clientEnabled = std::get<1>(GetParam());
@@ -971,9 +996,10 @@ TEST_P(AsyncMcClientTFOTest, testTfoWithSSL) {
971996

972997
auto offloadHandshake = std::get<2>(GetParam());
973998
auto constexpr nConnAttempts = 10;
999+
auto tfoSupported = tfoSupportedOnHost();
9741000

9751001
auto mech = std::get<3>(GetParam());
976-
auto sendReq = [serverEnabled, clientEnabled, mech](TestClient& client) {
1002+
auto sendReq = [serverEnabled, clientEnabled, tfoSupported, mech](TestClient& client) {
9771003
client.setConnectionStatusCallbacks(
9781004
[&](const folly::AsyncTransportWrapper& sock, int64_t) {
9791005
if (mech == SecurityMech::TLS_TO_PLAINTEXT) {
@@ -982,10 +1008,16 @@ TEST_P(AsyncMcClientTFOTest, testTfoWithSSL) {
9821008
auto stats = socket->getStats();
9831009
if (clientEnabled) {
9841010
EXPECT_TRUE(stats.tfoAttempted);
985-
EXPECT_TRUE(stats.tfoFinished);
986-
// we can not guarantee socket->getTFOSucceeded() will return true
987-
// unless there are specific kernel + host settings applied
988-
if (!serverEnabled) {
1011+
1012+
if (tfoSupported) {
1013+
EXPECT_TRUE(stats.tfoFinished);
1014+
if (serverEnabled) {
1015+
EXPECT_TRUE(stats.tfoSuccess);
1016+
} else {
1017+
EXPECT_FALSE(stats.tfoSuccess);
1018+
}
1019+
} else {
1020+
EXPECT_FALSE(stats.tfoFinished);
9891021
EXPECT_FALSE(stats.tfoSuccess);
9901022
}
9911023
} else {
@@ -995,10 +1027,16 @@ TEST_P(AsyncMcClientTFOTest, testTfoWithSSL) {
9951027
auto* socket = sock.getUnderlyingTransport<folly::AsyncSocket>();
9961028
if (clientEnabled) {
9971029
EXPECT_TRUE(socket->getTFOAttempted());
998-
EXPECT_TRUE(socket->getTFOFinished());
999-
// we can not guarantee socket->getTFOSucceeded() will return true
1000-
// unless there are specific kernel + host settings applied
1001-
if (!serverEnabled) {
1030+
1031+
if (tfoSupported) {
1032+
EXPECT_TRUE(socket->getTFOFinished());
1033+
if (serverEnabled) {
1034+
EXPECT_TRUE(socket->getTFOSucceded());
1035+
} else {
1036+
EXPECT_FALSE(socket->getTFOSucceded());
1037+
}
1038+
} else {
1039+
EXPECT_FALSE(socket->getTFOFinished());
10021040
EXPECT_FALSE(socket->getTFOSucceded());
10031041
}
10041042
} else {
@@ -1143,6 +1181,9 @@ TEST_P(AsyncMcClientSSLOffloadTest, closeNow) {
11431181
EXPECT_FALSE(upCalled);
11441182
EXPECT_TRUE(downReason.has_value());
11451183
EXPECT_EQ(*downReason, ConnectionDownReason::ABORTED);
1184+
1185+
server->shutdown();
1186+
server->join();
11461187
}
11471188

11481189
TEST_P(AsyncMcClientSSLOffloadTest, clientReset) {
@@ -1164,6 +1205,9 @@ TEST_P(AsyncMcClientSSLOffloadTest, clientReset) {
11641205
evb.loopOnce();
11651206
client.reset();
11661207
evb.loop();
1208+
1209+
server->shutdown();
1210+
server->join();
11671211
}
11681212

11691213
INSTANTIATE_TEST_CASE_P(AsyncMcClientTest, AsyncMcClientSSLOffloadTest, Bool());

‎mcrouter/lib/network/test/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ target_link_libraries(
3030
FBThrift::thrift-core)
3131

3232
add_executable(mcrouter_mock_mc_thrift_server MockMc.cpp MockMcThriftServer.cpp)
33+
set_property(TARGET mcrouter_mock_mc_thrift_server PROPERTY OUTPUT_NAME mock_mc_thrift_server)
3334

3435
target_link_libraries(
3536
mcrouter_mock_mc_thrift_server

‎mcrouter/lib/network/test/MockMc.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,10 @@ MockMc::findUnexpired(folly::StringPiece key) {
245245
return it;
246246
}
247247

248+
size_t MockMc::itemCount() const noexcept {
249+
return citems_.size();
250+
}
251+
248252
void MockMc::flushAll() {
249253
citems_.clear();
250254
}

‎mcrouter/lib/network/test/MockMc.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,8 @@ class MockMc {
146146

147147
CasResult cas(folly::StringPiece key, Item item, uint64_t token);
148148

149+
size_t itemCount() const noexcept;
150+
149151
/**
150152
* clear all items
151153
*/

‎mcrouter/lib/network/test/MockMcOnRequest.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ namespace facebook {
1919
namespace memcache {
2020

2121
static std::atomic_uint64_t cmd_get_count{0};
22+
static std::atomic_uint64_t cmd_lease_get_count{0};
23+
static std::atomic_uint64_t cmd_lease_set_count{0};
2224

2325
class MockMcOnRequest {
2426
public:
@@ -127,6 +129,7 @@ class MockMcOnRequest {
127129
template <class Context>
128130
void onRequest(Context&& ctx, McLeaseGetRequest&& req) {
129131
using Reply = McLeaseGetReply;
132+
++cmd_lease_get_count;
130133

131134
auto key = req.key_ref()->fullKey().str();
132135

@@ -144,6 +147,7 @@ class MockMcOnRequest {
144147
template <class Context>
145148
void onRequest(Context&& ctx, McLeaseSetRequest&& req) {
146149
using Reply = McLeaseSetReply;
150+
++cmd_lease_set_count;
147151

148152
auto key = req.key_ref()->fullKey().str();
149153

@@ -355,6 +359,9 @@ class MockMcOnRequest {
355359
if (key == "__mockmc__") {
356360
StatsReply stats;
357361
stats.addStat("cmd_get_count", cmd_get_count.load());
362+
stats.addStat("cmd_lease_get", cmd_lease_get_count.load());
363+
stats.addStat("cmd_lease_set", cmd_lease_set_count.load());
364+
stats.addStat("total_items", mc_.itemCount());
358365
Context::reply(std::move(ctx), stats.getReply());
359366
} else {
360367
Context::reply(std::move(ctx), Reply(carbon::Result::BAD_COMMAND));

‎mcrouter/lib/network/test/TestClientServerUtil.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ void TestServerOnRequest::onRequest(
8787
processReply(std::move(ctx), Reply(carbon::Result::NOTFOUND));
8888
} else if (req.key_ref()->fullKey() == "shutdown") {
8989
shutdownLock_.post();
90+
// TODO
91+
std::this_thread::sleep_for(std::chrono::milliseconds(100));
9092
processReply(std::move(ctx), Reply(carbon::Result::NOTFOUND));
9193
flushQueue();
9294
} else if (req.key_ref()->fullKey() == "busy") {

‎mcrouter/test/MCProcess.py‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,6 @@ def stats(self, spec=None):
591591
if spec:
592592
q = "stats {spec}\r\n".format(spec=spec)
593593
self._sendall(q)
594-
595594
s = {}
596595
line = None
597596
fds = select.select([self.fd], [], [], 20.0)
@@ -800,13 +799,14 @@ def __init__(self, args, port=None, base_dir=None):
800799
self.stats_dir,
801800
"--debug-fifo-root",
802801
self.debug_fifo_root,
803-
"--rss-limit-mb",
804-
"16384",
805802
"--fibers-stack-size",
806803
"65536",
807804
]
808805
)
809806

807+
if not McrouterGlobals.ossVersion():
808+
args.extend(["--rss-limit-mb", "16384"])
809+
810810
listen_sock = None
811811
pass_fds = []
812812
if port is None:
@@ -1030,7 +1030,8 @@ def __init__(self, port=None, ssl_port=None, extra_args=None):
10301030
pass_fds = []
10311031

10321032
# if mockmc is used here, we initialize the same way as MockMemcached
1033-
if McrouterGlobals.binPath("mockmc") == args[0]:
1033+
self.is_mock_server = McrouterGlobals.binPath("mockmc") == args[0]
1034+
if self.is_mock_server:
10341035
if port is None:
10351036
listen_sock = create_listen_socket()
10361037
port = listen_sock.getsockname()[1]
@@ -1103,6 +1104,11 @@ def __init__(self, port=None, ssl_port=None, extra_args=None):
11031104
tries -= 1
11041105
self.disconnect()
11051106

1107+
def stats(self, spec=None):
1108+
if self.is_mock_server:
1109+
return super().stats('__mockmc__')
1110+
return super().stats(spec)
1111+
11061112
def getsslport(self):
11071113
return self.ssl_port
11081114

‎mcrouter/test/cpp_unit_tests/McrouterClientUsage.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ TEST(CarbonRouterClient, basicUsageSameThreadClient) {
116116
// gracefully. This ensures graceful destruction of the static
117117
// CarbonRouterInstance instance.
118118
router->shutdown();
119+
ioThreadPool->join();
119120
ioThreadPool.reset();
120121
EXPECT_TRUE(replyReceived);
121122
}

‎mcrouter/test/mcrouter_config.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ def binPath(name):
2121
"mcrouter": "./mcrouter/mcrouter",
2222
"mcpiper": "./mcrouter/tools/mcpiper/mcpiper",
2323
"mockmc": "./mcrouter/lib/network/test/mock_mc_server",
24+
"mockmcthrift": "./mcrouter/lib/network/test/mock_mc_thrift_server",
2425
"mockmcdual": "./mcrouter/lib/network/test/mock_mc_server_dual",
2526
"prodmc": "./mcrouter/lib/network/test/mock_mc_server",
2627
}
@@ -34,6 +35,10 @@ def preprocessArgs(args):
3435
def useThriftClient():
3536
return False
3637

38+
@staticmethod
39+
def ossVersion() -> bool:
40+
return True
41+
3742
@staticmethod
3843
def createThriftTestClient(addr, port) -> ThriftTestClient:
3944
raise NotImplementedError

‎mcrouter/test/test_async_files_attr.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,8 @@
99
import os
1010
import time
1111

12-
from libfb.py import parutil
1312
from mcrouter.test.McrouterTestCase import McrouterTestCase
14-
13+
from mcrouter.test.config import McrouterGlobals
1514

1615
class TestAsyncFilesAttr(McrouterTestCase):
1716
stat_prefix = "libmcrouter.mcrouter.0."
@@ -53,6 +52,9 @@ def test_stats_no_requests(self):
5352
self.check_stats(mcrouter.stats_dir)
5453

5554
def test_async_files_attr(self):
55+
if McrouterGlobals.ossVersion():
56+
self.skipTest("The mcrouter client is not available in the OSS build")
57+
from libfb.py import parutil
5658
mcrouter = self.add_mcrouter(self.config, extra_args=self.extra_args)
5759
binary = parutil.get_file_path("mcrouter/client_binary")
5860
port = str(mcrouter.getport())

0 commit comments

Comments
 (0)