diff options
-rw-r--r-- | tools/aapt2/ConfigDescription.cpp | 11 | ||||
-rw-r--r-- | tools/aapt2/DominatorTree_test.cpp | 61 | ||||
-rw-r--r-- | tools/aapt2/optimize/ResourceDeduper_test.cpp | 91 | ||||
-rw-r--r-- | tools/aapt2/test/Common.h | 15 |
4 files changed, 115 insertions, 63 deletions
diff --git a/tools/aapt2/ConfigDescription.cpp b/tools/aapt2/ConfigDescription.cpp index 46098cbc3aa4..7ff0c7227c9c 100644 --- a/tools/aapt2/ConfigDescription.cpp +++ b/tools/aapt2/ConfigDescription.cpp @@ -877,7 +877,16 @@ ConfigDescription ConfigDescription::CopyWithoutSdkVersion() const { } bool ConfigDescription::Dominates(const ConfigDescription& o) const { - if (*this == DefaultConfig() || *this == o) { + if (*this == o) { + return true; + } + + // Locale de-duping is not-trivial, disable for now (b/62409213). + if (diff(o) & CONFIG_LOCALE) { + return false; + } + + if (*this == DefaultConfig()) { return true; } return MatchWithDensity(o) && !o.MatchWithDensity(*this) && diff --git a/tools/aapt2/DominatorTree_test.cpp b/tools/aapt2/DominatorTree_test.cpp index e89c6beb0c57..efc523fcf59b 100644 --- a/tools/aapt2/DominatorTree_test.cpp +++ b/tools/aapt2/DominatorTree_test.cpp @@ -69,14 +69,12 @@ class PrettyPrinter : public DominatorTree::Visitor { TEST(DominatorTreeTest, DefaultDominatesEverything) { const ConfigDescription default_config = {}; const ConfigDescription land_config = test::ParseConfigOrDie("land"); - const ConfigDescription sw600dp_land_config = - test::ParseConfigOrDie("sw600dp-land-v13"); + const ConfigDescription sw600dp_land_config = test::ParseConfigOrDie("sw600dp-land-v13"); std::vector<std::unique_ptr<ResourceConfigValue>> configs; configs.push_back(util::make_unique<ResourceConfigValue>(default_config, "")); configs.push_back(util::make_unique<ResourceConfigValue>(land_config, "")); - configs.push_back( - util::make_unique<ResourceConfigValue>(sw600dp_land_config, "")); + configs.push_back(util::make_unique<ResourceConfigValue>(sw600dp_land_config, "")); DominatorTree tree(configs); PrettyPrinter printer; @@ -91,16 +89,13 @@ TEST(DominatorTreeTest, DefaultDominatesEverything) { TEST(DominatorTreeTest, ProductsAreDominatedSeparately) { const ConfigDescription default_config = {}; const ConfigDescription land_config = test::ParseConfigOrDie("land"); - const ConfigDescription sw600dp_land_config = - test::ParseConfigOrDie("sw600dp-land-v13"); + const ConfigDescription sw600dp_land_config = test::ParseConfigOrDie("sw600dp-land-v13"); std::vector<std::unique_ptr<ResourceConfigValue>> configs; configs.push_back(util::make_unique<ResourceConfigValue>(default_config, "")); configs.push_back(util::make_unique<ResourceConfigValue>(land_config, "")); - configs.push_back( - util::make_unique<ResourceConfigValue>(default_config, "phablet")); - configs.push_back( - util::make_unique<ResourceConfigValue>(sw600dp_land_config, "phablet")); + configs.push_back(util::make_unique<ResourceConfigValue>(default_config, "phablet")); + configs.push_back(util::make_unique<ResourceConfigValue>(sw600dp_land_config, "phablet")); DominatorTree tree(configs); PrettyPrinter printer; @@ -118,16 +113,11 @@ TEST(DominatorTreeTest, MoreSpecificConfigurationsAreDominated) { const ConfigDescription en_config = test::ParseConfigOrDie("en"); const ConfigDescription en_v21_config = test::ParseConfigOrDie("en-v21"); const ConfigDescription ldrtl_config = test::ParseConfigOrDie("ldrtl-v4"); - const ConfigDescription ldrtl_xhdpi_config = - test::ParseConfigOrDie("ldrtl-xhdpi-v4"); - const ConfigDescription sw300dp_config = - test::ParseConfigOrDie("sw300dp-v13"); - const ConfigDescription sw540dp_config = - test::ParseConfigOrDie("sw540dp-v14"); - const ConfigDescription sw600dp_config = - test::ParseConfigOrDie("sw600dp-v14"); - const ConfigDescription sw720dp_config = - test::ParseConfigOrDie("sw720dp-v13"); + const ConfigDescription ldrtl_xhdpi_config = test::ParseConfigOrDie("ldrtl-xhdpi-v4"); + const ConfigDescription sw300dp_config = test::ParseConfigOrDie("sw300dp-v13"); + const ConfigDescription sw540dp_config = test::ParseConfigOrDie("sw540dp-v14"); + const ConfigDescription sw600dp_config = test::ParseConfigOrDie("sw600dp-v14"); + const ConfigDescription sw720dp_config = test::ParseConfigOrDie("sw720dp-v13"); const ConfigDescription v20_config = test::ParseConfigOrDie("v20"); std::vector<std::unique_ptr<ResourceConfigValue>> configs; @@ -135,8 +125,7 @@ TEST(DominatorTreeTest, MoreSpecificConfigurationsAreDominated) { configs.push_back(util::make_unique<ResourceConfigValue>(en_config, "")); configs.push_back(util::make_unique<ResourceConfigValue>(en_v21_config, "")); configs.push_back(util::make_unique<ResourceConfigValue>(ldrtl_config, "")); - configs.push_back( - util::make_unique<ResourceConfigValue>(ldrtl_xhdpi_config, "")); + configs.push_back(util::make_unique<ResourceConfigValue>(ldrtl_xhdpi_config, "")); configs.push_back(util::make_unique<ResourceConfigValue>(sw300dp_config, "")); configs.push_back(util::make_unique<ResourceConfigValue>(sw540dp_config, "")); configs.push_back(util::make_unique<ResourceConfigValue>(sw600dp_config, "")); @@ -148,15 +137,37 @@ TEST(DominatorTreeTest, MoreSpecificConfigurationsAreDominated) { std::string expected = "<default>\n" - " en\n" - " en-v21\n" " ldrtl-v4\n" " ldrtl-xhdpi-v4\n" " sw300dp-v13\n" " sw540dp-v14\n" " sw600dp-v14\n" " sw720dp-v13\n" - " v20\n"; + " v20\n" + "en\n" + " en-v21\n"; + EXPECT_EQ(expected, printer.ToString(&tree)); +} + +TEST(DominatorTreeTest, LocalesAreNeverDominated) { + const ConfigDescription fr_config = test::ParseConfigOrDie("fr"); + const ConfigDescription fr_rCA_config = test::ParseConfigOrDie("fr-rCA"); + const ConfigDescription fr_rFR_config = test::ParseConfigOrDie("fr-rFR"); + + std::vector<std::unique_ptr<ResourceConfigValue>> configs; + configs.push_back(util::make_unique<ResourceConfigValue>(ConfigDescription::DefaultConfig(), "")); + configs.push_back(util::make_unique<ResourceConfigValue>(fr_config, "")); + configs.push_back(util::make_unique<ResourceConfigValue>(fr_rCA_config, "")); + configs.push_back(util::make_unique<ResourceConfigValue>(fr_rFR_config, "")); + + DominatorTree tree(configs); + PrettyPrinter printer; + + std::string expected = + "<default>\n" + "fr\n" + "fr-rCA\n" + "fr-rFR\n"; EXPECT_EQ(expected, printer.ToString(&tree)); } diff --git a/tools/aapt2/optimize/ResourceDeduper_test.cpp b/tools/aapt2/optimize/ResourceDeduper_test.cpp index 4d00fa6efe37..d9f384c095ca 100644 --- a/tools/aapt2/optimize/ResourceDeduper_test.cpp +++ b/tools/aapt2/optimize/ResourceDeduper_test.cpp @@ -19,69 +19,88 @@ #include "ResourceTable.h" #include "test/Test.h" +using ::aapt::test::HasValue; +using ::testing::Not; + namespace aapt { TEST(ResourceDeduperTest, SameValuesAreDeduped) { std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); const ConfigDescription default_config = {}; + const ConfigDescription ldrtl_config = test::ParseConfigOrDie("ldrtl"); + const ConfigDescription ldrtl_v21_config = test::ParseConfigOrDie("ldrtl-v21"); const ConfigDescription en_config = test::ParseConfigOrDie("en"); const ConfigDescription en_v21_config = test::ParseConfigOrDie("en-v21"); - // Chosen because this configuration is compatible with en. + // Chosen because this configuration is compatible with ldrtl/en. const ConfigDescription land_config = test::ParseConfigOrDie("land"); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .AddString("android:string/dedupe", ResourceId{}, default_config, - "dedupe") - .AddString("android:string/dedupe", ResourceId{}, en_config, "dedupe") - .AddString("android:string/dedupe", ResourceId{}, land_config, - "dedupe") - .AddString("android:string/dedupe2", ResourceId{}, default_config, - "dedupe") - .AddString("android:string/dedupe2", ResourceId{}, en_config, - "dedupe") - .AddString("android:string/dedupe2", ResourceId{}, en_v21_config, - "keep") - .AddString("android:string/dedupe2", ResourceId{}, land_config, - "dedupe") + .AddString("android:string/dedupe", ResourceId{}, default_config, "dedupe") + .AddString("android:string/dedupe", ResourceId{}, ldrtl_config, "dedupe") + .AddString("android:string/dedupe", ResourceId{}, land_config, "dedupe") + + .AddString("android:string/dedupe2", ResourceId{}, default_config, "dedupe") + .AddString("android:string/dedupe2", ResourceId{}, ldrtl_config, "dedupe") + .AddString("android:string/dedupe2", ResourceId{}, ldrtl_v21_config, "keep") + .AddString("android:string/dedupe2", ResourceId{}, land_config, "dedupe") + + .AddString("android:string/dedupe3", ResourceId{}, default_config, "dedupe") + .AddString("android:string/dedupe3", ResourceId{}, en_config, "dedupe") + .AddString("android:string/dedupe3", ResourceId{}, en_v21_config, "dedupe") .Build(); ASSERT_TRUE(ResourceDeduper().Consume(context.get(), table.get())); - EXPECT_EQ(nullptr, test::GetValueForConfig<String>( - table.get(), "android:string/dedupe", en_config)); - EXPECT_EQ(nullptr, test::GetValueForConfig<String>( - table.get(), "android:string/dedupe", land_config)); - EXPECT_EQ(nullptr, test::GetValueForConfig<String>( - table.get(), "android:string/dedupe2", en_config)); - EXPECT_NE(nullptr, test::GetValueForConfig<String>( - table.get(), "android:string/dedupe2", en_v21_config)); + EXPECT_THAT(table, Not(HasValue("android:string/dedupe", ldrtl_config))); + EXPECT_THAT(table, Not(HasValue("android:string/dedupe", land_config))); + + EXPECT_THAT(table, HasValue("android:string/dedupe2", ldrtl_v21_config)); + EXPECT_THAT(table, Not(HasValue("android:string/dedupe2", ldrtl_config))); + + EXPECT_THAT(table, HasValue("android:string/dedupe3", default_config)); + EXPECT_THAT(table, HasValue("android:string/dedupe3", en_config)); + EXPECT_THAT(table, Not(HasValue("android:string/dedupe3", en_v21_config))); } TEST(ResourceDeduperTest, DifferentValuesAreKept) { std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); const ConfigDescription default_config = {}; - const ConfigDescription en_config = test::ParseConfigOrDie("en"); - const ConfigDescription en_v21_config = test::ParseConfigOrDie("en-v21"); - // Chosen because this configuration is compatible with en. + const ConfigDescription ldrtl_config = test::ParseConfigOrDie("ldrtl"); + const ConfigDescription ldrtl_v21_config = test::ParseConfigOrDie("ldrtl-v21"); + // Chosen because this configuration is compatible with ldrtl. const ConfigDescription land_config = test::ParseConfigOrDie("land"); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .AddString("android:string/keep", ResourceId{}, default_config, - "keep") - .AddString("android:string/keep", ResourceId{}, en_config, "keep") - .AddString("android:string/keep", ResourceId{}, en_v21_config, - "keep2") + .AddString("android:string/keep", ResourceId{}, default_config, "keep") + .AddString("android:string/keep", ResourceId{}, ldrtl_config, "keep") + .AddString("android:string/keep", ResourceId{}, ldrtl_v21_config, "keep2") .AddString("android:string/keep", ResourceId{}, land_config, "keep2") .Build(); ASSERT_TRUE(ResourceDeduper().Consume(context.get(), table.get())); - EXPECT_NE(nullptr, test::GetValueForConfig<String>( - table.get(), "android:string/keep", en_config)); - EXPECT_NE(nullptr, test::GetValueForConfig<String>( - table.get(), "android:string/keep", en_v21_config)); - EXPECT_NE(nullptr, test::GetValueForConfig<String>( - table.get(), "android:string/keep", land_config)); + EXPECT_THAT(table, HasValue("android:string/keep", ldrtl_config)); + EXPECT_THAT(table, HasValue("android:string/keep", ldrtl_v21_config)); + EXPECT_THAT(table, HasValue("android:string/keep", land_config)); +} + +TEST(ResourceDeduperTest, LocalesValuesAreKept) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); + const ConfigDescription default_config = {}; + const ConfigDescription fr_config = test::ParseConfigOrDie("fr"); + const ConfigDescription fr_rCA_config = test::ParseConfigOrDie("fr-rCA"); + + std::unique_ptr<ResourceTable> table = + test::ResourceTableBuilder() + .AddString("android:string/keep", ResourceId{}, default_config, "keep") + .AddString("android:string/keep", ResourceId{}, fr_config, "keep") + .AddString("android:string/keep", ResourceId{}, fr_rCA_config, "keep") + .Build(); + + ASSERT_TRUE(ResourceDeduper().Consume(context.get(), table.get())); + EXPECT_THAT(table, HasValue("android:string/keep", default_config)); + EXPECT_THAT(table, HasValue("android:string/keep", fr_config)); + EXPECT_THAT(table, HasValue("android:string/keep", fr_rCA_config)); } } // namespace aapt diff --git a/tools/aapt2/test/Common.h b/tools/aapt2/test/Common.h index bc8d274a9c6e..11821031b131 100644 --- a/tools/aapt2/test/Common.h +++ b/tools/aapt2/test/Common.h @@ -141,10 +141,23 @@ MATCHER_P(ValueEq, a, return arg.Equals(&a); } -MATCHER_P(StrValueEq, a, std::string(negation ? "isn't" : "is") + " equal to " + ::testing::PrintToString(a)) { +MATCHER_P(StrValueEq, a, + std::string(negation ? "isn't" : "is") + " equal to " + ::testing::PrintToString(a)) { return *(arg.value) == a; } +MATCHER_P(HasValue, name, + std::string(negation ? "does not have" : "has") + " value " + + ::testing::PrintToString(name)) { + return GetValueForConfig<Value>(&(*arg), name, {}) != nullptr; +} + +MATCHER_P2(HasValue, name, config, + std::string(negation ? "does not have" : "has") + " value " + + ::testing::PrintToString(name) + " for config " + ::testing::PrintToString(config)) { + return GetValueForConfig<Value>(&(*arg), name, config) != nullptr; +} + } // namespace test } // namespace aapt |