From 0ddd5410e3d4dd7323d0b1b4651c8a7b085806c4 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:33:58 +0000 Subject: [PATCH 1/7] Update OWNERS for Weaver Weaver does not have a clear owner, but list the people who seem to be most involved with it currently. Bug: 291284381 Test: N/A Change-Id: I69de46f8154bf91272a7197ce71c13c745a7208e (cherry picked from commit f4483a2ce98b8d863c84b4148f788ccd85db7086) Merged-In: I69de46f8154bf91272a7197ce71c13c745a7208e --- weaver/1.0/vts/functional/OWNERS | 3 --- weaver/OWNERS | 6 ++++++ weaver/aidl/vts/OWNERS | 2 -- 3 files changed, 6 insertions(+), 5 deletions(-) delete mode 100644 weaver/1.0/vts/functional/OWNERS create mode 100644 weaver/OWNERS delete mode 100644 weaver/aidl/vts/OWNERS diff --git a/weaver/1.0/vts/functional/OWNERS b/weaver/1.0/vts/functional/OWNERS deleted file mode 100644 index ec8c304b67..0000000000 --- a/weaver/1.0/vts/functional/OWNERS +++ /dev/null @@ -1,3 +0,0 @@ -# Bug component: 186411 -chengyouho@google.com -frankwoo@google.com diff --git a/weaver/OWNERS b/weaver/OWNERS new file mode 100644 index 0000000000..7e579f63cd --- /dev/null +++ b/weaver/OWNERS @@ -0,0 +1,6 @@ +# Bug component: 1081729 +ebiggers@google.com +paulcrowley@google.com +swillden@google.com +wfrichar@google.com +chengyouho@google.com diff --git a/weaver/aidl/vts/OWNERS b/weaver/aidl/vts/OWNERS deleted file mode 100644 index 40d95e4bf0..0000000000 --- a/weaver/aidl/vts/OWNERS +++ /dev/null @@ -1,2 +0,0 @@ -chengyouho@google.com -frankwoo@google.com From 1cd59722b67b7b745a74ea30288d4bd6f754491d Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:33:59 +0000 Subject: [PATCH 2/7] Make VtsHalWeaverTargetTest test both HIDL and AIDL services VtsHalWeaverTargetTest and VtsHalWeaverV1_0TargetTest are identical except for whether they use AIDL or HIDL. Unfortunately, the HIDL test is needed for several more years. For now, we have to make some substantial fixes to both tests. To make continued maintenance of this test easier, update VtsHalWeaverTargetTest to handle both AIDL and HIDL. The test cases are still written in terms of the AIDL API, so it should still be straightforward to remove HIDL support when the time comes. Bug: 291284381 Test: 'atest VtsHalWeaverTargetTest' on bramble Change-Id: I6b760930146ad1b08f17ef810a86c0058601c3bf (cherry picked from commit b59654f239745583f53fda7d62bb06f1144a347d) Merged-In: I6b760930146ad1b08f17ef810a86c0058601c3bf --- weaver/aidl/vts/Android.bp | 5 +- weaver/aidl/vts/VtsHalWeaverTargetTest.cpp | 200 ++++++++++++++++++--- 2 files changed, 181 insertions(+), 24 deletions(-) diff --git a/weaver/aidl/vts/Android.bp b/weaver/aidl/vts/Android.bp index 557fe47aa7..ee03b28136 100644 --- a/weaver/aidl/vts/Android.bp +++ b/weaver/aidl/vts/Android.bp @@ -34,7 +34,10 @@ cc_test { "libbinder_ndk", "libbase", ], - static_libs: ["android.hardware.weaver-V2-ndk"], + static_libs: [ + "android.hardware.weaver-V2-ndk", + "android.hardware.weaver@1.0", + ], test_suites: [ "general-tests", "vts", diff --git a/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp b/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp index f016515a6a..b75a56f039 100644 --- a/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp +++ b/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp @@ -19,6 +19,9 @@ #include #include #include +#include +#include +#include #include @@ -27,29 +30,162 @@ using ::aidl::android::hardware::weaver::WeaverConfig; using ::aidl::android::hardware::weaver::WeaverReadResponse; using ::aidl::android::hardware::weaver::WeaverReadStatus; -using ::ndk::SpAIBinder; +using HidlIWeaver = ::android::hardware::weaver::V1_0::IWeaver; +using HidlWeaverConfig = ::android::hardware::weaver::V1_0::WeaverConfig; +using HidlWeaverReadStatus = ::android::hardware::weaver::V1_0::WeaverReadStatus; +using HidlWeaverReadResponse = ::android::hardware::weaver::V1_0::WeaverReadResponse; +using HidlWeaverStatus = ::android::hardware::weaver::V1_0::WeaverStatus; const std::vector KEY{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; const std::vector WRONG_KEY{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; const std::vector VALUE{16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}; const std::vector OTHER_VALUE{0, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233, 255, 255}; -struct WeaverAidlTest : public ::testing::TestWithParam { - virtual void SetUp() override { - weaver = IWeaver::fromBinder( - SpAIBinder(AServiceManager_waitForService(GetParam().c_str()))); - ASSERT_NE(weaver, nullptr); +class WeaverAdapter { + public: + virtual ~WeaverAdapter() {} + virtual bool isReady() = 0; + virtual ::ndk::ScopedAStatus getConfig(WeaverConfig* _aidl_return) = 0; + virtual ::ndk::ScopedAStatus read(int32_t in_slotId, const std::vector& in_key, + WeaverReadResponse* _aidl_return) = 0; + virtual ::ndk::ScopedAStatus write(int32_t in_slotId, const std::vector& in_key, + const std::vector& in_value) = 0; +}; + +class WeaverAidlAdapter : public WeaverAdapter { + public: + WeaverAidlAdapter(const std::string& param) + : aidl_weaver_(IWeaver::fromBinder( + ::ndk::SpAIBinder(AServiceManager_waitForService(param.c_str())))) {} + ~WeaverAidlAdapter() {} + + bool isReady() { return aidl_weaver_ != nullptr; } + + ::ndk::ScopedAStatus getConfig(WeaverConfig* _aidl_return) { + return aidl_weaver_->getConfig(_aidl_return); } - virtual void TearDown() override {} + ::ndk::ScopedAStatus read(int32_t in_slotId, const std::vector& in_key, + WeaverReadResponse* _aidl_return) { + return aidl_weaver_->read(in_slotId, in_key, _aidl_return); + } - std::shared_ptr weaver; + ::ndk::ScopedAStatus write(int32_t in_slotId, const std::vector& in_key, + const std::vector& in_value) { + return aidl_weaver_->write(in_slotId, in_key, in_value); + } + + private: + std::shared_ptr aidl_weaver_; }; +class WeaverHidlAdapter : public WeaverAdapter { + public: + WeaverHidlAdapter(const std::string& param) : hidl_weaver_(HidlIWeaver::getService(param)) {} + ~WeaverHidlAdapter() {} + + bool isReady() { return hidl_weaver_ != nullptr; } + + ::ndk::ScopedAStatus getConfig(WeaverConfig* _aidl_return) { + bool callbackCalled = false; + HidlWeaverStatus status; + HidlWeaverConfig config; + auto ret = hidl_weaver_->getConfig([&](HidlWeaverStatus s, HidlWeaverConfig c) { + callbackCalled = true; + status = s; + config = c; + }); + if (!ret.isOk() || !callbackCalled || status != HidlWeaverStatus::OK) { + return ::ndk::ScopedAStatus::fromStatus(STATUS_FAILED_TRANSACTION); + } + _aidl_return->slots = config.slots; + _aidl_return->keySize = config.keySize; + _aidl_return->valueSize = config.valueSize; + return ::ndk::ScopedAStatus::ok(); + } + + ::ndk::ScopedAStatus read(int32_t in_slotId, const std::vector& in_key, + WeaverReadResponse* _aidl_return) { + bool callbackCalled = false; + HidlWeaverReadStatus status; + std::vector value; + uint32_t timeout; + auto ret = hidl_weaver_->read(in_slotId, in_key, + [&](HidlWeaverReadStatus s, HidlWeaverReadResponse r) { + callbackCalled = true; + status = s; + value = r.value; + timeout = r.timeout; + }); + if (!ret.isOk() || !callbackCalled) { + return ::ndk::ScopedAStatus::fromStatus(STATUS_FAILED_TRANSACTION); + } + switch (status) { + case HidlWeaverReadStatus::OK: + _aidl_return->status = WeaverReadStatus::OK; + break; + case HidlWeaverReadStatus::FAILED: + _aidl_return->status = WeaverReadStatus::FAILED; + break; + case HidlWeaverReadStatus::INCORRECT_KEY: + _aidl_return->status = WeaverReadStatus::INCORRECT_KEY; + break; + case HidlWeaverReadStatus::THROTTLE: + _aidl_return->status = WeaverReadStatus::THROTTLE; + break; + default: + ADD_FAILURE() << "Unknown HIDL read status: " << static_cast(status); + _aidl_return->status = WeaverReadStatus::FAILED; + break; + } + _aidl_return->value = value; + _aidl_return->timeout = timeout; + return ::ndk::ScopedAStatus::ok(); + } + + ::ndk::ScopedAStatus write(int32_t in_slotId, const std::vector& in_key, + const std::vector& in_value) { + auto status = hidl_weaver_->write(in_slotId, in_key, in_value); + switch (status) { + case HidlWeaverStatus::OK: + return ::ndk::ScopedAStatus::ok(); + case HidlWeaverStatus::FAILED: + return ::ndk::ScopedAStatus::fromStatus(STATUS_FAILED_TRANSACTION); + default: + ADD_FAILURE() << "Unknown HIDL write status: " << status.description(); + return ::ndk::ScopedAStatus::fromStatus(STATUS_FAILED_TRANSACTION); + } + } + + private: + android::sp hidl_weaver_; +}; + +class WeaverTest : public ::testing::TestWithParam> { + protected: + void SetUp() override; + void TearDown() override {} + + std::unique_ptr weaver; +}; + +void WeaverTest::SetUp() { + std::string api, instance_name; + std::tie(api, instance_name) = GetParam(); + if (api == "hidl") { + weaver.reset(new WeaverHidlAdapter(instance_name)); + } else if (api == "aidl") { + weaver.reset(new WeaverAidlAdapter(instance_name)); + } else { + FAIL() << "Bad test parameterization"; + } + ASSERT_TRUE(weaver->isReady()); +} + /* * Checks config values are suitably large */ -TEST_P(WeaverAidlTest, GetConfig) { +TEST_P(WeaverTest, GetConfig) { WeaverConfig config; auto ret = weaver->getConfig(&config); @@ -64,7 +200,7 @@ TEST_P(WeaverAidlTest, GetConfig) { /* * Gets the config twice and checks they are the same */ -TEST_P(WeaverAidlTest, GettingConfigMultipleTimesGivesSameResult) { +TEST_P(WeaverTest, GettingConfigMultipleTimesGivesSameResult) { WeaverConfig config1; WeaverConfig config2; @@ -80,7 +216,7 @@ TEST_P(WeaverAidlTest, GettingConfigMultipleTimesGivesSameResult) { /* * Gets the number of slots from the config and writes a key and value to the last one */ -TEST_P(WeaverAidlTest, WriteToLastSlot) { +TEST_P(WeaverTest, WriteToLastSlot) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); @@ -95,7 +231,7 @@ TEST_P(WeaverAidlTest, WriteToLastSlot) { * Writes a key and value to a slot * Reads the slot with the same key and receives the value that was previously written */ -TEST_P(WeaverAidlTest, WriteFollowedByReadGivesTheSameValue) { +TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { constexpr uint32_t slotId = 0; const auto ret = weaver->write(slotId, KEY, VALUE); ASSERT_TRUE(ret.isOk()); @@ -121,7 +257,7 @@ TEST_P(WeaverAidlTest, WriteFollowedByReadGivesTheSameValue) { * Overwrites the slot with a new key and value * Reads the slot with the new key and receives the new value */ -TEST_P(WeaverAidlTest, OverwritingSlotUpdatesTheValue) { +TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { constexpr uint32_t slotId = 0; const auto initialWriteRet = weaver->write(slotId, WRONG_KEY, VALUE); ASSERT_TRUE(initialWriteRet.isOk()); @@ -149,7 +285,7 @@ TEST_P(WeaverAidlTest, OverwritingSlotUpdatesTheValue) { * Writes a key and value to a slot * Reads the slot with a different key so does not receive the value */ -TEST_P(WeaverAidlTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { +TEST_P(WeaverTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { constexpr uint32_t slotId = 0; const auto ret = weaver->write(slotId, KEY, VALUE); ASSERT_TRUE(ret.isOk()); @@ -171,7 +307,7 @@ TEST_P(WeaverAidlTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { /* * Writing to an invalid slot fails */ -TEST_P(WeaverAidlTest, WritingToInvalidSlotFails) { +TEST_P(WeaverTest, WritingToInvalidSlotFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -188,7 +324,7 @@ TEST_P(WeaverAidlTest, WritingToInvalidSlotFails) { /* * Reading from an invalid slot fails rather than incorrect key */ -TEST_P(WeaverAidlTest, ReadingFromInvalidSlotFails) { +TEST_P(WeaverTest, ReadingFromInvalidSlotFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -218,7 +354,7 @@ TEST_P(WeaverAidlTest, ReadingFromInvalidSlotFails) { /* * Writing a key that is too large fails */ -TEST_P(WeaverAidlTest, WriteWithTooLargeKeyFails) { +TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -233,7 +369,7 @@ TEST_P(WeaverAidlTest, WriteWithTooLargeKeyFails) { /* * Writing a value that is too large fails */ -TEST_P(WeaverAidlTest, WriteWithTooLargeValueFails) { +TEST_P(WeaverTest, WriteWithTooLargeValueFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -248,7 +384,7 @@ TEST_P(WeaverAidlTest, WriteWithTooLargeValueFails) { /* * Reading with a key that is loo large fails */ -TEST_P(WeaverAidlTest, ReadWithTooLargeKeyFails) { +TEST_P(WeaverTest, ReadWithTooLargeKeyFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -273,11 +409,29 @@ TEST_P(WeaverAidlTest, ReadWithTooLargeKeyFails) { EXPECT_EQ(status, WeaverReadStatus::FAILED); } -GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(WeaverAidlTest); +// Instantiate the test for each HIDL Weaver service. INSTANTIATE_TEST_SUITE_P( - PerInstance, WeaverAidlTest, - testing::ValuesIn(android::getAidlHalInstanceNames(IWeaver::descriptor)), - android::PrintInstanceNameToString); + PerHidlInstance, WeaverTest, + testing::Combine(testing::Values("hidl"), + testing::ValuesIn(android::hardware::getAllHalInstanceNames( + HidlIWeaver::descriptor))), + [](const testing::TestParamInfo>& info) { + return android::hardware::PrintInstanceNameToString( + testing::TestParamInfo{std::get<1>(info.param), info.index}); + }); + +// Instantiate the test for each AIDL Weaver service. +INSTANTIATE_TEST_SUITE_P( + PerAidlInstance, WeaverTest, + testing::Combine(testing::Values("aidl"), + testing::ValuesIn(android::getAidlHalInstanceNames(IWeaver::descriptor))), + [](const testing::TestParamInfo>& info) { + // This name_generator makes the instance name be included in the test case names, e.g. + // "PerAidlInstance/WeaverTest#GetConfig/0_android_hardware_weaver_IWeaver_default" + // instead of "PerAidlInstance/WeaverTest#GetConfig/0". + return android::PrintInstanceNameToString( + testing::TestParamInfo{std::get<1>(info.param), info.index}); + }); int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); From a8fc1ea0545a136c59058fb9a2b7103c86f558f1 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:33:59 +0000 Subject: [PATCH 3/7] Move VtsHalWeaverTargetTest to common directory Since VtsHalWeaverTargetTest now handles both AIDL and HIDL, move it from weaver/aidl/vts/ to weaver/vts/. Bug: 291284381 Test: 'atest VtsHalWeaverTargetTest' on bramble Change-Id: Icfa0ff3b22b036110df327674fda44820057aabd (cherry picked from commit f0d6907d20a72391762e3e479e4a5833e4091b3b) Merged-In: Icfa0ff3b22b036110df327674fda44820057aabd --- weaver/{aidl => }/vts/Android.bp | 0 weaver/{aidl => }/vts/VtsHalWeaverTargetTest.cpp | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename weaver/{aidl => }/vts/Android.bp (100%) rename weaver/{aidl => }/vts/VtsHalWeaverTargetTest.cpp (100%) diff --git a/weaver/aidl/vts/Android.bp b/weaver/vts/Android.bp similarity index 100% rename from weaver/aidl/vts/Android.bp rename to weaver/vts/Android.bp diff --git a/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp b/weaver/vts/VtsHalWeaverTargetTest.cpp similarity index 100% rename from weaver/aidl/vts/VtsHalWeaverTargetTest.cpp rename to weaver/vts/VtsHalWeaverTargetTest.cpp From c9691dd624fc6a080c95687936184c31c93b5a56 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:33:59 +0000 Subject: [PATCH 4/7] Remove redundant HIDL Weaver VTS test Now that there is a single Weaver VTS test that covers both the HIDL and AIDL services (weaver/vts/), the HIDL-specific test can be deleted. Bug: 291284381 Test: 'atest VtsHalWeaverTargetTest' on bramble Change-Id: Ie942825c154e6792e6ffdbf0c59248de9de10d92 (cherry picked from commit e2e40d69a6185494855e691e37cea2e40c7edd2f) Merged-In: Ie942825c154e6792e6ffdbf0c59248de9de10d92 --- weaver/1.0/vts/functional/Android.bp | 32 -- .../functional/VtsHalWeaverV1_0TargetTest.cpp | 343 ------------------ 2 files changed, 375 deletions(-) delete mode 100644 weaver/1.0/vts/functional/Android.bp delete mode 100644 weaver/1.0/vts/functional/VtsHalWeaverV1_0TargetTest.cpp diff --git a/weaver/1.0/vts/functional/Android.bp b/weaver/1.0/vts/functional/Android.bp deleted file mode 100644 index cc1d28465d..0000000000 --- a/weaver/1.0/vts/functional/Android.bp +++ /dev/null @@ -1,32 +0,0 @@ -// -// Copyright (C) 2017 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -package { - // See: http://go/android-license-faq - // A large-scale-change added 'default_applicable_licenses' to import - // all of the 'license_kinds' from "hardware_interfaces_license" - // to get the below license kinds: - // SPDX-license-identifier-Apache-2.0 - default_applicable_licenses: ["hardware_interfaces_license"], -} - -cc_test { - name: "VtsHalWeaverV1_0TargetTest", - defaults: ["VtsHalTargetTestDefaults"], - srcs: ["VtsHalWeaverV1_0TargetTest.cpp"], - static_libs: ["android.hardware.weaver@1.0"], - test_suites: ["general-tests", "vts"], -} diff --git a/weaver/1.0/vts/functional/VtsHalWeaverV1_0TargetTest.cpp b/weaver/1.0/vts/functional/VtsHalWeaverV1_0TargetTest.cpp deleted file mode 100644 index 66465a9798..0000000000 --- a/weaver/1.0/vts/functional/VtsHalWeaverV1_0TargetTest.cpp +++ /dev/null @@ -1,343 +0,0 @@ -/* - * Copyright (C) 2017 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include -#include -#include -#include - -#include - -using ::android::hardware::weaver::V1_0::IWeaver; -using ::android::hardware::weaver::V1_0::WeaverConfig; -using ::android::hardware::weaver::V1_0::WeaverReadStatus; -using ::android::hardware::weaver::V1_0::WeaverReadResponse; -using ::android::hardware::weaver::V1_0::WeaverStatus; -using ::android::hardware::Return; -using ::android::sp; - -const std::vector KEY{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; -const std::vector WRONG_KEY{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; -const std::vector VALUE{16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}; -const std::vector OTHER_VALUE{0, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233, 255, 255}; - -struct WeaverHidlTest : public ::testing::TestWithParam { - virtual void SetUp() override { - weaver = IWeaver::getService(GetParam()); - ASSERT_NE(weaver, nullptr); - } - - virtual void TearDown() override {} - - sp weaver; -}; - -/* - * Checks config values are suitably large - */ -TEST_P(WeaverHidlTest, GetConfig) { - WeaverStatus status; - WeaverConfig config; - - bool callbackCalled = false; - auto ret = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - callbackCalled = true; - status = s; - config = c; - }); - ASSERT_TRUE(ret.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverStatus::OK); - - EXPECT_GE(config.slots, 16u); - EXPECT_GE(config.keySize, 16u); - EXPECT_GE(config.valueSize, 16u); -} - -/* - * Gets the config twice and checks they are the same - */ -TEST_P(WeaverHidlTest, GettingConfigMultipleTimesGivesSameResult) { - WeaverConfig config1; - WeaverConfig config2; - - WeaverStatus status; - bool callbackCalled = false; - auto ret = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - callbackCalled = true; - status = s; - config1 = c; - }); - ASSERT_TRUE(ret.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverStatus::OK); - - callbackCalled = false; - ret = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - callbackCalled = true; - status = s; - config2 = c; - }); - ASSERT_TRUE(ret.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverStatus::OK); - - EXPECT_EQ(config1, config2); -} - -/* - * Gets the number of slots from the config and writes a key and value to the last one - */ -TEST_P(WeaverHidlTest, WriteToLastSlot) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - const uint32_t lastSlot = config.slots - 1; - const auto writeRet = weaver->write(lastSlot, KEY, VALUE); - ASSERT_TRUE(writeRet.isOk()); - ASSERT_EQ(writeRet, WeaverStatus::OK); -} - -/* - * Writes a key and value to a slot - * Reads the slot with the same key and receives the value that was previously written - */ -TEST_P(WeaverHidlTest, WriteFollowedByReadGivesTheSameValue) { - constexpr uint32_t slotId = 0; - const auto ret = weaver->write(slotId, KEY, VALUE); - ASSERT_TRUE(ret.isOk()); - ASSERT_EQ(ret, WeaverStatus::OK); - - bool callbackCalled = false; - WeaverReadStatus status; - std::vector readValue; - uint32_t timeout; - const auto readRet = weaver->read(slotId, KEY, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - status = s; - readValue = r.value; - timeout = r.timeout; - }); - ASSERT_TRUE(readRet.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverReadStatus::OK); - EXPECT_EQ(readValue, VALUE); - EXPECT_EQ(timeout, 0u); -} - -/* - * Writes a key and value to a slot - * Overwrites the slot with a new key and value - * Reads the slot with the new key and receives the new value - */ -TEST_P(WeaverHidlTest, OverwritingSlotUpdatesTheValue) { - constexpr uint32_t slotId = 0; - const auto initialWriteRet = weaver->write(slotId, WRONG_KEY, VALUE); - ASSERT_TRUE(initialWriteRet.isOk()); - ASSERT_EQ(initialWriteRet, WeaverStatus::OK); - - const auto overwriteRet = weaver->write(slotId, KEY, OTHER_VALUE); - ASSERT_TRUE(overwriteRet.isOk()); - ASSERT_EQ(overwriteRet, WeaverStatus::OK); - - bool callbackCalled = false; - WeaverReadStatus status; - std::vector readValue; - uint32_t timeout; - const auto readRet = weaver->read(slotId, KEY, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - status = s; - readValue = r.value; - timeout = r.timeout; - }); - ASSERT_TRUE(readRet.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverReadStatus::OK); - EXPECT_EQ(readValue, OTHER_VALUE); - EXPECT_EQ(timeout, 0u); -} - -/* - * Writes a key and value to a slot - * Reads the slot with a different key so does not receive the value - */ -TEST_P(WeaverHidlTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { - constexpr uint32_t slotId = 0; - const auto ret = weaver->write(slotId, KEY, VALUE); - ASSERT_TRUE(ret.isOk()); - ASSERT_EQ(ret, WeaverStatus::OK); - - bool callbackCalled = false; - WeaverReadStatus status; - std::vector readValue; - const auto readRet = - weaver->read(slotId, WRONG_KEY, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - status = s; - readValue = r.value; - }); - ASSERT_TRUE(callbackCalled); - ASSERT_TRUE(readRet.isOk()); - ASSERT_EQ(status, WeaverReadStatus::INCORRECT_KEY); - EXPECT_TRUE(readValue.empty()); -} - -/* - * Writing to an invalid slot fails - */ -TEST_P(WeaverHidlTest, WritingToInvalidSlotFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - if (config.slots == std::numeric_limits::max()) { - // If there are no invalid slots then pass - return; - } - - const auto writeRet = weaver->write(config.slots, KEY, VALUE); - ASSERT_TRUE(writeRet.isOk()); - ASSERT_EQ(writeRet, WeaverStatus::FAILED); -} - -/* - * Reading from an invalid slot fails rather than incorrect key - */ -TEST_P(WeaverHidlTest, ReadingFromInvalidSlotFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - if (config.slots == std::numeric_limits::max()) { - // If there are no invalid slots then pass - return; - } - - bool callbackCalled = false; - WeaverReadStatus readStatus; - std::vector readValue; - uint32_t timeout; - const auto readRet = - weaver->read(config.slots, KEY, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - readStatus = s; - readValue = r.value; - timeout = r.timeout; - }); - ASSERT_TRUE(callbackCalled); - ASSERT_TRUE(readRet.isOk()); - ASSERT_EQ(readStatus, WeaverReadStatus::FAILED); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(timeout, 0u); -} - -/* - * Writing a key that is too large fails - */ -TEST_P(WeaverHidlTest, WriteWithTooLargeKeyFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - std::vector bigKey(config.keySize + 1); - - constexpr uint32_t slotId = 0; - const auto writeRet = weaver->write(slotId, bigKey, VALUE); - ASSERT_TRUE(writeRet.isOk()); - ASSERT_EQ(writeRet, WeaverStatus::FAILED); -} - -/* - * Writing a value that is too large fails - */ -TEST_P(WeaverHidlTest, WriteWithTooLargeValueFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - std::vector bigValue(config.valueSize + 1); - - constexpr uint32_t slotId = 0; - const auto writeRet = weaver->write(slotId, KEY, bigValue); - ASSERT_TRUE(writeRet.isOk()); - ASSERT_EQ(writeRet, WeaverStatus::FAILED); -} - -/* - * Reading with a key that is loo large fails - */ -TEST_P(WeaverHidlTest, ReadWithTooLargeKeyFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - std::vector bigKey(config.keySize + 1); - - constexpr uint32_t slotId = 0; - bool callbackCalled = false; - WeaverReadStatus readStatus; - std::vector readValue; - uint32_t timeout; - const auto readRet = - weaver->read(slotId, bigKey, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - readStatus = s; - readValue = r.value; - timeout = r.timeout; - }); - ASSERT_TRUE(callbackCalled); - ASSERT_TRUE(readRet.isOk()); - ASSERT_EQ(readStatus, WeaverReadStatus::FAILED); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(timeout, 0u); -} - -GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(WeaverHidlTest); -INSTANTIATE_TEST_SUITE_P( - PerInstance, WeaverHidlTest, - testing::ValuesIn(android::hardware::getAllHalInstanceNames(IWeaver::descriptor)), - android::hardware::PrintInstanceNameToString); From 42d76ae5dfa9aae45f0f971946ec9e962b21c8b6 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:34:00 +0000 Subject: [PATCH 5/7] Simplify Weaver VTS test - Get the config in SetUp() so that it's easily available to test cases. - Rename "weaver" class member to "weaver_" to match coding style. - Eliminate unnecessary variables when checking WeaverReadResponse. - Fix a typo. Bug: 291284381 Test: 'atest VtsHalWeaverTargetTest' on bramble Change-Id: Ia6dca996103057bfdc9002bc9ab2c039e2333ed9 (cherry picked from commit 961a138e47542b46b574f1f9ba94177e89e06c11) Merged-In: Ia6dca996103057bfdc9002bc9ab2c039e2333ed9 --- weaver/vts/VtsHalWeaverTargetTest.cpp | 171 ++++++++------------------ 1 file changed, 51 insertions(+), 120 deletions(-) diff --git a/weaver/vts/VtsHalWeaverTargetTest.cpp b/weaver/vts/VtsHalWeaverTargetTest.cpp index b75a56f039..047481e123 100644 --- a/weaver/vts/VtsHalWeaverTargetTest.cpp +++ b/weaver/vts/VtsHalWeaverTargetTest.cpp @@ -166,64 +166,56 @@ class WeaverTest : public ::testing::TestWithParam weaver; + std::unique_ptr weaver_; + WeaverConfig config_; }; void WeaverTest::SetUp() { std::string api, instance_name; std::tie(api, instance_name) = GetParam(); if (api == "hidl") { - weaver.reset(new WeaverHidlAdapter(instance_name)); + weaver_.reset(new WeaverHidlAdapter(instance_name)); } else if (api == "aidl") { - weaver.reset(new WeaverAidlAdapter(instance_name)); + weaver_.reset(new WeaverAidlAdapter(instance_name)); } else { FAIL() << "Bad test parameterization"; } - ASSERT_TRUE(weaver->isReady()); + ASSERT_TRUE(weaver_->isReady()); + + auto ret = weaver_->getConfig(&config_); + ASSERT_TRUE(ret.isOk()); + ASSERT_GT(config_.slots, 0); + GTEST_LOG_(INFO) << "WeaverConfig: slots=" << config_.slots << ", keySize=" << config_.keySize + << ", valueSize=" << config_.valueSize; } /* * Checks config values are suitably large */ TEST_P(WeaverTest, GetConfig) { - WeaverConfig config; - - auto ret = weaver->getConfig(&config); - - ASSERT_TRUE(ret.isOk()); - - EXPECT_GE(config.slots, 16u); - EXPECT_GE(config.keySize, 16u); - EXPECT_GE(config.valueSize, 16u); + EXPECT_GE(config_.slots, 16u); + EXPECT_GE(config_.keySize, 16u); + EXPECT_GE(config_.valueSize, 16u); } /* * Gets the config twice and checks they are the same */ TEST_P(WeaverTest, GettingConfigMultipleTimesGivesSameResult) { - WeaverConfig config1; WeaverConfig config2; - auto ret = weaver->getConfig(&config1); + auto ret = weaver_->getConfig(&config2); ASSERT_TRUE(ret.isOk()); - ret = weaver->getConfig(&config2); - ASSERT_TRUE(ret.isOk()); - - EXPECT_EQ(config1, config2); + EXPECT_EQ(config_, config2); } /* * Gets the number of slots from the config and writes a key and value to the last one */ TEST_P(WeaverTest, WriteToLastSlot) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - - ASSERT_TRUE(configRet.isOk()); - - const uint32_t lastSlot = config.slots - 1; - const auto writeRet = weaver->write(lastSlot, KEY, VALUE); + const uint32_t lastSlot = config_.slots - 1; + const auto writeRet = weaver_->write(lastSlot, KEY, VALUE); ASSERT_TRUE(writeRet.isOk()); } @@ -233,23 +225,15 @@ TEST_P(WeaverTest, WriteToLastSlot) { */ TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { constexpr uint32_t slotId = 0; - const auto ret = weaver->write(slotId, KEY, VALUE); + const auto ret = weaver_->write(slotId, KEY, VALUE); ASSERT_TRUE(ret.isOk()); WeaverReadResponse response; - std::vector readValue; - uint32_t timeout; - WeaverReadStatus status; - const auto readRet = weaver->read(slotId, KEY, &response); - - readValue = response.value; - timeout = response.timeout; - status = response.status; - + const auto readRet = weaver_->read(slotId, KEY, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_EQ(readValue, VALUE); - EXPECT_EQ(timeout, 0u); - EXPECT_EQ(status, WeaverReadStatus::OK); + EXPECT_EQ(response.value, VALUE); + EXPECT_EQ(response.timeout, 0u); + EXPECT_EQ(response.status, WeaverReadStatus::OK); } /* @@ -259,26 +243,18 @@ TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { */ TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { constexpr uint32_t slotId = 0; - const auto initialWriteRet = weaver->write(slotId, WRONG_KEY, VALUE); + const auto initialWriteRet = weaver_->write(slotId, WRONG_KEY, VALUE); ASSERT_TRUE(initialWriteRet.isOk()); - const auto overwriteRet = weaver->write(slotId, KEY, OTHER_VALUE); + const auto overwriteRet = weaver_->write(slotId, KEY, OTHER_VALUE); ASSERT_TRUE(overwriteRet.isOk()); WeaverReadResponse response; - std::vector readValue; - uint32_t timeout; - WeaverReadStatus status; - const auto readRet = weaver->read(slotId, KEY, &response); - - readValue = response.value; - timeout = response.timeout; - status = response.status; - + const auto readRet = weaver_->read(slotId, KEY, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_EQ(readValue, OTHER_VALUE); - EXPECT_EQ(timeout, 0u); - EXPECT_EQ(status, WeaverReadStatus::OK); + EXPECT_EQ(response.value, OTHER_VALUE); + EXPECT_EQ(response.timeout, 0u); + EXPECT_EQ(response.status, WeaverReadStatus::OK); } /* @@ -287,37 +263,26 @@ TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { */ TEST_P(WeaverTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { constexpr uint32_t slotId = 0; - const auto ret = weaver->write(slotId, KEY, VALUE); - ASSERT_TRUE(ret.isOk()); + const auto writeRet = weaver_->write(slotId, KEY, VALUE); + ASSERT_TRUE(writeRet.isOk()); WeaverReadResponse response; - std::vector readValue; - WeaverReadStatus status; - const auto readRet = - weaver->read(slotId, WRONG_KEY, &response); - - readValue = response.value; - status = response.status; - + const auto readRet = weaver_->read(slotId, WRONG_KEY, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(status, WeaverReadStatus::INCORRECT_KEY); + EXPECT_TRUE(response.value.empty()); + EXPECT_EQ(response.status, WeaverReadStatus::INCORRECT_KEY); } /* * Writing to an invalid slot fails */ TEST_P(WeaverTest, WritingToInvalidSlotFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - if (config.slots == std::numeric_limits::max()) { + if (config_.slots == std::numeric_limits::max()) { // If there are no invalid slots then pass return; } - const auto writeRet = weaver->write(config.slots, KEY, VALUE); + const auto writeRet = weaver_->write(config_.slots, KEY, VALUE); ASSERT_FALSE(writeRet.isOk()); } @@ -325,44 +290,27 @@ TEST_P(WeaverTest, WritingToInvalidSlotFails) { * Reading from an invalid slot fails rather than incorrect key */ TEST_P(WeaverTest, ReadingFromInvalidSlotFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - if (config.slots == std::numeric_limits::max()) { + if (config_.slots == std::numeric_limits::max()) { // If there are no invalid slots then pass return; } WeaverReadResponse response; - std::vector readValue; - uint32_t timeout; - WeaverReadStatus status; - const auto readRet = - weaver->read(config.slots, KEY, &response); - - readValue = response.value; - timeout = response.timeout; - status = response.status; - + const auto readRet = weaver_->read(config_.slots, KEY, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(timeout, 0u); - EXPECT_EQ(status, WeaverReadStatus::FAILED); + EXPECT_TRUE(response.value.empty()); + EXPECT_EQ(response.timeout, 0u); + EXPECT_EQ(response.status, WeaverReadStatus::FAILED); } /* * Writing a key that is too large fails */ TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - std::vector bigKey(config.keySize + 1); + std::vector bigKey(config_.keySize + 1); constexpr uint32_t slotId = 0; - const auto writeRet = weaver->write(slotId, bigKey, VALUE); + const auto writeRet = weaver_->write(slotId, bigKey, VALUE); ASSERT_FALSE(writeRet.isOk()); } @@ -370,43 +318,26 @@ TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { * Writing a value that is too large fails */ TEST_P(WeaverTest, WriteWithTooLargeValueFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - std::vector bigValue(config.valueSize + 1); + std::vector bigValue(config_.valueSize + 1); constexpr uint32_t slotId = 0; - const auto writeRet = weaver->write(slotId, KEY, bigValue); + const auto writeRet = weaver_->write(slotId, KEY, bigValue); ASSERT_FALSE(writeRet.isOk()); } /* - * Reading with a key that is loo large fails + * Reading with a key that is too large fails */ TEST_P(WeaverTest, ReadWithTooLargeKeyFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - std::vector bigKey(config.keySize + 1); + std::vector bigKey(config_.keySize + 1); constexpr uint32_t slotId = 0; WeaverReadResponse response; - std::vector readValue; - uint32_t timeout; - WeaverReadStatus status; - const auto readRet = - weaver->read(slotId, bigKey, &response); - - readValue = response.value; - timeout = response.timeout; - status = response.status; - + const auto readRet = weaver_->read(slotId, bigKey, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(timeout, 0u); - EXPECT_EQ(status, WeaverReadStatus::FAILED); + EXPECT_TRUE(response.value.empty()); + EXPECT_EQ(response.timeout, 0u); + EXPECT_EQ(response.status, WeaverReadStatus::FAILED); } // Instantiate the test for each HIDL Weaver service. From 86b9048544e01f3af3cd055ae841c007979018d3 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:34:00 +0000 Subject: [PATCH 6/7] Don't overwrite in-use Weaver slots during VTS VtsHalWeaverTargetTest always overwrote the first and last Weaver slots. Before Android 14, apparently this didn't cause problems because Android didn't use Weaver for users that never had an LSKF set. However, now users get a Weaver slot right away. That typically means that the first Weaver slot will be used by user 0. Fix the test to read /metadata/password_slots/slot_map to determine which slots are in use by the system, and only write to unused slots. Bug: 291284381 Test: 'atest -v VtsHalWeaverTargetTest'. Checked for INFO messages showing that slots 1 and 63 were used by the test. Then rebooted and verified that the device can still be unlocked. Change-Id: Id2cce4240d68999471e7d1e8fc7a8449587eed97 (cherry picked from commit 31380e7bc9122147e7a5d65cc1739ab78f853b24) Merged-In: Id2cce4240d68999471e7d1e8fc7a8449587eed97 --- weaver/vts/VtsHalWeaverTargetTest.cpp | 77 ++++++++++++++++++++++----- 1 file changed, 65 insertions(+), 12 deletions(-) diff --git a/weaver/vts/VtsHalWeaverTargetTest.cpp b/weaver/vts/VtsHalWeaverTargetTest.cpp index 047481e123..9ee6d93cf0 100644 --- a/weaver/vts/VtsHalWeaverTargetTest.cpp +++ b/weaver/vts/VtsHalWeaverTargetTest.cpp @@ -17,6 +17,9 @@ #include #include +#include +#include +#include #include #include #include @@ -36,6 +39,7 @@ using HidlWeaverReadStatus = ::android::hardware::weaver::V1_0::WeaverReadStatus using HidlWeaverReadResponse = ::android::hardware::weaver::V1_0::WeaverReadResponse; using HidlWeaverStatus = ::android::hardware::weaver::V1_0::WeaverStatus; +const std::string kSlotMapFile = "/metadata/password_slots/slot_map"; const std::vector KEY{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; const std::vector WRONG_KEY{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; const std::vector VALUE{16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}; @@ -165,9 +169,12 @@ class WeaverTest : public ::testing::TestWithParam weaver_; WeaverConfig config_; + uint32_t first_free_slot_; + uint32_t last_free_slot_; }; void WeaverTest::SetUp() { @@ -187,6 +194,56 @@ void WeaverTest::SetUp() { ASSERT_GT(config_.slots, 0); GTEST_LOG_(INFO) << "WeaverConfig: slots=" << config_.slots << ", keySize=" << config_.keySize << ", valueSize=" << config_.valueSize; + + FindFreeSlots(); + GTEST_LOG_(INFO) << "First free slot is " << first_free_slot_ << ", last free slot is " + << last_free_slot_; +} + +void WeaverTest::FindFreeSlots() { + // Determine which Weaver slots are in use by the system. These slots can't be used by the test. + std::set used_slots; + if (access(kSlotMapFile.c_str(), F_OK) == 0) { + std::string contents; + ASSERT_TRUE(android::base::ReadFileToString(kSlotMapFile, &contents)) + << "Failed to read " << kSlotMapFile; + for (const auto& line : android::base::Split(contents, "\n")) { + auto trimmed_line = android::base::Trim(line); + if (trimmed_line[0] == '#' || trimmed_line[0] == '\0') continue; + auto slot_and_user = android::base::Split(trimmed_line, "="); + uint32_t slot; + ASSERT_TRUE(slot_and_user.size() == 2 && + android::base::ParseUint(slot_and_user[0], &slot)) + << "Error parsing " << kSlotMapFile << " at \"" << line << "\""; + GTEST_LOG_(INFO) << "Slot " << slot << " is in use by " << slot_and_user[1]; + ASSERT_LT(slot, config_.slots); + used_slots.insert(slot); + } + } + // Starting in Android 14, the system will always use at least one Weaver slot if Weaver is + // supported at all. Make sure we saw at least one. + // TODO: uncomment after Android 14 is merged into AOSP + // ASSERT_FALSE(used_slots.empty()) + //<< "Could not determine which Weaver slots are in use by the system"; + + // Find the first free slot. + int found = 0; + for (uint32_t i = 0; i < config_.slots; i++) { + if (used_slots.find(i) == used_slots.end()) { + first_free_slot_ = i; + found++; + break; + } + } + // Find the last free slot. + for (uint32_t i = config_.slots; i > 0; i--) { + if (used_slots.find(i - 1) == used_slots.end()) { + last_free_slot_ = i - 1; + found++; + break; + } + } + ASSERT_EQ(found, 2) << "All Weaver slots are already in use by the system"; } /* @@ -211,11 +268,10 @@ TEST_P(WeaverTest, GettingConfigMultipleTimesGivesSameResult) { } /* - * Gets the number of slots from the config and writes a key and value to the last one + * Writes a key and value to the last free slot */ TEST_P(WeaverTest, WriteToLastSlot) { - const uint32_t lastSlot = config_.slots - 1; - const auto writeRet = weaver_->write(lastSlot, KEY, VALUE); + const auto writeRet = weaver_->write(last_free_slot_, KEY, VALUE); ASSERT_TRUE(writeRet.isOk()); } @@ -224,7 +280,7 @@ TEST_P(WeaverTest, WriteToLastSlot) { * Reads the slot with the same key and receives the value that was previously written */ TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { - constexpr uint32_t slotId = 0; + const uint32_t slotId = first_free_slot_; const auto ret = weaver_->write(slotId, KEY, VALUE); ASSERT_TRUE(ret.isOk()); @@ -242,7 +298,7 @@ TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { * Reads the slot with the new key and receives the new value */ TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { - constexpr uint32_t slotId = 0; + const uint32_t slotId = first_free_slot_; const auto initialWriteRet = weaver_->write(slotId, WRONG_KEY, VALUE); ASSERT_TRUE(initialWriteRet.isOk()); @@ -262,7 +318,7 @@ TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { * Reads the slot with a different key so does not receive the value */ TEST_P(WeaverTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { - constexpr uint32_t slotId = 0; + const uint32_t slotId = first_free_slot_; const auto writeRet = weaver_->write(slotId, KEY, VALUE); ASSERT_TRUE(writeRet.isOk()); @@ -309,8 +365,7 @@ TEST_P(WeaverTest, ReadingFromInvalidSlotFails) { TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { std::vector bigKey(config_.keySize + 1); - constexpr uint32_t slotId = 0; - const auto writeRet = weaver_->write(slotId, bigKey, VALUE); + const auto writeRet = weaver_->write(first_free_slot_, bigKey, VALUE); ASSERT_FALSE(writeRet.isOk()); } @@ -320,8 +375,7 @@ TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { TEST_P(WeaverTest, WriteWithTooLargeValueFails) { std::vector bigValue(config_.valueSize + 1); - constexpr uint32_t slotId = 0; - const auto writeRet = weaver_->write(slotId, KEY, bigValue); + const auto writeRet = weaver_->write(first_free_slot_, KEY, bigValue); ASSERT_FALSE(writeRet.isOk()); } @@ -331,9 +385,8 @@ TEST_P(WeaverTest, WriteWithTooLargeValueFails) { TEST_P(WeaverTest, ReadWithTooLargeKeyFails) { std::vector bigKey(config_.keySize + 1); - constexpr uint32_t slotId = 0; WeaverReadResponse response; - const auto readRet = weaver_->read(slotId, bigKey, &response); + const auto readRet = weaver_->read(first_free_slot_, bigKey, &response); ASSERT_TRUE(readRet.isOk()); EXPECT_TRUE(response.value.empty()); EXPECT_EQ(response.timeout, 0u); From 3d1e0b7555ccbc761536256e71d486a0d748f444 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 24 Jul 2023 17:21:28 +0000 Subject: [PATCH 7/7] Allow uninstantiated WeaverTest Add back GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST, which I had accidentally removed while merging the AIDL and HIDL tests. I think this is still needed on devices that don't support Weaver at all. Bug: 291284381 Test: atest VtsHalWeaverTargetTest Change-Id: Iac1b4476620e51c645e3ad57444ee386cb879029 (cherry picked from commit 47b145a0d8522c41d0aa57e46dbb120bab0590bb) Merged-In: Iac1b4476620e51c645e3ad57444ee386cb879029 --- weaver/vts/VtsHalWeaverTargetTest.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/weaver/vts/VtsHalWeaverTargetTest.cpp b/weaver/vts/VtsHalWeaverTargetTest.cpp index 9ee6d93cf0..754d46710b 100644 --- a/weaver/vts/VtsHalWeaverTargetTest.cpp +++ b/weaver/vts/VtsHalWeaverTargetTest.cpp @@ -393,6 +393,8 @@ TEST_P(WeaverTest, ReadWithTooLargeKeyFails) { EXPECT_EQ(response.status, WeaverReadStatus::FAILED); } +GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(WeaverTest); + // Instantiate the test for each HIDL Weaver service. INSTANTIATE_TEST_SUITE_P( PerHidlInstance, WeaverTest,