From b469f43982354eb7a36c1ad80580d94f7384b3b6 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Fri, 28 Jul 2023 22:13:47 +0000 Subject: binderRpcTest: -= tuple += named values Tuple by index is hard to verify the same semantics are preserved in all places, but when we get values directly by name, local checks make it clear that we use the same semantics everywhere. This change supports optimizing these tests, so we can easily select the combinations we want to run later. Bug: 292808096 Bug: 292820454 Test: binderRpcTest Change-Id: I8cac77d631cd2bb7d5a4c47ae4bbc267b050b0ce --- libs/binder/tests/binderRpcTest.cpp | 73 ++++++++++++++++++++------- libs/binder/tests/binderRpcTestFixture.h | 35 ++++++++----- libs/binder/tests/binderRpcTestTrusty.cpp | 33 ++++++++---- libs/binder/tests/binderRpcUniversalTests.cpp | 2 +- 4 files changed, 101 insertions(+), 42 deletions(-) diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index d352ce5bca..bf43edcbe5 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -249,12 +249,12 @@ std::unique_ptr BinderRpc::createRpcTestSocketServerProcessEtc( CHECK_EQ(options.numIncomingConnectionsBySession.size(), options.numSessions); } - SocketType socketType = std::get<0>(GetParam()); - RpcSecurity rpcSecurity = std::get<1>(GetParam()); - uint32_t clientVersion = std::get<2>(GetParam()); - uint32_t serverVersion = std::get<3>(GetParam()); - bool singleThreaded = std::get<4>(GetParam()); - bool noKernel = std::get<5>(GetParam()); + SocketType socketType = GetParam().type; + RpcSecurity rpcSecurity = GetParam().security; + uint32_t clientVersion = GetParam().clientVersion; + uint32_t serverVersion = GetParam().serverVersion; + bool singleThreaded = GetParam().singleThreaded; + bool noKernel = GetParam().noKernel; std::string path = android::base::GetExecutableDirectory(); auto servicePath = android::base::StringPrintf("%s/binder_rpc_test_service%s%s", path.c_str(), @@ -1121,12 +1121,27 @@ TEST_P(BinderRpc, Fds) { } #ifdef BINDER_RPC_TO_TRUSTY_TEST -INSTANTIATE_TEST_CASE_P(Trusty, BinderRpc, - ::testing::Combine(::testing::Values(SocketType::TIPC), - ::testing::Values(RpcSecurity::RAW), - ::testing::ValuesIn(testVersions()), - ::testing::ValuesIn(testVersions()), - ::testing::Values(true), ::testing::Values(true)), + +static std::vector getTrustyBinderRpcParams() { + std::vector ret; + + for (const auto& clientVersion : testVersions()) { + for (const auto& serverVersion : testVersions()) { + ret.push_back(BinderRpc::ParamType{ + .type = SocketType::TIPC, + .security = RpcSecurity::RAW, + .clientVersion = clientVersion, + .serverVersion = serverVersion, + .singleThreaded = true, + .noKernel = true, + }); + } + } + + return ret; +} + +INSTANTIATE_TEST_CASE_P(Trusty, BinderRpc, ::testing::ValuesIn(getTrustyBinderRpcParams()), BinderRpc::PrintParamInfo); #else // BINDER_RPC_TO_TRUSTY_TEST bool testSupportVsockLoopback() { @@ -1246,13 +1261,33 @@ static std::vector testSocketTypes(bool hasPreconnected = true) { return ret; } -INSTANTIATE_TEST_CASE_P(PerSocket, BinderRpc, - ::testing::Combine(::testing::ValuesIn(testSocketTypes()), - ::testing::ValuesIn(RpcSecurityValues()), - ::testing::ValuesIn(testVersions()), - ::testing::ValuesIn(testVersions()), - ::testing::Values(false, true), - ::testing::Values(false, true)), +static std::vector getBinderRpcParams() { + std::vector ret; + + for (const auto& type : testSocketTypes()) { + for (const auto& security : RpcSecurityValues()) { + for (const auto& clientVersion : testVersions()) { + for (const auto& serverVersion : testVersions()) { + for (bool singleThreaded : {false, true}) { + for (bool noKernel : {false, true}) { + ret.push_back(BinderRpc::ParamType{ + .type = type, + .security = security, + .clientVersion = clientVersion, + .serverVersion = serverVersion, + .singleThreaded = singleThreaded, + .noKernel = noKernel, + }); + } + } + } + } + } + } + return ret; +} + +INSTANTIATE_TEST_CASE_P(PerSocket, BinderRpc, ::testing::ValuesIn(getBinderRpcParams()), BinderRpc::PrintParamInfo); class BinderRpcServerRootObject diff --git a/libs/binder/tests/binderRpcTestFixture.h b/libs/binder/tests/binderRpcTestFixture.h index 0b8920b5a6..2c9646b30e 100644 --- a/libs/binder/tests/binderRpcTestFixture.h +++ b/libs/binder/tests/binderRpcTestFixture.h @@ -106,15 +106,23 @@ struct BinderRpcTestProcessSession { } }; -class BinderRpc : public ::testing::TestWithParam< - std::tuple> { +struct BinderRpcParam { + SocketType type; + RpcSecurity security; + uint32_t clientVersion; + uint32_t serverVersion; + bool singleThreaded; + bool noKernel; +}; +class BinderRpc : public ::testing::TestWithParam { public: - SocketType socketType() const { return std::get<0>(GetParam()); } - RpcSecurity rpcSecurity() const { return std::get<1>(GetParam()); } - uint32_t clientVersion() const { return std::get<2>(GetParam()); } - uint32_t serverVersion() const { return std::get<3>(GetParam()); } - bool serverSingleThreaded() const { return std::get<4>(GetParam()); } - bool noKernel() const { return std::get<5>(GetParam()); } + // TODO: avoid unnecessary layer of indirection + SocketType socketType() const { return GetParam().type; } + RpcSecurity rpcSecurity() const { return GetParam().security; } + uint32_t clientVersion() const { return GetParam().clientVersion; } + uint32_t serverVersion() const { return GetParam().serverVersion; } + bool serverSingleThreaded() const { return GetParam().singleThreaded; } + bool noKernel() const { return GetParam().noKernel; } bool clientOrServerSingleThreaded() const { return !kEnableRpcThreads || serverSingleThreaded(); @@ -148,15 +156,16 @@ public: } static std::string PrintParamInfo(const testing::TestParamInfo& info) { - auto [type, security, clientVersion, serverVersion, singleThreaded, noKernel] = info.param; - auto ret = PrintToString(type) + "_" + newFactory(security)->toCString() + "_clientV" + - std::to_string(clientVersion) + "_serverV" + std::to_string(serverVersion); - if (singleThreaded) { + auto ret = PrintToString(info.param.type) + "_" + + newFactory(info.param.security)->toCString() + "_clientV" + + std::to_string(info.param.clientVersion) + "_serverV" + + std::to_string(info.param.serverVersion); + if (info.param.singleThreaded) { ret += "_single_threaded"; } else { ret += "_multi_threaded"; } - if (noKernel) { + if (info.param.noKernel) { ret += "_no_kernel"; } else { ret += "_with_kernel"; diff --git a/libs/binder/tests/binderRpcTestTrusty.cpp b/libs/binder/tests/binderRpcTestTrusty.cpp index 28be10db76..fcb83bdabd 100644 --- a/libs/binder/tests/binderRpcTestTrusty.cpp +++ b/libs/binder/tests/binderRpcTestTrusty.cpp @@ -57,9 +57,9 @@ std::unique_ptr BinderRpc::createRpcTestSocketServerProcessEtc( [](size_t n) { return n != 0; }), "Non-zero incoming connections on Trusty"); - RpcSecurity rpcSecurity = std::get<1>(GetParam()); - uint32_t clientVersion = std::get<2>(GetParam()); - uint32_t serverVersion = std::get<3>(GetParam()); + RpcSecurity rpcSecurity = GetParam().security; + uint32_t clientVersion = GetParam().clientVersion; + uint32_t serverVersion = GetParam().serverVersion; auto ret = std::make_unique(); @@ -89,12 +89,27 @@ std::unique_ptr BinderRpc::createRpcTestSocketServerProcessEtc( return ret; } -INSTANTIATE_TEST_CASE_P(Trusty, BinderRpc, - ::testing::Combine(::testing::Values(SocketType::TIPC), - ::testing::Values(RpcSecurity::RAW), - ::testing::ValuesIn(testVersions()), - ::testing::ValuesIn(testVersions()), - ::testing::Values(false), ::testing::Values(true)), +static std::vector getTrustyBinderRpcParams() { + std::vector ret; + + for (const auto& clientVersion : testVersions()) { + for (const auto& serverVersion : testVersions()) { + ret.push_back(BinderRpc::ParamType{ + .type = SocketType::TIPC, + .security = RpcSecurity::RAW, + .clientVersion = clientVersion, + .serverVersion = serverVersion, + // TODO: should we test both versions here? + .singleThreaded = false, + .noKernel = true, + }); + } + } + + return ret; +} + +INSTANTIATE_TEST_CASE_P(Trusty, BinderRpc, ::testing::ValuesIn(getTrustyBinderRpcParams()), BinderRpc::PrintParamInfo); } // namespace android diff --git a/libs/binder/tests/binderRpcUniversalTests.cpp b/libs/binder/tests/binderRpcUniversalTests.cpp index 1f4601010c..e43508ee79 100644 --- a/libs/binder/tests/binderRpcUniversalTests.cpp +++ b/libs/binder/tests/binderRpcUniversalTests.cpp @@ -84,7 +84,7 @@ TEST_P(BinderRpc, SeparateRootObject) { GTEST_SKIP() << "This test requires a multi-threaded service"; } - SocketType type = std::get<0>(GetParam()); + SocketType type = GetParam().type; if (type == SocketType::PRECONNECTED || type == SocketType::UNIX || type == SocketType::UNIX_BOOTSTRAP || type == SocketType::UNIX_RAW) { // we can't get port numbers for unix sockets -- cgit v1.2.3-59-g8ed1b From f7421438375b98158021f7bc511c2cd074092d29 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Fri, 28 Jul 2023 22:41:44 +0000 Subject: binderRpcTest: meet presubmit SLO time Now, we only do all combinations with UNIX raw sockets, and with the other socket types, we run tests in the most common mode. This may cause some issues to be missed, but it's required to hit SLO for Android testing now. Fixes: 292808096 Fixes: 292820454 Test: binderRpcTest (which is the slowest variant) takes about 5 minutes now Change-Id: I069e633f4fce34de431595af91a6cd91e89239fb --- libs/binder/tests/binderRpcTest.cpp | 40 +++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index bf43edcbe5..5ed9603793 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -1264,26 +1264,40 @@ static std::vector testSocketTypes(bool hasPreconnected = true) { static std::vector getBinderRpcParams() { std::vector ret; + constexpr bool full = false; + for (const auto& type : testSocketTypes()) { - for (const auto& security : RpcSecurityValues()) { - for (const auto& clientVersion : testVersions()) { - for (const auto& serverVersion : testVersions()) { - for (bool singleThreaded : {false, true}) { - for (bool noKernel : {false, true}) { - ret.push_back(BinderRpc::ParamType{ - .type = type, - .security = security, - .clientVersion = clientVersion, - .serverVersion = serverVersion, - .singleThreaded = singleThreaded, - .noKernel = noKernel, - }); + if (full || type == SocketType::UNIX) { + for (const auto& security : RpcSecurityValues()) { + for (const auto& clientVersion : testVersions()) { + for (const auto& serverVersion : testVersions()) { + for (bool singleThreaded : {false, true}) { + for (bool noKernel : {false, true}) { + ret.push_back(BinderRpc::ParamType{ + .type = type, + .security = security, + .clientVersion = clientVersion, + .serverVersion = serverVersion, + .singleThreaded = singleThreaded, + .noKernel = noKernel, + }); + } } } } } + } else { + ret.push_back(BinderRpc::ParamType{ + .type = type, + .security = RpcSecurity::RAW, + .clientVersion = RPC_WIRE_PROTOCOL_VERSION, + .serverVersion = RPC_WIRE_PROTOCOL_VERSION, + .singleThreaded = false, + .noKernel = false, + }); } } + return ret; } -- cgit v1.2.3-59-g8ed1b