From 04d91871df8df89c4f31438e524c65036f979546 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Fri, 2 Aug 2019 17:48:34 -0700 Subject: [PATCH] liblp: Add PropertyFetcher. Use dependency injection so that GetProperty / GetBoolProperty can be mocked in tests. Test: run liblp_test_static Change-Id: I8efa85fbbd7aebce2541f748f840e512f3729c30 --- fs_mgr/liblp/Android.bp | 1 + fs_mgr/liblp/builder.cpp | 24 ++------- fs_mgr/liblp/builder_test.cpp | 34 +++++++------ fs_mgr/liblp/include/liblp/builder.h | 9 ---- fs_mgr/liblp/include/liblp/property_fetcher.h | 42 ++++++++++++++++ fs_mgr/liblp/io_test.cpp | 10 +++- fs_mgr/liblp/mock_property_fetcher.h | 47 ++++++++++++++++++ fs_mgr/liblp/property_fetcher.cpp | 49 +++++++++++++++++++ 8 files changed, 171 insertions(+), 45 deletions(-) create mode 100644 fs_mgr/liblp/include/liblp/property_fetcher.h create mode 100644 fs_mgr/liblp/mock_property_fetcher.h create mode 100644 fs_mgr/liblp/property_fetcher.cpp diff --git a/fs_mgr/liblp/Android.bp b/fs_mgr/liblp/Android.bp index b5041613a..5b377ae1e 100644 --- a/fs_mgr/liblp/Android.bp +++ b/fs_mgr/liblp/Android.bp @@ -36,6 +36,7 @@ cc_library { "builder.cpp", "images.cpp", "partition_opener.cpp", + "property_fetcher.cpp", "reader.cpp", "utility.cpp", "writer.cpp", diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp index a434c936a..dab3d553c 100644 --- a/fs_mgr/liblp/builder.cpp +++ b/fs_mgr/liblp/builder.cpp @@ -21,19 +21,16 @@ #include #include -#include #include #include "liblp/liblp.h" +#include "liblp/property_fetcher.h" #include "reader.h" #include "utility.h" namespace android { namespace fs_mgr { -std::optional MetadataBuilder::sABOverride; -std::optional MetadataBuilder::sRetrofitDap; - static constexpr std::string_view kDefaultGroup = "default"; bool LinearExtent::AddTo(LpMetadata* out) const { @@ -211,14 +208,6 @@ std::unique_ptr MetadataBuilder::NewForUpdate(const IPartitionO return New(*metadata.get(), &opener); } -void MetadataBuilder::OverrideABForTesting(bool ab_device) { - sABOverride = ab_device; -} - -void MetadataBuilder::OverrideRetrofitDynamicParititonsForTesting(bool retrofit) { - sRetrofitDap = retrofit; -} - MetadataBuilder::MetadataBuilder() : auto_slot_suffixing_(false) { memset(&geometry_, 0, sizeof(geometry_)); geometry_.magic = LP_METADATA_GEOMETRY_MAGIC; @@ -1051,17 +1040,12 @@ void MetadataBuilder::SetAutoSlotSuffixing() { } bool MetadataBuilder::IsABDevice() { - if (sABOverride.has_value()) { - return *sABOverride; - } - return !android::base::GetProperty("ro.boot.slot_suffix", "").empty(); + return !IPropertyFetcher::GetInstance()->GetProperty("ro.boot.slot_suffix", "").empty(); } bool MetadataBuilder::IsRetrofitDynamicPartitionsDevice() { - if (sRetrofitDap.has_value()) { - return *sRetrofitDap; - } - return android::base::GetBoolProperty("ro.boot.dynamic_partitions_retrofit", false); + return IPropertyFetcher::GetInstance()->GetBoolProperty("ro.boot.dynamic_partitions_retrofit", + false); } bool MetadataBuilder::IsRetrofitMetadata() const { diff --git a/fs_mgr/liblp/builder_test.cpp b/fs_mgr/liblp/builder_test.cpp index 377ec685b..6d2787307 100644 --- a/fs_mgr/liblp/builder_test.cpp +++ b/fs_mgr/liblp/builder_test.cpp @@ -19,36 +19,40 @@ #include #include +#include "mock_property_fetcher.h" #include "utility.h" using namespace std; using namespace android::fs_mgr; +using ::android::fs_mgr::MockPropertyFetcher; +using ::testing::_; +using ::testing::AnyNumber; using ::testing::ElementsAre; +using ::testing::NiceMock; +using ::testing::Return; + +static void ResetPropertyFetcher() { + IPropertyFetcher::OverrideForTesting(std::make_unique>()); +} + +MockPropertyFetcher* GetMockedInstance() { + return static_cast(IPropertyFetcher::GetInstance()); +} class Environment : public ::testing::Environment { public: - void SetUp() override { - MetadataBuilder::OverrideABForTesting(false); - MetadataBuilder::OverrideRetrofitDynamicParititonsForTesting(false); - } + void SetUp() override { ResetPropertyFetcher(); } }; int main(int argc, char** argv) { - ::testing::AddGlobalTestEnvironment(new Environment); ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } class BuilderTest : public ::testing::Test { public: - void SetUp() override { - MetadataBuilder::OverrideABForTesting(false); - MetadataBuilder::OverrideRetrofitDynamicParititonsForTesting(false); - } - void TearDown() override { - MetadataBuilder::OverrideABForTesting(false); - MetadataBuilder::OverrideRetrofitDynamicParititonsForTesting(false); - } + void SetUp() override { ResetPropertyFetcher(); } + void TearDown() override { ResetPropertyFetcher(); } }; TEST_F(BuilderTest, BuildBasic) { @@ -785,7 +789,9 @@ TEST_F(BuilderTest, ABExtents) { // A and B slots should be allocated from separate halves of the partition, // to mitigate allocating too many extents. (b/120433288) - MetadataBuilder::OverrideABForTesting(true); + ON_CALL(*GetMockedInstance(), GetProperty("ro.boot.slot_suffix", _)) + .WillByDefault(Return("_a")); + auto builder = MetadataBuilder::New(device_info, 65536, 2); ASSERT_NE(builder, nullptr); Partition* system_a = builder->AddPartition("system_a", 0); diff --git a/fs_mgr/liblp/include/liblp/builder.h b/fs_mgr/liblp/include/liblp/builder.h index a2221ef5c..1a2624082 100644 --- a/fs_mgr/liblp/include/liblp/builder.h +++ b/fs_mgr/liblp/include/liblp/builder.h @@ -196,12 +196,6 @@ class MetadataBuilder { return New(device_info, metadata_max_size, metadata_slot_count); } - // Used by the test harness to override whether the device is "A/B". - static void OverrideABForTesting(bool ab_device); - - // Used by the test harness to override whether the device is "retrofitting dynamic partitions". - static void OverrideRetrofitDynamicParititonsForTesting(bool retrofit); - // Define a new partition group. By default there is one group called // "default", with an unrestricted size. A non-zero size will restrict the // total space used by all partitions in the group. @@ -347,9 +341,6 @@ class MetadataBuilder { const std::vector& free_list, uint64_t sectors_needed) const; - static std::optional sABOverride; - static std::optional sRetrofitDap; - LpMetadataGeometry geometry_; LpMetadataHeader header_; std::vector> partitions_; diff --git a/fs_mgr/liblp/include/liblp/property_fetcher.h b/fs_mgr/liblp/include/liblp/property_fetcher.h new file mode 100644 index 000000000..e73a1f562 --- /dev/null +++ b/fs_mgr/liblp/include/liblp/property_fetcher.h @@ -0,0 +1,42 @@ +// +// Copyright (C) 2019 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. +// + +#pragma once + +#include + +namespace android { +namespace fs_mgr { + +class IPropertyFetcher { + public: + virtual ~IPropertyFetcher() = default; + virtual std::string GetProperty(const std::string& key, const std::string& defaultValue) = 0; + virtual bool GetBoolProperty(const std::string& key, bool defaultValue) = 0; + + static IPropertyFetcher* GetInstance(); + static void OverrideForTesting(std::unique_ptr&&); +}; + +class PropertyFetcher : public IPropertyFetcher { + public: + ~PropertyFetcher() = default; + std::string GetProperty(const std::string& key, const std::string& defaultValue) override; + bool GetBoolProperty(const std::string& key, bool defaultValue) override; +}; + +} // namespace fs_mgr +} // namespace android diff --git a/fs_mgr/liblp/io_test.cpp b/fs_mgr/liblp/io_test.cpp index 70dd85f90..299086384 100644 --- a/fs_mgr/liblp/io_test.cpp +++ b/fs_mgr/liblp/io_test.cpp @@ -23,10 +23,12 @@ #include #include #include +#include #include #include #include "images.h" +#include "mock_property_fetcher.h" #include "reader.h" #include "test_partition_opener.h" #include "utility.h" @@ -34,6 +36,8 @@ using namespace std; using namespace android::fs_mgr; +using ::testing::_; +using ::testing::Return; using unique_fd = android::base::unique_fd; // Our tests assume a 128KiB disk with two 512 byte metadata slots. @@ -664,7 +668,8 @@ TEST(liblp, AutoSlotSuffixing) { } TEST(liblp, UpdateRetrofit) { - MetadataBuilder::OverrideRetrofitDynamicParititonsForTesting(true); + ON_CALL(*GetMockedInstance(), GetBoolProperty("ro.boot.dynamic_partitions_retrofit", _)) + .WillByDefault(Return(true)); unique_ptr builder = CreateDefaultBuilder(); ASSERT_NE(builder, nullptr); @@ -695,7 +700,8 @@ TEST(liblp, UpdateRetrofit) { } TEST(liblp, UpdateNonRetrofit) { - MetadataBuilder::OverrideRetrofitDynamicParititonsForTesting(false); + ON_CALL(*GetMockedInstance(), GetBoolProperty("ro.boot.dynamic_partitions_retrofit", _)) + .WillByDefault(Return(false)); unique_fd fd = CreateFlashedDisk(); ASSERT_GE(fd, 0); diff --git a/fs_mgr/liblp/mock_property_fetcher.h b/fs_mgr/liblp/mock_property_fetcher.h new file mode 100644 index 000000000..eb91de2fe --- /dev/null +++ b/fs_mgr/liblp/mock_property_fetcher.h @@ -0,0 +1,47 @@ +/* + * Copyright (C) 2019 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. + */ + +#pragma once + +#include + +#include + +namespace android { +namespace fs_mgr { + +class MockPropertyFetcher : public IPropertyFetcher { + public: + MOCK_METHOD2(GetProperty, std::string(const std::string&, const std::string&)); + MOCK_METHOD2(GetBoolProperty, bool(const std::string&, bool)); + + // By default, return default_value for all functions. + MockPropertyFetcher() { + using ::testing::_; + using ::testing::Invoke; + ON_CALL(*this, GetProperty(_, _)).WillByDefault(Invoke([](const auto&, const auto& def) { + return def; + })); + ON_CALL(*this, GetBoolProperty(_, _)).WillByDefault(Invoke([](const auto&, auto def) { + return def; + })); + } +}; + +} // namespace fs_mgr +} // namespace android + +android::fs_mgr::MockPropertyFetcher* GetMockedInstance(); diff --git a/fs_mgr/liblp/property_fetcher.cpp b/fs_mgr/liblp/property_fetcher.cpp new file mode 100644 index 000000000..038ef4dc4 --- /dev/null +++ b/fs_mgr/liblp/property_fetcher.cpp @@ -0,0 +1,49 @@ +/* + * Copyright (C) 2019 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 "liblp/property_fetcher.h" + +#include + +#include + +namespace android { +namespace fs_mgr { + +std::string PropertyFetcher::GetProperty(const std::string& key, const std::string& default_value) { + return android::base::GetProperty(key, default_value); +} + +bool PropertyFetcher::GetBoolProperty(const std::string& key, bool default_value) { + return android::base::GetBoolProperty(key, default_value); +} + +static std::unique_ptr* GetInstanceAllocation() { + static std::unique_ptr instance = std::make_unique(); + return &instance; +} + +IPropertyFetcher* IPropertyFetcher::GetInstance() { + return GetInstanceAllocation()->get(); +} + +void IPropertyFetcher::OverrideForTesting(std::unique_ptr&& fetcher) { + GetInstanceAllocation()->swap(fetcher); + fetcher.reset(); +} + +} // namespace fs_mgr +} // namespace android