Fix subplan filtering

For filtering based on previous results, explicitly exclude modules that
contain 0 tests that should be rerun. This is especially necessary when
not-executed results should not be rerun, since these tests are not in
the result and cannot be excluded by filters directly. This also
prevents the loading of modules that contain no tests post-filtering.

Also rename -r option for subplan creation to conflict less with the
previous session's 'result', and modify the filter collections within
the compatibility testing infrastructure to be sets, rather than lists.

bug:29736541
Change-Id: I425c502c2565cdbe597da52321f5144dc532290f
diff --git a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/command/CompatibilityConsole.java b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/command/CompatibilityConsole.java
index ce5d023..b8ab089 100644
--- a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/command/CompatibilityConsole.java
+++ b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/command/CompatibilityConsole.java
@@ -216,8 +216,8 @@
         helpBuilder.append("Options:\n");
         helpBuilder.append("  --session/-s <session_id>: The session used to create a subplan.\n");
         helpBuilder.append("  --name/-n <subplan_name>: The name of the new subplan.\n");
-        helpBuilder.append("  --result/-r <status>: Which results to include in the subplan. ");
-        helpBuilder.append("One of passed, failed, not_executed. Repeatable.\n");
+        helpBuilder.append("  --result-type/-r <status>: Which results to include in the");
+        helpBuilder.append(" subplan. One of passed, failed, not_executed. Repeatable.\n");
 
         return helpBuilder.toString();
     }
diff --git a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/result/SubPlanCreator.java b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/result/SubPlanCreator.java
index 1c697e4..cbe3d2e 100644
--- a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/result/SubPlanCreator.java
+++ b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/result/SubPlanCreator.java
@@ -20,9 +20,13 @@
 import com.android.compatibility.common.tradefed.testtype.ISubPlan;
 import com.android.compatibility.common.tradefed.testtype.SubPlan;
 import com.android.compatibility.common.tradefed.util.OptionHelper;
+import com.android.compatibility.common.util.ICaseResult;
 import com.android.compatibility.common.util.IInvocationResult;
+import com.android.compatibility.common.util.IModuleResult;
+import com.android.compatibility.common.util.ITestResult;
 import com.android.compatibility.common.util.ResultHandler;
 import com.android.compatibility.common.util.TestFilter;
+import com.android.compatibility.common.util.TestStatus;
 import com.android.ddmlib.Log.LogLevel;
 import com.android.tradefed.config.ArgsOptionParser;
 import com.android.tradefed.config.ConfigurationException;
@@ -35,11 +39,12 @@
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
-import java.util.List;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.Set;
 
 /**
@@ -47,9 +52,18 @@
  */
 public class SubPlanCreator {
 
-    private static final String PASSED = "passed";
-    private static final String FAILED = "failed";
-    private static final String NOT_EXECUTED = "not_executed";
+    // result types
+    public static final String PASSED = "passed";
+    public static final String FAILED = "failed";
+    public static final String NOT_EXECUTED = "not_executed";
+    // static mapping of result types to TestStatuses
+    private static final Map<String, TestStatus> mStatusMap;
+    static {
+        Map<String, TestStatus> statusMap = new HashMap<String, TestStatus>();
+        statusMap.put(PASSED, TestStatus.PASS);
+        statusMap.put(FAILED, TestStatus.FAIL);
+        mStatusMap = Collections.unmodifiableMap(statusMap);
+    }
 
     @Option (name = "name", shortName = 'n', description = "the name of the subplan to create",
             importance=Importance.IF_UNSET)
@@ -59,21 +73,21 @@
             importance=Importance.IF_UNSET)
     private Integer mSessionId = null;
 
