split-select: Fix rules generated for anydpi density

Change-Id: I9de569ca9a76eb22df4d0e178df847ba1c7d0b01

12 files changed, 350 insertions, 245 deletions

diff --git a/tools/aapt/AaptConfig.cpp b/tools/aapt/AaptConfig.cpp
index 848d9a1..e88c27a 100644
--- a/tools/aapt/AaptConfig.cpp
+++ b/tools/aapt/AaptConfig.cpp
@@ -21,6 +21,7 @@
 #include "AaptAssets.h"
 #include "AaptUtil.h"
 #include "ResourceFilter.h"
+#include "SdkConstants.h"
 
 using android::String8;
 using android::Vector;
@@ -241,7 +242,7 @@ void applyVersionForCompatibility(ConfigDescription* config) {
 
     uint16_t minSdk = 0;
     if (config->density == ResTable_config::DENSITY_ANY) {
-        minSdk = SDK_L;
+        minSdk = SDK_LOLLIPOP;
     } else if (config->smallestScreenWidthDp != ResTable_config::SCREENWIDTH_ANY
             || config->screenWidthDp != ResTable_config::SCREENWIDTH_ANY
             || config->screenHeightDp != ResTable_config::SCREENHEIGHT_ANY) {
@@ -800,7 +801,7 @@ bool isDensityOnly(const ResTable_config& config) {
     }
 
     if (config.density == ResTable_config::DENSITY_ANY) {
-        if (config.sdkVersion != SDK_L) {
+        if (config.sdkVersion != SDK_LOLLIPOP) {
             // Someone modified the sdkVersion from the default, this is not safe to assume.
             return false;
         }
diff --git a/tools/aapt/Bundle.h b/tools/aapt/Bundle.h
index cb34448..0e130f4 100644
--- a/tools/aapt/Bundle.h
+++ b/tools/aapt/Bundle.h
@@ -14,18 +14,7 @@
 #include <utils/String8.h>
 #include <utils/Vector.h>
 
-enum {
-    SDK_CUPCAKE = 3,
-    SDK_DONUT = 4,
-    SDK_ECLAIR = 5,
-    SDK_ECLAIR_0_1 = 6,
-    SDK_MR1 = 7,
-    SDK_FROYO = 8,
-    SDK_HONEYCOMB_MR2 = 13,
-    SDK_ICE_CREAM_SANDWICH = 14,
-    SDK_ICE_CREAM_SANDWICH_MR1 = 15,
-    SDK_L = 21,
-};
+#include "SdkConstants.h"
 
 /*
  * Things we can do.
diff --git a/tools/aapt/ResourceTable.cpp b/tools/aapt/ResourceTable.cpp
index beff604..bdc6586 100644
--- a/tools/aapt/ResourceTable.cpp
+++ b/tools/aapt/ResourceTable.cpp
@@ -10,6 +10,7 @@
 #include "XMLNode.h"
 #include "ResourceFilter.h"
 #include "ResourceIdCache.h"
+#include "SdkConstants.h"
 
 #include <androidfw/ResourceTypes.h>
 #include <utils/ByteOrder.h>
@@ -4223,7 +4224,7 @@ static bool isMinSdkVersionLOrAbove(const Bundle* bundle) {
         }
 
         const int minSdk = atoi(bundle->getMinSdkVersion());
-        if (minSdk >= SDK_L) {
+        if (minSdk >= SDK_LOLLIPOP) {
             return true;
         }
     }
@@ -4314,7 +4315,7 @@ status_t ResourceTable::modifyForCompat(const Bundle* bundle) {
                     }
 
                     const ConfigDescription& config = entries.keyAt(ei);
-                    if (config.sdkVersion >= SDK_L) {
+                    if (config.sdkVersion >= SDK_LOLLIPOP) {
                         // We don't need to do anything if the resource is
                         // already qualified for version 21 or higher.
                         continue;
@@ -4336,9 +4337,9 @@ status_t ResourceTable::modifyForCompat(const Bundle* bundle) {
                     }
 
                     // Duplicate the entry under the same configuration
-                    // but with sdkVersion == SDK_L.
+                    // but with sdkVersion == SDK_LOLLIPOP.
                     ConfigDescription newConfig(config);
-                    newConfig.sdkVersion = SDK_L;
+                    newConfig.sdkVersion = SDK_LOLLIPOP;
                     entriesToAdd.add(key_value_pair_t<ConfigDescription, sp<Entry> >(
                             newConfig, new Entry(*e)));
 
@@ -4361,7 +4362,7 @@ status_t ResourceTable::modifyForCompat(const Bundle* bundle) {
                     if (bundle->getVerbose()) {
                         entriesToAdd[i].value->getPos()
                                 .printf("using v%d attributes; synthesizing resource %s:%s/%s for configuration %s.",
-                                        SDK_L,
+                                        SDK_LOLLIPOP,
                                         String8(p->getName()).string(),
                                         String8(t->getName()).string(),
                                         String8(entriesToAdd[i].value->getName()).string(),
@@ -4388,7 +4389,7 @@ status_t ResourceTable::modifyForCompat(const Bundle* bundle,
         return NO_ERROR;
     }
 
-    if (target->getResourceType() == "" || target->getGroupEntry().toParams().sdkVersion >= SDK_L) {
+    if (target->getResourceType() == "" || target->getGroupEntry().toParams().sdkVersion >= SDK_LOLLIPOP) {
         // Skip resources that have no type (AndroidManifest.xml) or are already version qualified with v21
         // or higher.
         return NO_ERROR;
@@ -4424,7 +4425,7 @@ status_t ResourceTable::modifyForCompat(const Bundle* bundle,
     }
 
     ConfigDescription newConfig(target->getGroupEntry().toParams());
-    newConfig.sdkVersion = SDK_L;
+    newConfig.sdkVersion = SDK_LOLLIPOP;
 
     // Look to see if we already have an overriding v21 configuration.
     sp<ConfigList> cl = getConfigList(String16(mAssets->getPackage()),
@@ -4446,7 +4447,7 @@ status_t ResourceTable::modifyForCompat(const Bundle* bundle,
         if (bundle->getVerbose()) {
             SourcePos(target->getSourceFile(), -1).printf(
                     "using v%d attributes; synthesizing resource %s:%s/%s for configuration %s.",
-                    SDK_L,
+                    SDK_LOLLIPOP,
                     mAssets->getPackage().string(),
                     newFile->getResourceType().string(),
                     String8(resourceName).string(),
diff --git a/tools/aapt/SdkConstants.h b/tools/aapt/SdkConstants.h
new file mode 100644
index 0000000..7fd1030
--- /dev/null
+++ b/tools/aapt/SdkConstants.h
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2014 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.
+ */
+
+#ifndef H_AAPT_SDK_CONSTANTS
+#define H_AAPT_SDK_CONSTANTS
+
+enum {
+    SDK_CUPCAKE = 3,
+    SDK_DONUT = 4,
+    SDK_ECLAIR = 5,
+    SDK_ECLAIR_0_1 = 6,
+    SDK_ECLAIR_MR1 = 7,
+    SDK_FROYO = 8,
+    SDK_GINGERBREAD = 9,
+    SDK_GINGERBREAD_MR1 = 10,
+    SDK_HONEYCOMB = 11,
+    SDK_HONEYCOMB_MR1 = 12,
+    SDK_HONEYCOMB_MR2 = 13,
+    SDK_ICE_CREAM_SANDWICH = 14,
+    SDK_ICE_CREAM_SANDWICH_MR1 = 15,
+    SDK_JELLY_BEAN = 16,
+    SDK_JELLY_BEAN_MR1 = 17,
+    SDK_JELLY_BEAN_MR2 = 18,
+    SDK_KITKAT = 19,
+    SDK_KITKAT_WATCH = 20,
+    SDK_LOLLIPOP = 21,
+};
+
+#endif // H_AAPT_SDK_CONSTANTS
diff --git a/tools/split-select/Android.mk b/tools/split-select/Android.mk
index dc48ea8..968d22b 100644
--- a/tools/split-select/Android.mk
+++ b/tools/split-select/Android.mk
@@ -34,7 +34,8 @@ sources := \
 testSources := \
     Grouper_test.cpp \
     Rule_test.cpp \
-    RuleGenerator_test.cpp
+    RuleGenerator_test.cpp \
+    TestRules.cpp
 
 cIncludes := \
     external/zlib \
diff --git a/tools/split-select/Rule.cpp b/tools/split-select/Rule.cpp
index 9559fe2..48d21ff 100644
--- a/tools/split-select/Rule.cpp
+++ b/tools/split-select/Rule.cpp
@@ -29,6 +29,16 @@ inline static void indentStr(String8& str, int indent) {
     }
 }
 
+Rule::Rule(const Rule& rhs)
+    : RefBase()
+    , op(rhs.op)
+    , key(rhs.key)
+    , negate(rhs.negate)
+    , stringArgs(rhs.stringArgs)
+    , longArgs(rhs.longArgs)
+    , subrules(rhs.subrules) {
+}
+
 String8 Rule::toJson(int indent) const {
     String8 str;
     indentStr(str, indent);
diff --git a/tools/split-select/Rule.h b/tools/split-select/Rule.h
index 8029931..08a2075 100644
--- a/tools/split-select/Rule.h
+++ b/tools/split-select/Rule.h
@@ -28,6 +28,7 @@ namespace split {
 
 struct Rule : public virtual android::RefBase {
     inline Rule();
+    Rule(const Rule& rhs);
 
     enum Operator {
         LESS_THAN = 1,
diff --git a/tools/split-select/RuleGenerator.cpp b/tools/split-select/RuleGenerator.cpp
index b8f3bcb..72bb0c7 100644
--- a/tools/split-select/RuleGenerator.cpp
+++ b/tools/split-select/RuleGenerator.cpp
@@ -15,6 +15,7 @@
  */
 
 #include "RuleGenerator.h"
+#include "aapt/SdkConstants.h"
 
 #include <algorithm>
 #include <cmath>
@@ -32,18 +33,21 @@ static inline int findMid(int l, int h) {
 }
 
 sp<Rule> RuleGenerator::generateDensity(const Vector<int>& allDensities, size_t index) {
-    sp<Rule> densityRule = new Rule();
-    densityRule->op = Rule::AND_SUBRULES;
-
-    const bool anyDensity = allDensities[index] == ResTable_config::DENSITY_ANY;
-    sp<Rule> any = new Rule();
-    any->op = Rule::EQUALS;
-    any->key = Rule::SCREEN_DENSITY;
-    any->longArgs.add((int)ResTable_config::DENSITY_ANY);
-    any->negate = !anyDensity;
-    densityRule->subrules.add(any);
-
-    if (!anyDensity) {
+    if (allDensities[index] != ResTable_config::DENSITY_ANY) {
+        sp<Rule> densityRule = new Rule();
+        densityRule->op = Rule::AND_SUBRULES;
+
+        const bool hasAnyDensity = std::find(allDensities.begin(),
+                allDensities.end(), ResTable_config::DENSITY_ANY) != allDensities.end();
+
+        if (hasAnyDensity) {
+            sp<Rule> version = new Rule();
+            version->op = Rule::LESS_THAN;
+            version->key = Rule::SDK_VERSION;
+            version->longArgs.add((long) SDK_LOLLIPOP);
+            densityRule->subrules.add(version);
+        }
+
         if (index > 0) {
             sp<Rule> gt = new Rule();
             gt->op = Rule::GREATER_THAN;
@@ -59,8 +63,14 @@ sp<Rule> RuleGenerator::generateDensity(const Vector<int>& allDensities, size_t
             lt->longArgs.add(findMid(allDensities[index], allDensities[index + 1]));
             densityRule->subrules.add(lt);
         }
+        return densityRule;
+    } else {
+        // SDK_VERSION is handled elsewhere, so we always pick DENSITY_ANY if it's
+        // available.
+        sp<Rule> always = new Rule();
+        always->op = Rule::ALWAYS_TRUE;
+        return always;
     }
-    return densityRule;
 }
 
 sp<Rule> RuleGenerator::generateAbi(const Vector<abi::Variant>& splitAbis, size_t index) {
diff --git a/tools/split-select/RuleGenerator_test.cpp b/tools/split-select/RuleGenerator_test.cpp
index ee387be..778d604 100644
--- a/tools/split-select/RuleGenerator_test.cpp
+++ b/tools/split-select/RuleGenerator_test.cpp
@@ -16,154 +16,95 @@
 
 #include "RuleGenerator.h"
 
-#include <algorithm>
+#include "aapt/SdkConstants.h"
+#include "TestRules.h"
+
 #include <gtest/gtest.h>
-#include <utils/String8.h>
+#include <utils/Vector.h>
 
 using namespace android;
+using namespace split::test;
 
 namespace split {
 
-static void expectDensityRule(const Vector<int>& densities, int density, int greaterThan, int lessThan);
-static void expectAbiRule(const Vector<abi::Variant>& abis, abi::Variant variant, const char* a);
-static void expectAbiRule(const Vector<abi::Variant>& abis, abi::Variant variant, const char* a, const char* b);
-
 TEST(RuleGeneratorTest, testAbiRules) {
     Vector<abi::Variant> abis;
-    abis.add(abi::Variant_armeabi);
-    abis.add(abi::Variant_armeabi_v7a);
-    abis.add(abi::Variant_x86);
-    std::sort(abis.begin(), abis.end());
-
-    expectAbiRule(abis, abi::Variant_armeabi, "armeabi");
-    expectAbiRule(abis, abi::Variant_armeabi_v7a, "armeabi-v7a", "arm64-v8a");
-    expectAbiRule(abis, abi::Variant_x86, "x86", "x86_64");
-}
+    const ssize_t armeabiIndex = abis.add(abi::Variant_armeabi);
+    const ssize_t armeabi_v7aIndex = abis.add(abi::Variant_armeabi_v7a);
+    const ssize_t x86Index = abis.add(abi::Variant_x86);
 
-TEST(RuleGeneratorTest, testDensityRules) {
-    Vector<int> densities;
-    densities.add(ConfigDescription::DENSITY_HIGH);
-    densities.add(ConfigDescription::DENSITY_XHIGH);
-    densities.add(ConfigDescription::DENSITY_XXHIGH);
-    densities.add(ConfigDescription::DENSITY_ANY);
-
-    ASSERT_LT(263, ConfigDescription::DENSITY_XHIGH);
-    ASSERT_GT(262, ConfigDescription::DENSITY_HIGH);
-    ASSERT_LT(363, ConfigDescription::DENSITY_XXHIGH);
-    ASSERT_GT(362, ConfigDescription::DENSITY_XHIGH);
-
-    expectDensityRule(densities, ConfigDescription::DENSITY_HIGH, 0, 263);
-    expectDensityRule(densities, ConfigDescription::DENSITY_XHIGH, 262, 363);
-    expectDensityRule(densities, ConfigDescription::DENSITY_XXHIGH, 362, 0);
-    expectDensityRule(densities, ConfigDescription::DENSITY_ANY, 0, 0);
-}
+    EXPECT_RULES_EQ(RuleGenerator::generateAbi(abis, armeabiIndex),
+            ContainsAnyRule(Rule::NATIVE_PLATFORM, "armeabi")
+    );
+
+    EXPECT_RULES_EQ(RuleGenerator::generateAbi(abis, armeabi_v7aIndex),
+            ContainsAnyRule(Rule::NATIVE_PLATFORM, "armeabi-v7a", "arm64-v8a")
+    );
 
-//
-// Helper methods.
-//
-
-static void expectDensityRule(const Vector<int>& densities, int density, int greaterThan, int lessThan) {
-    const int* iter = std::find(densities.begin(), densities.end(), density);
-    if (densities.end() == iter) {
-        ADD_FAILURE() << density << "dpi was not in the density list.";
-        return;
-    }
-
-    sp<Rule> rule = RuleGenerator::generateDensity(densities, iter - densities.begin());
-    if (rule->op != Rule::AND_SUBRULES) {
-        ADD_FAILURE() << "Op in rule for " << density << "dpi is not Rule::AND_SUBRULES.";
-        return;
-    }
-
-    size_t index = 0;
-
-    bool isAnyDpi = density == ConfigDescription::DENSITY_ANY;
-
-    sp<Rule> anyDpiRule = rule->subrules[index++];
-    EXPECT_EQ(Rule::EQUALS, anyDpiRule->op)
-            << "for " << density << "dpi ANY DPI rule";
-    EXPECT_EQ(Rule::SCREEN_DENSITY, anyDpiRule->key)
-            << "for " << density << "dpi ANY DPI rule";
-    EXPECT_EQ(isAnyDpi == false, anyDpiRule->negate)
-            << "for " << density << "dpi ANY DPI rule";
-    if (anyDpiRule->longArgs.size() == 1) {
-        EXPECT_EQ(ConfigDescription::DENSITY_ANY, anyDpiRule->longArgs[0])
-            << "for " << density << "dpi ANY DPI rule";
-    } else {
-        EXPECT_EQ(1u, anyDpiRule->longArgs.size())
-            << "for " << density << "dpi ANY DPI rule";
-    }
-
-
-    if (greaterThan != 0) {
-        sp<Rule> greaterThanRule = rule->subrules[index++];
-        EXPECT_EQ(Rule::GREATER_THAN, greaterThanRule->op)
-                << "for " << density << "dpi GREATER_THAN rule";
-        EXPECT_EQ(Rule::SCREEN_DENSITY, greaterThanRule->key)
-                << "for " << density << "dpi GREATER_THAN rule";
-        if (greaterThanRule->longArgs.size() == 1) {
-            EXPECT_EQ(greaterThan, greaterThanRule->longArgs[0])
-                << "for " << density << "dpi GREATER_THAN rule";
-        } else {
-            EXPECT_EQ(1u, greaterThanRule->longArgs.size())
-                << "for " << density << "dpi GREATER_THAN rule";
-        }
-    }
-
-    if (lessThan != 0) {
-        sp<Rule> lessThanRule = rule->subrules[index++];
-        EXPECT_EQ(Rule::LESS_THAN, lessThanRule->op)
-                << "for " << density << "dpi LESS_THAN rule";
-        EXPECT_EQ(Rule::SCREEN_DENSITY, lessThanRule->key)
-                << "for " << density << "dpi LESS_THAN rule";
-        if (lessThanRule->longArgs.size() == 1) {
-            EXPECT_EQ(lessThan, lessThanRule->longArgs[0])
-                << "for " << density << "dpi LESS_THAN rule";
-        } else {
-            EXPECT_EQ(1u, lessThanRule->longArgs.size())
-                << "for " << density << "dpi LESS_THAN rule";
-        }
-    }
+    EXPECT_RULES_EQ(RuleGenerator::generateAbi(abis, x86Index),
+            ContainsAnyRule(Rule::NATIVE_PLATFORM, "x86", "x86_64")
+    );
 }
 
-static void expectAbiRule(const Vector<abi::Variant>& abis, abi::Variant variant, const Vector<const char*>& matches) {
-    const abi::Variant* iter = std::find(abis.begin(), abis.end(), variant);
-    if (abis.end() == iter) {
-        ADD_FAILURE() << abi::toString(variant) << " was not in the abi list.";
-        return;
-    }
-
-    sp<Rule> rule = RuleGenerator::generateAbi(abis, iter - abis.begin());
-
-    EXPECT_EQ(Rule::CONTAINS_ANY, rule->op)
-            << "for " << abi::toString(variant) << " rule";
-    EXPECT_EQ(Rule::NATIVE_PLATFORM, rule->key)
-            << " for " << abi::toString(variant) << " rule";
-    EXPECT_EQ(matches.size(), rule->stringArgs.size())
-            << " for " << abi::toString(variant) << " rule";
-
-    const size_t matchCount = matches.size();
-    for (size_t i = 0; i < matchCount; i++) {
-        const char* match = matches[i];
-        if (rule->stringArgs.end() ==
-                std::find(rule->stringArgs.begin(), rule->stringArgs.end(), String8(match))) {
-            ADD_FAILURE() << "Rule for abi " << abi::toString(variant)
-                    << " does not contain match for expected abi " << match;
-        }
-    }
+TEST(RuleGeneratorTest, densityConstantsAreSane) {
+    EXPECT_LT(263, ConfigDescription::DENSITY_XHIGH);
+    EXPECT_GT(262, ConfigDescription::DENSITY_HIGH);
+    EXPECT_LT(363, ConfigDescription::DENSITY_XXHIGH);
+    EXPECT_GT(362, ConfigDescription::DENSITY_XHIGH);
 }
 
-static void expectAbiRule(const Vector<abi::Variant>& abis, abi::Variant variant, const char* a) {
-    Vector<const char*> matches;
-    matches.add(a);
-    expectAbiRule(abis, variant, matches);
+TEST(RuleGeneratorTest, testDensityRules) {
+    Vector<int> densities;
+    const ssize_t highIndex = densities.add(ConfigDescription::DENSITY_HIGH);
+    const ssize_t xhighIndex = densities.add(ConfigDescription::DENSITY_XHIGH);
+    const ssize_t xxhighIndex = densities.add(ConfigDescription::DENSITY_XXHIGH);
+
+    EXPECT_RULES_EQ(RuleGenerator::generateDensity(densities, highIndex),
+            AndRule()
+            .add(LtRule(Rule::SCREEN_DENSITY, 263))
+    );
+
+    EXPECT_RULES_EQ(RuleGenerator::generateDensity(densities, xhighIndex),
+            AndRule()
+            .add(GtRule(Rule::SCREEN_DENSITY, 262))
+            .add(LtRule(Rule::SCREEN_DENSITY, 363))
+    );
+
+    EXPECT_RULES_EQ(RuleGenerator::generateDensity(densities, xxhighIndex),
+            AndRule()
+            .add(GtRule(Rule::SCREEN_DENSITY, 362))
+    );
 }
 
-static void expectAbiRule(const Vector<abi::Variant>& abis, abi::Variant variant, const char* a, const char* b) {
-    Vector<const char*> matches;
-    matches.add(a);
-    matches.add(b);
-    expectAbiRule(abis, variant, matches);
+TEST(RuleGeneratorTest, testDensityRulesWithAnyDpi) {
+    Vector<int> densities;
+    const ssize_t highIndex = densities.add(ConfigDescription::DENSITY_HIGH);
+    const ssize_t xhighIndex = densities.add(ConfigDescription::DENSITY_XHIGH);
+    const ssize_t xxhighIndex = densities.add(ConfigDescription::DENSITY_XXHIGH);
+    const ssize_t anyIndex = densities.add(ConfigDescription::DENSITY_ANY);
+
+    EXPECT_RULES_EQ(RuleGenerator::generateDensity(densities, highIndex),
+            AndRule()
+            .add(LtRule(Rule::SDK_VERSION, SDK_LOLLIPOP))
+            .add(LtRule(Rule::SCREEN_DENSITY, 263))
+    );
+
+    EXPECT_RULES_EQ(RuleGenerator::generateDensity(densities, xhighIndex),
+            AndRule()
+            .add(LtRule(Rule::SDK_VERSION, SDK_LOLLIPOP))
+            .add(GtRule(Rule::SCREEN_DENSITY, 262))
+            .add(LtRule(Rule::SCREEN_DENSITY, 363))
+    );
+
+    EXPECT_RULES_EQ(RuleGenerator::generateDensity(densities, xxhighIndex),
+            AndRule()
+            .add(LtRule(Rule::SDK_VERSION, SDK_LOLLIPOP))
+            .add(GtRule(Rule::SCREEN_DENSITY, 362))
+    );
+
+    // We expect AlwaysTrue because anydpi always has attached v21 to the configuration
+    // and the rest of the rule generation code generates the sdk version checks.
+    EXPECT_RULES_EQ(RuleGenerator::generateDensity(densities, anyIndex), AlwaysTrue());
 }
 
 } // namespace split
diff --git a/tools/split-select/Rule_test.cpp b/tools/split-select/Rule_test.cpp
index aca7433..c6cff0d 100644
--- a/tools/split-select/Rule_test.cpp
+++ b/tools/split-select/Rule_test.cpp
@@ -17,42 +17,28 @@
 #include "Rule.h"
 
 #include "SplitDescription.h"
+#include "TestRules.h"
 
 #include <algorithm>
-#include <string>
 #include <gtest/gtest.h>
+#include <string>
 #include <utils/String8.h>
 
 using namespace android;
+using namespace split::test;
 
 namespace split {
 
 TEST(RuleTest, generatesValidJson) {
-    sp<Rule> rule = new Rule();
-    rule->op = Rule::AND_SUBRULES;
-
-    sp<Rule> subrule = new Rule();
-    subrule->op = Rule::EQUALS;
-    subrule->key = Rule::SDK_VERSION;
-    subrule->longArgs.add(7);
-    rule->subrules.add(subrule);
-
-    subrule = new Rule();
-    subrule->op = Rule::OR_SUBRULES;
-    rule->subrules.add(subrule);
-
-    sp<Rule> subsubrule = new Rule();
-    subsubrule->op = Rule::GREATER_THAN;
-    subsubrule->key = Rule::SCREEN_DENSITY;
-    subsubrule->longArgs.add(10);
-    subrule->subrules.add(subsubrule);
-
-    subsubrule = new Rule();
-    subsubrule->op = Rule::LESS_THAN;
-    subsubrule->key = Rule::SCREEN_DENSITY;
-    subsubrule->longArgs.add(5);
-    subrule->subrules.add(subsubrule);
-
+    Rule rule(AndRule()
+        .add(EqRule(Rule::SDK_VERSION, 7))
+        .add(OrRule()
+                .add(GtRule(Rule::SCREEN_DENSITY, 10))
+                .add(LtRule(Rule::SCREEN_DENSITY, 5))
+        )
+    );
+
+    // Expected
     std::string expected(
             "{"
             "  \"op\": \"AND_SUBRULES\","
@@ -79,69 +65,36 @@ TEST(RuleTest, generatesValidJson) {
             "     }"
             "  ]"
             "}");
-    // Trim
     expected.erase(std::remove_if(expected.begin(), expected.end(), ::isspace), expected.end());
 
-    std::string result(rule->toJson().string());
-
-    // Trim
+    // Result
+    std::string result(rule.toJson().string());
     result.erase(std::remove_if(result.begin(), result.end(), ::isspace), result.end());
 
     ASSERT_EQ(expected, result);
 }
 
 TEST(RuleTest, simplifiesSingleSubruleRules) {
-    sp<Rule> rule = new Rule();
-    rule->op = Rule::AND_SUBRULES;
+    sp<Rule> rule = new Rule(AndRule()
+        .add(EqRule(Rule::SDK_VERSION, 7))
+    );
 
-    sp<Rule> subrule = new Rule();
-    subrule->op = Rule::EQUALS;
-    subrule->key = Rule::SDK_VERSION;
-    subrule->longArgs.add(7);
-    rule->subrules.add(subrule);
-
-    sp<Rule> simplified = Rule::simplify(rule);
-    EXPECT_EQ(Rule::EQUALS, simplified->op);
-    EXPECT_EQ(Rule::SDK_VERSION, simplified->key);
-    ASSERT_EQ(1u, simplified->longArgs.size());
-    EXPECT_EQ(7, simplified->longArgs[0]);
+    EXPECT_RULES_EQ(Rule::simplify(rule), EqRule(Rule::SDK_VERSION, 7));
 }
 
 TEST(RuleTest, simplifiesNestedSameOpSubrules) {
-    sp<Rule> rule = new Rule();
-    rule->op = Rule::AND_SUBRULES;
-
-    sp<Rule> subrule = new Rule();
-    subrule->op = Rule::AND_SUBRULES;
-    rule->subrules.add(subrule);
-
-    sp<Rule> subsubrule = new Rule();
-    subsubrule->op = Rule::EQUALS;
-    subsubrule->key = Rule::SDK_VERSION;
-    subsubrule->longArgs.add(7);
-    subrule->subrules.add(subsubrule);
-
-    subrule = new Rule();
-    subrule->op = Rule::EQUALS;
-    subrule->key = Rule::SDK_VERSION;
-    subrule->longArgs.add(8);
-    rule->subrules.add(subrule);
-
-    sp<Rule> simplified = Rule::simplify(rule);
-    EXPECT_EQ(Rule::AND_SUBRULES, simplified->op);
-    ASSERT_EQ(2u, simplified->subrules.size());
-
-    sp<Rule> simplifiedSubrule = simplified->subrules[0];
-    EXPECT_EQ(Rule::EQUALS, simplifiedSubrule->op);
-    EXPECT_EQ(Rule::SDK_VERSION, simplifiedSubrule->key);
-    ASSERT_EQ(1u, simplifiedSubrule->longArgs.size());
-    EXPECT_EQ(7, simplifiedSubrule->longArgs[0]);
-
-    simplifiedSubrule = simplified->subrules[1];
-    EXPECT_EQ(Rule::EQUALS, simplifiedSubrule->op);
-    EXPECT_EQ(Rule::SDK_VERSION, simplifiedSubrule->key);
-    ASSERT_EQ(1u, simplifiedSubrule->longArgs.size());
-    EXPECT_EQ(8, simplifiedSubrule->longArgs[0]);
+    sp<Rule> rule = new Rule(AndRule()
+        .add(AndRule()
+            .add(EqRule(Rule::SDK_VERSION, 7))
+        )
+        .add(EqRule(Rule::SDK_VERSION, 8))
+    );
+
+    EXPECT_RULES_EQ(Rule::simplify(rule),
+            AndRule()
+                .add(EqRule(Rule::SDK_VERSION, 7))
+                .add(EqRule(Rule::SDK_VERSION, 8))
+    );
 }
 
 } // namespace split
diff --git a/tools/split-select/TestRules.cpp b/tools/split-select/TestRules.cpp
new file mode 100644
index 0000000..f980dc4
--- /dev/null
+++ b/tools/split-select/TestRules.cpp
@@ -0,0 +1,90 @@
+/*
+ * Copyright (C) 2014 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 "TestRules.h"
+
+#include <utils/String8.h>
+
+using android::String8;
+using android::sp;
+
+namespace split {
+namespace test {
+
+const Rule EqRule(Rule::Key key, long value) {
+    Rule rule;
+    rule.op = Rule::EQUALS;
+    rule.key = key;
+    rule.longArgs.add(value);
+    return rule;
+}
+
+const Rule GtRule(Rule::Key key, long value) {
+    Rule rule;
+    rule.op = Rule::GREATER_THAN;
+    rule.key = key;
+    rule.longArgs.add(value);
+    return rule;
+}
+
+const Rule LtRule(Rule::Key key, long value) {
+    Rule rule;
+    rule.op = Rule::LESS_THAN;
+    rule.key = key;
+    rule.longArgs.add(value);
+    return rule;
+}
+
+const Rule ContainsAnyRule(Rule::Key key, const char* str1) {
+    Rule rule;
+    rule.op = Rule::CONTAINS_ANY;
+    rule.key = key;
+    rule.stringArgs.add(String8(str1));
+    return rule;
+}
+
+const Rule ContainsAnyRule(Rule::Key key, const char* str1, const char* str2) {
+    Rule rule;
+    rule.op = Rule::CONTAINS_ANY;
+    rule.key = key;
+    rule.stringArgs.add(String8(str1));
+    rule.stringArgs.add(String8(str2));
+    return rule;
+}
+
+const Rule AlwaysTrue() {
+    Rule rule;
+    rule.op = Rule::ALWAYS_TRUE;
+    return rule;
+}
+
+::testing::AssertionResult RulePredFormat(
+        const char*, const char*,
+        const sp<Rule>& actual, const Rule& expected) {
+    const String8 expectedStr(expected.toJson());
+    const String8 actualStr(actual != NULL ? actual->toJson() : "");
+
+    if (expectedStr != actualStr) {
+        return ::testing::AssertionFailure()
+                << "Expected: " << expectedStr.string() << "\n"
+                << "  Actual: " << actualStr.string();
+    }
+    return ::testing::AssertionSuccess();
+}
+
+
+} // namespace test
+} // namespace split
diff --git a/tools/split-select/TestRules.h b/tools/split-select/TestRules.h
new file mode 100644
index 0000000..50b7ad1
--- /dev/null
+++ b/tools/split-select/TestRules.h
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2014 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.
+ */
+
+#ifndef H_AAPT_SPLIT_TEST_RULES
+#define H_AAPT_SPLIT_TEST_RULES
+
+#include "Rule.h"
+
+#include <gtest/gtest.h>
+
+namespace split {
+namespace test {
+
+struct AndRule : public Rule {
+    AndRule() {
+        op = Rule::AND_SUBRULES;
+    }
+
+    AndRule& add(const Rule& rhs) {
+        subrules.add(new Rule(rhs));
+        return *this;
+    }
+};
+
+struct OrRule : public Rule {
+    OrRule() {
+        op = Rule::OR_SUBRULES;
+    }
+
+    OrRule& add(const Rule& rhs) {
+        subrules.add(new Rule(rhs));
+        return *this;
+    }
+};
+
+const Rule EqRule(Rule::Key key, long value);
+const Rule LtRule(Rule::Key key, long value);
+const Rule GtRule(Rule::Key key, long value);
+const Rule ContainsAnyRule(Rule::Key key, const char* str1);
+const Rule ContainsAnyRule(Rule::Key key, const char* str1, const char* str2);
+const Rule AlwaysTrue();
+
+::testing::AssertionResult RulePredFormat(
+        const char* actualExpr, const char* expectedExpr,
+        const android::sp<Rule>& actual, const Rule& expected);
+
+#define EXPECT_RULES_EQ(actual, expected) \
+        EXPECT_PRED_FORMAT2(::split::test::RulePredFormat, actual, expected)
+
+} // namespace test
+} // namespace split
+
+#endif // H_AAPT_SPLIT_TEST_RULES