-    @Option (name = "result", shortName = 'r',
+    @Option (name = "result-type", shortName = 'r',
             description = "the result type to include. One of passed, failed, not_executed."
             + " Option may be repeated",
             importance=Importance.IF_UNSET)
-    private Set<String> mResultFilterStrings = new HashSet<String>();
+    private Set<String> mResultTypes = new HashSet<String>();
 
     @Option(name = CompatibilityTest.INCLUDE_FILTER_OPTION,
             description = "the include module filters to apply.",
             importance = Importance.NEVER)
-    private List<String> mIncludeFilters = new ArrayList<>();
+    private Set<String> mIncludeFilters = new HashSet<String>();
 
     @Option(name = CompatibilityTest.EXCLUDE_FILTER_OPTION,
             description = "the exclude module filters to apply.",
             importance = Importance.NEVER)
-    private List<String> mExcludeFilters = new ArrayList<>();
+    private Set<String> mExcludeFilters = new HashSet<String>();
 
     @Option(name = CompatibilityTest.MODULE_OPTION, shortName = 'm',
             description = "the test module to run.",
@@ -87,7 +101,7 @@
 
     @Option(name = CompatibilityTest.ABI_OPTION, shortName = 'a',
             description = "the abi to test.",
-            importance = Importance.IF_UNSET)
+            importance = Importance.NEVER)
     private String mAbiName = null;
 
     File mSubPlanFile = null;
@@ -104,10 +118,26 @@
     /**
      * Create a {@link SubPlanCreator} using the specified option values.
      */
-    public SubPlanCreator(String name, int session, Collection<String> resultFilterStrings) {
+    public SubPlanCreator(String name, int session, Collection<String> resultTypes) {
         mSubPlanName = name;
         mSessionId = session;
-        mResultFilterStrings.addAll(resultFilterStrings);
+        mResultTypes.addAll(resultTypes);
+    }
+
+    /**
+     * Set the result from which to derive the subplan.
+     * @param result
+     */
+    public void setResult(IInvocationResult result) {
+        mResult = result;
+    }
+
+    /**
+     * Add a result type from which to derive the subplan. PASSED, FAILED, or NOT_EXECUTED
+     * @param resultType
+     */
+    public void addResultType(String resultType) {
+        mResultTypes.add(resultType);
     }
 
     /**
@@ -137,8 +167,7 @@
     /**
      * Create a subplan derived from a result.
      * <p/>
-     * {@link Option} values must all be set before this is called.
-     *
+     * {@link Option} values must be set before this is called.
      * @param build
      * @return subplan
      * @throws ConfigurationException
@@ -147,56 +176,135 @@
             throws ConfigurationException {
         setupFields(buildHelper);
         ISubPlan subPlan = new SubPlan();
-        // At least one of the following three is true
-        boolean notExecuted = mResultFilterStrings.contains(NOT_EXECUTED);
-        boolean passed = mResultFilterStrings.contains(PASSED);
-        boolean failed = mResultFilterStrings.contains(FAILED);
-        if (notExecuted) {
-            // if including not_executed tests, include filters from previous session to
-            // track which tests must run
-            subPlan.addAllIncludeFilters(new HashSet<String>(mIncludeFilters));
-            subPlan.addAllExcludeFilters(new HashSet<String>(mExcludeFilters));
-            if (mModuleName != null) {
-                subPlan.addIncludeFilter(
-                        new TestFilter(mAbiName, mModuleName, mTestName).toString());
-            }
-            if (!passed) {
-                subPlan.excludePassed(mResult);
-            }
-            if (!failed) {
-                subPlan.excludeFailed(mResult);
-            }
-        } else {
-            // if only including executed tests, add each filter explicitly without filters from
-            // previous session
-            if (passed) {
-                subPlan.includePassed(mResult);
-            }
-            if (failed) {
-                subPlan.includeFailed(mResult);
-            }
+
+        // add filters from previous session to track which tests must run
+        subPlan.addAllIncludeFilters(mIncludeFilters);
+        subPlan.addAllExcludeFilters(mExcludeFilters);
+        if (mModuleName != null) {
+            subPlan.addIncludeFilter(new TestFilter(mAbiName, mModuleName, mTestName).toString());
         }
 
+        for (IModuleResult module : mResult.getModules()) {
+            if (shouldRunModule(module)) {
+                Set<TestStatus> statusesToRun = getStatusesToRun();
+                TestFilter moduleInclude =
+                            new TestFilter(module.getAbi(), module.getName(), null /*test*/);
+                if (shouldRunEntireModule(module)) {
+                    // include entire module
+                    subPlan.addIncludeFilter(moduleInclude.toString());
+                } else if (mResultTypes.contains(NOT_EXECUTED) && !module.isDone()) {
+                    // add module include and test excludes
+                    subPlan.addIncludeFilter(moduleInclude.toString());
+                    for (ICaseResult caseResult : module.getResults()) {
+                        for (ITestResult testResult : caseResult.getResults()) {
+                            if (!statusesToRun.contains(testResult.getResultStatus())) {
+                                TestFilter testExclude = new TestFilter(module.getAbi(),
+                                        module.getName(), testResult.getFullName());
+                                subPlan.addExcludeFilter(testExclude.toString());
+                            }
+                        }
+                    }
+                } else {
+                    // Not-executed tests should not be rerun and/or this module is completed
+                    // In any such case, it suffices to add includes for each test to rerun
+                    for (ICaseResult caseResult : module.getResults()) {
+                        for (ITestResult testResult : caseResult.getResults()) {
+                            if (statusesToRun.contains(testResult.getResultStatus())) {
+                                TestFilter testInclude = new TestFilter(module.getAbi(),
+                                        module.getName(), testResult.getFullName());
+                                subPlan.addIncludeFilter(testInclude.toString());
+                            }
+                        }
+                    }
+                }
+            } else {
+                // module should not run, exclude entire module
+                TestFilter moduleExclude =
+                        new TestFilter(module.getAbi(), module.getName(), null /*test*/);
+                subPlan.addExcludeFilter(moduleExclude.toString());
+            }
+        }
         return subPlan;
     }
 
     /**
+     * Whether any tests within the given {@link IModuleResult} should be run, based on
+     * the content of mResultTypes.
+     * @param module
+     * @return true if at least one test in the module should run
+     */
+    private boolean shouldRunModule(IModuleResult module) {
+        if (mResultTypes.contains(NOT_EXECUTED) && !module.isDone()) {
+            // module has not executed tests that the subplan should run
+            return true;
+        }
+        for (TestStatus status : getStatusesToRun()) {
+            if (module.countResults(status) > 0) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Whether all tests within the given {@link IModuleResult} should be run, based on
+     * the content of mResultTypes.
+     * @param module
+     * @return true if every test in the module should run
+     */
+    private boolean shouldRunEntireModule(IModuleResult module) {
+        if (!mResultTypes.contains(NOT_EXECUTED) && !module.isDone()) {
+            // module has not executed tests that the subplan should not run
+            return false;
+        }
+        Set<TestStatus> statusesToRun = getStatusesToRun();
+        for (TestStatus status : TestStatus.values()) {
+            if (!statusesToRun.contains(status)) {
+                // status is a TestStatus we don't want to run
+                if (module.countResults(status) > 0) {
+                    return false;
+                }
+            }
+        }
+        return true;
+    }
+
+    /**
+     * Retrieves a {@link Set} of {@link TestStatus}es to run, based on the content of
+     * mResultTypes. Does not account for result type NOT_EXECUTED, since no such TestStatus
+     * exists.
+     * @return set of TestStatuses to run
+     */
+    private Set<TestStatus> getStatusesToRun() {
+        Set<TestStatus> statusesToRun = new HashSet<TestStatus>();
+        for (String resultType : mResultTypes) {
+            // no test status exists for not-executed tests
+            if (resultType != NOT_EXECUTED) {
+                statusesToRun.add(mStatusMap.get(resultType));
+            }
+        }
+        return statusesToRun;
+    }
+
+    /**
      * Ensure that all {@Option}s and fields are populated with valid values.
      * @param buildHelper
      * @throws ConfigurationException if any option has an invalid value
      */
     private void setupFields(CompatibilityBuildHelper buildHelper) throws ConfigurationException {
-        if (mSessionId == null) {
-            throw new ConfigurationException("Missing --session argument");
-        }
-        try {
-            mResult = ResultHandler.findResult(buildHelper.getResultsDir(), mSessionId);
-        } catch (FileNotFoundException e) {
-            throw new RuntimeException(e);
-        }
         if (mResult == null) {
-            throw new IllegalArgumentException(String.format(
-                    "Could not find session with id %d", mSessionId));
+            if (mSessionId == null) {
+                throw new ConfigurationException("Missing --session argument");
+            }
+            try {
+                mResult = ResultHandler.findResult(buildHelper.getResultsDir(), mSessionId);
+            } catch (FileNotFoundException e) {
+                throw new RuntimeException(e);
+            }
+            if (mResult == null) {
+                throw new IllegalArgumentException(String.format(
+                        "Could not find session with id %d", mSessionId));
+            }
         }
 
         String retryCommandLineArgs = mResult.getCommandLineArgs();
@@ -210,18 +318,15 @@
             }
         }
 
-        if (mResultFilterStrings.isEmpty()) {
+        if (mResultTypes.isEmpty()) {
             // add all valid values, include all tests of all statuses
-            mResultFilterStrings.addAll(
+            mResultTypes.addAll(
                     new HashSet<String>(Arrays.asList(PASSED, FAILED, NOT_EXECUTED)));
         }
         // validate all test status values
-        for (String filterString : mResultFilterStrings) {
-            if (!filterString.equals(PASSED)
-                    && !filterString.equals(FAILED)
-                    && !filterString.equals(NOT_EXECUTED)) {
-                throw new ConfigurationException(String.format("result filter string %s invalid",
-                        filterString));
+        for (String type : mResultTypes) {
+            if (!type.equals(PASSED) && !type.equals(FAILED) && !type.equals(NOT_EXECUTED)) {
+                throw new ConfigurationException(String.format("result type %s invalid", type));
             }
         }
 
@@ -239,10 +344,19 @@
         }
     }
 
+    /**
+     * Helper to create a plan name if none is explicitly set
+     */
     private String createPlanName() {
         StringBuilder sb = new StringBuilder();
-        sb.append(String.join("_", mResultFilterStrings));
-        sb.append(Integer.toString(mSessionId));
+        sb.append(String.join("_", mResultTypes));
+        sb.append("_");
+        if (mSessionId != null) {
+            sb.append(Integer.toString(mSessionId));
+            sb.append("_");
+        }
+        // use unique start time for name
+        sb.append(CompatibilityBuildHelper.getDirSuffix(mResult.getStartTime()));
         return sb.toString();
     }
 }
diff --git a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/CompatibilityTest.java b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/CompatibilityTest.java
index 70e3588..9bd8647 100644
--- a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/CompatibilityTest.java
+++ b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/CompatibilityTest.java
@@ -18,6 +18,7 @@
 
 import com.android.compatibility.SuiteInfo;
 import com.android.compatibility.common.tradefed.build.CompatibilityBuildHelper;
+import com.android.compatibility.common.tradefed.result.SubPlanCreator;
 import com.android.compatibility.common.tradefed.targetprep.NetworkConnectivityChecker;
 import com.android.compatibility.common.tradefed.targetprep.SystemStatusChecker;
 import com.android.compatibility.common.tradefed.util.OptionHelper;
@@ -115,12 +116,12 @@
     @Option(name = INCLUDE_FILTER_OPTION,
             description = "the include module filters to apply.",
             importance = Importance.ALWAYS)
-    private List<String> mIncludeFilters = new ArrayList<>();
+    private Set<String> mIncludeFilters = new HashSet<>();
 
     @Option(name = EXCLUDE_FILTER_OPTION,
             description = "the exclude module filters to apply.",
             importance = Importance.ALWAYS)
-    private List<String> mExcludeFilters = new ArrayList<>();
+    private Set<String> mExcludeFilters = new HashSet<>();
 
     @Option(name = MODULE_OPTION,
             shortName = 'm',
@@ -611,15 +612,26 @@
                 }
             }
 
-            ISubPlan retryPlan = new SubPlan();
-            retryPlan.excludePassed(result); // always exclude passed tests on retry
+            SubPlanCreator retryPlanCreator = new SubPlanCreator();
+            retryPlanCreator.setResult(result);
             if (RetryType.FAILED.equals(mRetryType)) {
-                    retryPlan.includeFailed(result); // retry only failed tests
+                // retry only failed tests
+                retryPlanCreator.addResultType(SubPlanCreator.FAILED);
             } else if (RetryType.NOT_EXECUTED.equals(mRetryType)){
-                    retryPlan.excludeFailed(result); // retry only not executed tests
+                // retry only not executed tests
+                retryPlanCreator.addResultType(SubPlanCreator.NOT_EXECUTED);
+            } else {
+                // retry both failed and not executed tests
+                retryPlanCreator.addResultType(SubPlanCreator.FAILED);
+                retryPlanCreator.addResultType(SubPlanCreator.NOT_EXECUTED);
             }
-            mIncludeFilters.addAll(retryPlan.getIncludeFilters());
-            mExcludeFilters.addAll(retryPlan.getExcludeFilters());
+            try {
+                ISubPlan retryPlan = retryPlanCreator.createSubPlan(mBuildHelper);
+                mIncludeFilters.addAll(retryPlan.getIncludeFilters());
+                mExcludeFilters.addAll(retryPlan.getExcludeFilters());
+            } catch (ConfigurationException e) {
+                throw new RuntimeException ("Failed to create subplan for retry", e);
+            }
         }
         if (mSubPlan != null) {
             try {
@@ -670,8 +682,8 @@
     }
 
     /* Helper method designed to remove filters in a list not applicable to the given module */
-    private static void cleanFilters(List<String> filters, String module) {
-        List<String> cleanedFilters = new ArrayList<String>();
+    private static void cleanFilters(Set<String> filters, String module) {
+        Set<String> cleanedFilters = new HashSet<String>();
         for (String filter : filters) {
             if (module.equals(TestFilter.createFrom(filter).getName())) {
                 cleanedFilters.add(filter); // Module name matches, filter passes
diff --git a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/IModuleRepo.java b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/IModuleRepo.java
index 7cbf7a3..540373b 100644
--- a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/IModuleRepo.java
+++ b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/IModuleRepo.java
@@ -47,8 +47,8 @@
      * Initializes the repository.
      */
     void initialize(int shards, File testsDir, Set<IAbi> abis, List<String> deviceTokens,
-            List<String> testArgs, List<String> moduleArgs, List<String> mIncludeFilters,
-            List<String> mExcludeFilters, IBuildInfo buildInfo);
+            List<String> testArgs, List<String> moduleArgs, Set<String> mIncludeFilters,
+            Set<String> mExcludeFilters, IBuildInfo buildInfo);
 
     /**
      * @return a {@link Map} of all modules to run on the device referenced by the given serial.
diff --git a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/ISubPlan.java b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/ISubPlan.java
index adc0227..eda40b2 100644
--- a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/ISubPlan.java
+++ b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/ISubPlan.java
@@ -15,7 +15,6 @@
  */
 package com.android.compatibility.common.tradefed.testtype;
 
-import com.android.compatibility.common.util.IInvocationResult;
 import com.android.tradefed.testtype.ITestFilterReceiver;
 import com.android.tradefed.util.xml.AbstractXmlParser.ParseException;
 
@@ -37,30 +36,6 @@
     public void parse(InputStream xmlInputStream) throws ParseException;
 
     /**
-     * Add include filters for {@link ITestResult}s that have passed.
-     * @param result the {@link IInvocationResult} from which to read {@link TestStatus}es
-     */
-    public void includePassed(IInvocationResult result);
-
-    /**
-     * Add include filters for {@link ITestResult}s that have failed.
-     * @param result the {@link IInvocationResult} from which to read {@link TestStatus}es
-     */
-    public void includeFailed(IInvocationResult result);
-
-    /**
-     * Add exclude filters for {@link ITestResult}s that have passed.
-     * @param result the {@link IInvocationResult} from which to read {@link TestStatus}es
-     */
-    public void excludePassed(IInvocationResult result);
-
-    /**
-     * Add exclude filters for {@link ITestResult}s that have failed.
-     * @param result the {@link IInvocationResult} from which to read {@link TestStatus}es
-     */
-    public void excludeFailed(IInvocationResult result);
-
-    /**
      * Retrieve the set of include filters previously added or parsed from XML.
      * @return a set of include filter strings
      */
diff --git a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/ModuleRepo.java b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/ModuleRepo.java
index c4ff1b0..3d1b234 100644
--- a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/ModuleRepo.java
+++ b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/ModuleRepo.java
@@ -233,8 +233,8 @@
      */
     @Override
     public void initialize(int shards, File testsDir, Set<IAbi> abis, List<String> deviceTokens,
-            List<String> testArgs, List<String> moduleArgs, List<String> includeFilters,
-            List<String> excludeFilters, IBuildInfo buildInfo) {
+            List<String> testArgs, List<String> moduleArgs, Set<String> includeFilters,
+            Set<String> excludeFilters, IBuildInfo buildInfo) {
         CLog.d("Initializing ModuleRepo\nShards:%d\nTests Dir:%s\nABIs:%s\nDevice Tokens:%s\n" +
                 "Test Args:%s\nModule Args:%s\nIncludes:%s\nExcludes:%s",
                 shards, testsDir.getAbsolutePath(), abis, deviceTokens, testArgs, moduleArgs,
@@ -355,7 +355,7 @@
         return shardedList;
     }
 
-    private static void addFilters(List<String> stringFilters,
+    private static void addFilters(Set<String> stringFilters,
             Map<String, List<TestFilter>> filters, Set<IAbi> abis) {
         for (String filterString : stringFilters) {
             TestFilter filter = TestFilter.createFrom(filterString);
diff --git a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/SubPlan.java b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/SubPlan.java
index 80ca30e..54d2869 100644
--- a/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/SubPlan.java
+++ b/common/host-side/tradefed/src/com/android/compatibility/common/tradefed/testtype/SubPlan.java
@@ -15,13 +15,7 @@
  */
 package com.android.compatibility.common.tradefed.testtype;
 
-import com.android.compatibility.common.util.ICaseResult;
-import com.android.compatibility.common.util.IInvocationResult;
-import com.android.compatibility.common.util.IModuleResult;
-import com.android.compatibility.common.util.ITestResult;
-
 import com.android.compatibility.common.util.TestFilter;
-import com.android.compatibility.common.util.TestStatus;
 import com.android.tradefed.util.xml.AbstractXmlParser;
 
 import org.kxml2.io.KXmlSerializer;
@@ -66,88 +60,6 @@
      * {@inheritDoc}
      */
     @Override
-    public void includePassed(IInvocationResult result) {
-        for (IModuleResult module : result.getModules()) {
-            if (module.isPassed()) {
-                // Whole module passed, exclude
-                TestFilter filter =
-                        new TestFilter(module.getAbi(), module.getName(), null /*test*/);
-                mIncludes.add(filter.toString());
-            } else {
-                for (ICaseResult testResultList : module.getResults()) {
-                    for (ITestResult testResult : testResultList.getResults(TestStatus.PASS)) {
-                        // Test passed, exclude
-                        TestFilter filter = new TestFilter(
-                                module.getAbi(), module.getName(), testResult.getFullName());
-                        mIncludes.add(filter.toString());
-                    }
-                }
-            }
-        }
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    public void includeFailed(IInvocationResult result) {
-        for (IModuleResult moduleResult : result.getModules()) {
-            for (ICaseResult caseResult : moduleResult.getResults()) {
-                for (ITestResult testResult : caseResult.getResults(TestStatus.FAIL)) {
-                    // Test failed, include for retry
-                    TestFilter filter = new TestFilter(moduleResult.getAbi(),
-                            moduleResult.getName(), testResult.getFullName());
-                    mIncludes.add(filter.toString());
-                }
-            }
-        }
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    public void excludePassed(IInvocationResult result) {
-        for (IModuleResult module : result.getModules()) {
-            if (module.isPassed()) {
-                // Whole module passed, exclude
-                TestFilter filter =
-                        new TestFilter(module.getAbi(), module.getName(), null /*test*/);
-                mExcludes.add(filter.toString());
-            } else {
-                for (ICaseResult testResultList : module.getResults()) {
-                    for (ITestResult testResult : testResultList.getResults(TestStatus.PASS)) {
-                        // Test passed, exclude
-                        TestFilter filter = new TestFilter(
-                                module.getAbi(), module.getName(), testResult.getFullName());
-                        mExcludes.add(filter.toString());
-                    }
-                }
-            }
-        }
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    public void excludeFailed(IInvocationResult result) {
-        for (IModuleResult moduleResult : result.getModules()) {
-            for (ICaseResult caseResult : moduleResult.getResults()) {
-                for (ITestResult testResult : caseResult.getResults(TestStatus.FAIL)) {
-                    // Test failed, include for retry
-                    TestFilter filter = new TestFilter(moduleResult.getAbi(),
-                            moduleResult.getName(), testResult.getFullName());
-                    mExcludes.add(filter.toString());
-                }
-            }
-        }
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    @Override
     public void addIncludeFilter(String filter) {
         mIncludes.add(filter);
     }
diff --git a/common/host-side/tradefed/tests/src/com/android/compatibility/common/tradefed/result/SubPlanCreatorTest.java b/common/host-side/tradefed/tests/src/com/android/compatibility/common/tradefed/result/SubPlanCreatorTest.java
index 2ad7260..192aabb 100644
--- a/common/host-side/tradefed/tests/src/com/android/compatibility/common/tradefed/result/SubPlanCreatorTest.java
+++ b/common/host-side/tradefed/tests/src/com/android/compatibility/common/tradefed/result/SubPlanCreatorTest.java
@@ -117,13 +117,13 @@
         ISubPlan plan = mSubPlanCreator.createSubPlan(mBuildHelper);
         Set<String> planIncludes = plan.getIncludeFilters();
         Set<String> planExcludes = plan.getExcludeFilters();
-        TestFilter passingTest1 =
-                new TestFilter(ABI, NAME_A, String.format("%s#%s", CLASS_A, METHOD_1));
-        TestFilter passingTest2 =
-                new TestFilter(ABI, NAME_B, String.format("%s#%s", CLASS_B, METHOD_4));
+        TestFilter mf1 = new TestFilter(ABI, NAME_A, null);
+        TestFilter tf1 = new TestFilter(ABI, NAME_A, String.format("%s#%s", CLASS_A, METHOD_1));
+        TestFilter tf3 = new TestFilter(ABI, NAME_B, String.format("%s#%s", CLASS_B, METHOD_3));
         assertTrue(planIncludes.contains("CtsMyModuleTestCases")); // command-line '-m' arg
-        assertTrue(planExcludes.contains(passingTest1.toString()));
-        assertTrue(planExcludes.contains(passingTest2.toString()));
+        assertTrue(planIncludes.contains(mf1.toString())); // include module with not-executed test
+        assertTrue(planExcludes.contains(tf1.toString())); // exclude passing test in that module
+        assertTrue(planIncludes.contains(tf3.toString())); // include failure in executed module
     }
 
     private void populateResults() throws Exception {
diff --git a/common/host-side/tradefed/tests/src/com/android/compatibility/common/tradefed/testtype/ModuleRepoTest.java b/common/host-side/tradefed/tests/src/com/android/compatibility/common/tradefed/testtype/ModuleRepoTest.java
index 3b8e4cb..5a0ebed 100644
--- a/common/host-side/tradefed/tests/src/com/android/compatibility/common/tradefed/testtype/ModuleRepoTest.java
+++ b/common/host-side/tradefed/tests/src/com/android/compatibility/common/tradefed/testtype/ModuleRepoTest.java
@@ -59,8 +59,8 @@
     private static final List<String> DEVICE_TOKENS = new ArrayList<>();
     private static final List<String> TEST_ARGS= new ArrayList<>();
     private static final List<String> MODULE_ARGS = new ArrayList<>();
-    private static final List<String> INCLUDES = new ArrayList<>();
-    private static final List<String> EXCLUDES = new ArrayList<>();
+    private static final Set<String> INCLUDES = new HashSet<>();
+    private static final Set<String> EXCLUDES = new HashSet<>();
     private static final Set<String> FILES = new HashSet<>();
     private static final String FILENAME = "%s.config";
     private static final String ABI_32 = "armeabi-v7a";
@@ -184,9 +184,9 @@
     }
 
     public void testFiltering() throws Exception {
-        List<String> includeFilters = new ArrayList<>();
+        Set<String> includeFilters = new HashSet<>();
         includeFilters.add(MODULE_NAME_A);
-        List<String> excludeFilters = new ArrayList<>();
+        Set<String> excludeFilters = new HashSet<>();
         excludeFilters.add(ID_A_32);
         excludeFilters.add(MODULE_NAME_B);
         mRepo.initialize(1, mTestsDir, ABIS, DEVICE_TOKENS, TEST_ARGS, MODULE_ARGS, includeFilters,
@@ -234,8 +234,8 @@
         abis.add(new Abi(ABI_64, "64"));
         ArrayList<String> emptyList = new ArrayList<>();
 
-        mRepo.initialize(3, mTestsDir, abis, DEVICE_TOKENS, emptyList, emptyList, emptyList,
-                         emptyList, mBuild);
+        mRepo.initialize(3, mTestsDir, abis, DEVICE_TOKENS, emptyList, emptyList, INCLUDES,
+                         EXCLUDES, mBuild);
 
         List<IModuleDef> modules = new ArrayList<>();
         modules.addAll(mRepo.getLargeModules());