Bug 1123763 - [manifestparser] Implement filter system for manifest.active_tests(), r=ted
authorAndrew Halberstadt <ahalberstadt@mozilla.com>
Tue, 10 Feb 2015 09:38:29 -0500
changeset 228674 d2717f1bd34872b5be255f2c0d4bed9c34a4cf3b
parent 228673 037072f411fe811230f90b3904bb0b456abd4aed
child 228675 b24f9a3b47a6f1bb6bc3b74932d07dc783719e0b
push idunknown
push userunknown
push dateunknown
reviewersted
bugs1123763
milestone38.0a1
Bug 1123763 - [manifestparser] Implement filter system for manifest.active_tests(), r=ted A filter is a callable that accepts an iterable of tests and a dictionary of values (e.g mozinfo.info) and returns an iterable of tests. Note filtering can mean modifying tests in addition to removing them. For example, this implements a "timeout-if" tag in the manifest: from manifestparser import expression import mozinfo def timeout_if(tests, values): for test in tests: if 'timeout-if' in test: timeout, condition = test['timeout-if'].split(',', 1) if expression.parse(condition, **values): test['timeout'] = timeout yield test tests = mp.active_tests(filters=[timeout_if], **mozinfo.info)
testing/mochitest/mochitest_options.py
testing/mochitest/runtests.py
testing/mozbase/docs/conf.py
testing/mozbase/docs/manifestparser.rst
testing/mozbase/manifestparser/manifestparser/filters.py
testing/mozbase/manifestparser/manifestparser/manifestparser.py
testing/mozbase/manifestparser/setup.py
testing/mozbase/manifestparser/tests/manifest.ini
testing/mozbase/manifestparser/tests/test_filters.py
testing/mozbase/manifestparser/tests/test_testmanifest.py
--- a/testing/mochitest/mochitest_options.py
+++ b/testing/mochitest/mochitest_options.py
@@ -158,17 +158,17 @@ class MochitestOptions(optparse.OptionPa
           "dest": "browserChrome",
           "help": "run browser chrome Mochitests",
           "default": False,
         }],
         [["--subsuite"],
         { "action": "store",
           "dest": "subsuite",
           "help": "subsuite of tests to run",
-          "default": "",
+          "default": None,
         }],
         [["--jetpack-package"],
         { "action": "store_true",
           "dest": "jetpackPackage",
           "help": "run jetpack package tests",
           "default": False,
         }],
         [["--jetpack-addon"],
--- a/testing/mochitest/runtests.py
+++ b/testing/mochitest/runtests.py
@@ -41,16 +41,17 @@ from automationutils import (
     ShutdownLeaks,
     printstatus,
     LSANLeaks,
     setAutomationLog,
 )
 
 from datetime import datetime
 from manifestparser import TestManifest
+from manifestparser.filters import subsuite
 from mochitest_options import MochitestOptions
 from mozprofile import Profile, Preferences
 from mozprofile.permissions import ServerLocations
 from urllib import quote_plus as encodeURIComponent
 from mozlog.structured.formatters import TbplFormatter
 from mozlog.structured import commandline
 
 # This should use the `which` module already in tree, but it is
@@ -1656,25 +1657,27 @@ class Mochitest(MochitestUtilsMixin):
       # Bug 883858 - return all tests including disabled tests
       testPath = self.getTestPath(options)
       testPath = testPath.replace('\\', '/')
       if testPath.endswith('.html') or \
          testPath.endswith('.xhtml') or \
          testPath.endswith('.xul') or \
          testPath.endswith('.js'):
           # In the case where we have a single file, we don't want to filter based on options such as subsuite.
-          tests = manifest.active_tests(disabled=disabled, options=None, **info)
+          tests = manifest.active_tests(disabled=disabled, **info)
           for test in tests:
             if 'disabled' in test:
               del test['disabled']
 
       else:
-        tests = manifest.active_tests(disabled=disabled, options=options, **info)
+        filters = [subsuite(options.subsuite)]
+        tests = manifest.active_tests(
+            disabled=disabled, filters=filters, **info)
         if len(tests) == 0:
-          tests = manifest.active_tests(disabled=True, options=options, **info)
+          tests = manifest.active_tests(disabled=True, **info)
 
     paths = []
 
     for test in tests:
       if len(tests) == 1 and 'disabled' in test:
         del test['disabled']
 
       pathAbs = os.path.abspath(test['path'])
--- a/testing/mozbase/docs/conf.py
+++ b/testing/mozbase/docs/conf.py
@@ -93,16 +93,25 @@ pygments_style = 'sphinx'
 #modindex_common_prefix = []
 
 
 # -- Options for HTML output ---------------------------------------------------
 
 # The theme to use for HTML and HTML Help pages.  See the documentation for
 # a list of builtin themes.
 html_theme = 'default'
+on_rtd = os.environ.get('READTHEDOCS', None) == 'True'
+
+if not on_rtd:
+    try:
+        import sphinx_rtd_theme
+        html_theme = 'sphinx_rtd_theme'
+        html_theme_path = [sphinx_rtd_theme.get_html_theme_path()]
+    except ImportError:
+        pass
 
 # Theme options are theme-specific and customize the look and feel of a theme
 # further.  For a list of options available for each theme, see the
 # documentation.
 #html_theme_options = {}
 
 # Add any paths that contain custom themes here, relative to this directory.
 #html_theme_path = []
--- a/testing/mozbase/docs/manifestparser.rst
+++ b/testing/mozbase/docs/manifestparser.rst
@@ -1,11 +1,13 @@
 Managing lists of tests
 =======================
 
+.. py:currentmodule:: manifestparser
+
 We don't always want to run all tests, all the time. Sometimes a test
 may be broken, in other cases we only want to run a test on a specific
 platform or build of Mozilla. To handle these cases (and more), we
 created a python library to create and use test "manifests", which
 codify this information.
 
 :mod:`manifestparser` --- Create and manage test manifests
 -----------------------------------------------------------
@@ -257,43 +259,83 @@ 3. Optionally, a harness will have an in
 from TestManifest if more harness-specific customization is desired at
 the manifest level.
 
 See the source code at https://github.com/mozilla/mozbase/tree/master/manifestparser
 and
 https://github.com/mozilla/mozbase/blob/master/manifestparser/manifestparser.py
 in particular.
 
-Using Manifests
-```````````````
+Filtering Manifests
+```````````````````
+
+After creating a `TestManifest` object, all manifest files are read and a list
+of test objects can be accessed via `TestManifest.tests`. However this list contains
+all test objects, whether they should be run or not. Normally they need to be
+filtered down only to the set of tests that should be run by the test harness.
+
+To do this, a test harness can call `TestManifest.active_tests`:
+
+.. code-block:: python
+
+    tests = manifest.active_tests(exists=True, disabled=True, **tags)
+
+By default, `active_tests` runs the filters found in
+:attr:`~.DEFAULT_FILTERS`. It also accepts two convenience arguments:
+
+1. `exists`: if True (default), filter out tests that do not exist on the local file system.
+2. `disabled`: if True (default), do not filter out tests containing the 'disabled' key
+   (which can be set by `skip-if`, `run-if` or manually).
 
-A test harness will normally call `TestManifest.active_tests`:
+This works for simple cases, but there are other built-in filters, or even custom filters
+that can be applied to the `TestManifest`. To do so, add the filter to `TestManifest.filters`:
+
+.. code-block:: python
+
+    from manifestparser.filters import subsuite
+    import mozinfo
+
+    filters = [subsuite('devtools')]
+    tests = manifest.active_tests(filters=filters, **mozinfo.info)
+
+.. automodule:: manifestparser.filters
+    :members:
+    :exclude-members: filterlist,InstanceFilter,DEFAULT_FILTERS
+
+.. autodata::  manifestparser.filters.DEFAULT_FILTERS
+    :annotation:
+
+For example, suppose we want to introduce a new key called `timeout-if` that adds a
+'timeout' property to a test if a certain condition is True. The syntax in the manifest
+files will look like this:
 
 .. code-block:: text
 
-    def active_tests(self, exists=True, disabled=True, **tags):
+    [test_foo.py]
+    timeout-if = 300, os == 'win'
 
-The manifests are passed to the `__init__` or `read` methods with
-appropriate arguments.  `active_tests` then allows you to select the
-tests you want:
+The value is <timeout>, <condition> where condition is the same format as the one in
+`skip-if`. In the above case, if os == 'win', a timeout of 300 seconds will be
+applied. Otherwise, no timeout will be applied. All we need to do is define the filter
+and add it:
+
+.. code-block:: python
 
-- exists : return only existing tests
-- disabled : whether to return disabled tests; if not these will be
-  filtered out; if True (the default), the `disabled` key of a
-  test's metadata will be present and will be set to the reason that a
-  test is disabled
-- tags : keys and values to filter on (e.g. `os='linux'`)
+    from manifestparser.expression import parse
+    import mozinfo
 
-`active_tests` looks for tests with `skip-if`
-`run-if`.  If the condition is or is not fulfilled,
-respectively, the test is marked as disabled.  For instance, if you
-pass `**dict(os='linux')` as `**tags`, if a test contains a line
-`skip-if = os == 'linux'` this test will be disabled, or
-`run-if = os = 'win'` in which case the test will also be disabled.  It
-is up to the harness to pass in tags appropriate to its usage.
+    def timeout_if(tests, values):
+        for test in tests:
+            if 'timeout-if' in test:
+                timeout, condition = test['timeout-if'].split(',', 1)
+                if parse(condition, **values):
+                    test['timeout'] = timeout
+            yield test
+
+    tests = manifest.active_tests(filters=[timeout_if], **mozinfo.info)
 
 Creating Manifests
 ``````````````````
 
 manifestparser comes with a console script, `manifestparser create`, that
 may be used to create a seed manifest structure from a directory of
 files.  Run `manifestparser help create` for usage information.
 
new file mode 100644
--- /dev/null
+++ b/testing/mozbase/manifestparser/manifestparser/filters.py
@@ -0,0 +1,179 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this file,
+# You can obtain one at http://mozilla.org/MPL/2.0/.
+
+"""
+A filter is a callable that accepts an iterable of test objects and a
+dictionary of values, and returns a new iterable of test objects. It is
+possible to define custom filters if the built-in ones are not enough.
+"""
+
+from collections import MutableSequence
+import os
+
+from .expression import (
+    parse,
+    ParseError,
+)
+
+
+# built-in filters
+
+def skip_if(tests, values):
+    """
+    Sets disabled on all tests containing the `skip-if` tag and whose condition
+    is True. This filter is added by default.
+    """
+    tag = 'skip-if'
+    for test in tests:
+        if tag in test and parse(test[tag], **values):
+            test.setdefault('disabled', '{}: {}'.format(tag, test[tag]))
+        yield test
+
+
+def run_if(tests, values):
+    """
+    Sets disabled on all tests containing the `run-if` tag and whose condition
+    is False. This filter is added by default.
+    """
+    tag = 'run-if'
+    for test in tests:
+        if tag in test and not parse(test[tag], **values):
+            test.setdefault('disabled', '{}: {}'.format(tag, test[tag]))
+        yield test
+
+
+def fail_if(tests, values):
+    """
+    Sets expected to 'fail' on all tests containing the `fail-if` tag and whose
+    condition is True. This filter is added by default.
+    """
+    tag = 'fail-if'
+    for test in tests:
+        if tag in test and parse(test[tag], **values):
+            test['expected'] = 'fail'
+        yield test
+
+
+def enabled(tests, values):
+    """
+    Removes all tests containing the `disabled` key. This filter can be
+    added by passing `disabled=False` into `active_tests`.
+    """
+    for test in tests:
+        if 'disabled' not in test:
+            yield test
+
+
+def exists(tests, values):
+    """
+    Removes all tests that do not exist on the file system. This filter is
+    added by default, but can be removed by passing `exists=False` into
+    `active_tests`.
+    """
+    for test in tests:
+        if os.path.exists(test['path']):
+            yield test
+
+
+# built-in instance filters
+
+class InstanceFilter(object):
+    """
+    Generally only one instance of a class filter should be applied at a time.
+    Two instances of `InstanceFilter` are considered equal if they have the
+    same class name. This ensures only a single instance is ever added to
+    `filterlist`.
+    """
+    def __eq__(self, other):
+        return self.__class__ == other.__class__
+
+
+class subsuite(InstanceFilter):
+    """
+    If `name` is None, removes all tests that have a `subsuite` key.
+    Otherwise removes all tests that do not have a subsuite matching `name`.
+
+    It is possible to specify conditional subsuite keys using:
+       subsuite = foo,condition
+
+    where 'foo' is the subsuite name, and 'condition' is the same type of
+    condition used for skip-if.  If the condition doesn't evaluate to true,
+    the subsuite designation will be removed from the test.
+
+    :param name: The name of the subsuite to run (default None)
+    """
+    def __init__(self, name=None):
+        self.name = name
+
+    def __call__(self, tests, values):
+        # Look for conditional subsuites, and replace them with the subsuite
+        # itself (if the condition is true), or nothing.
+        for test in tests:
+            subsuite = test.get('subsuite', '')
+            if ',' in subsuite:
+                try:
+                    subsuite, cond = subsuite.split(',')
+                except ValueError:
+                    raise ParseError("subsuite condition can't contain commas")
+                matched = parse(cond, **values)
+                if matched:
+                    test['subsuite'] = subsuite
+                else:
+                    test['subsuite'] = ''
+
+            # Filter on current subsuite
+            if self.name is None:
+                if not test.get('subsuite'):
+                    yield test
+            else:
+                if test.get('subsuite') == self.name:
+                    yield test
+
+
+# filter container
+
+DEFAULT_FILTERS = (
+    skip_if,
+    run_if,
+    fail_if,
+)
+"""
+By default :func:`~.active_tests` will run the :func:`~.skip_if`,
+:func:`~.run_if` and :func:`~.fail_if` filters.
+"""
+
+
+class filterlist(MutableSequence):
+    """
+    A MutableSequence that raises TypeError when adding a non-callable and
+    ValueError if the item is already added.
+    """
+
+    def __init__(self, items=None):
+        self.items = []
+        if items:
+            self.items = list(items)
+
+    def _validate(self, item):
+        if not callable(item):
+            raise TypeError("Filters must be callable!")
+        if item in self:
+            raise ValueError("Filter {} is already applied!".format(item))
+
+    def __getitem__(self, key):
+        return self.items[key]
+
+    def __setitem__(self, key, value):
+        self._validate(value)
+        self.items[key] = value
+
+    def __delitem__(self, key):
+        del self.items[key]
+
+    def __len__(self):
+        return len(self.items)
+
+    def insert(self, index, value):
+        self._validate(value)
+        self.items.insert(index, value)
old mode 100755
new mode 100644
--- a/testing/mozbase/manifestparser/manifestparser/manifestparser.py
+++ b/testing/mozbase/manifestparser/manifestparser/manifestparser.py
@@ -7,19 +7,21 @@
 from StringIO import StringIO
 import json
 import fnmatch
 import os
 import shutil
 import sys
 
 from .ini import read_ini
-from .expression import (
-    parse,
-    ParseError,
+from .filters import (
+    DEFAULT_FILTERS,
+    enabled,
+    exists as _exists,
+    filterlist,
 )
 
 relpath = os.path.relpath
 string = (basestring,)
 
 
 ### path normalization
 
@@ -306,21 +308,23 @@ class ManifestParser(object):
 
     def paths(self):
         return [i['path'] for i in self.tests]
 
 
     ### methods for auditing
 
     def missing(self, tests=None):
-        """return list of tests that do not exist on the filesystem"""
+        """
+        return list of tests that do not exist on the filesystem
+        """
         if tests is None:
             tests = self.tests
-        return [test for test in tests
-                if not os.path.exists(test['path'])]
+        existing = list(_exists(tests, {}))
+        return [t for t in tests if t not in existing]
 
     def check_missing(self, tests=None):
         missing = self.missing(tests=tests)
         if missing:
             missing_paths = [test['path'] for test in missing]
             if self.strict:
                 raise IOError("Strict mode enabled, test paths must exist. "
                               "The following test(s) are missing: %s" %
@@ -698,120 +702,48 @@ convert = ManifestParser.from_directorie
 
 
 class TestManifest(ManifestParser):
     """
     apply logic to manifests;  this is your integration layer :)
     specific harnesses may subclass from this if they need more logic
     """
 
-    def filter(self, values, tests):
-        """
-        filter on a specific list tag, e.g.:
-        run-if = os == win linux
-        skip-if = os == mac
-        """
-
-        # tags:
-        run_tag = 'run-if'
-        skip_tag = 'skip-if'
-        fail_tag = 'fail-if'
-
-        cache = {}
-
-        def _parse(cond):
-            if '#' in cond:
-                cond = cond[:cond.index('#')]
-            cond = cond.strip()
-            if cond in cache:
-                ret = cache[cond]
-            else:
-                ret = parse(cond, **values)
-                cache[cond] = ret
-            return ret
-
-        # loop over test
-        for test in tests:
-            reason = None # reason to disable
+    def __init__(self, *args, **kwargs):
+        ManifestParser.__init__(self, *args, **kwargs)
+        self.filters = filterlist(DEFAULT_FILTERS)
 
-            # tagged-values to run
-            if run_tag in test:
-                condition = test[run_tag]
-                if not _parse(condition):
-                    reason = '%s: %s' % (run_tag, condition)
-
-            # tagged-values to skip
-            if skip_tag in test:
-                condition = test[skip_tag]
-                if _parse(condition):
-                    reason = '%s: %s' % (skip_tag, condition)
+    def active_tests(self, exists=True, disabled=True, filters=None, **values):
+        """
+        Run all applied filters on the set of tests.
 
-            # mark test as disabled if there's a reason
-            if reason:
-                test.setdefault('disabled', reason)
-
-            # mark test as a fail if so indicated
-            if fail_tag in test:
-                condition = test[fail_tag]
-                if _parse(condition):
-                    test['expected'] = 'fail'
-
-    def active_tests(self, exists=True, disabled=True, options=None, **values):
-        """
-        - exists : return only existing tests
-        - disabled : whether to return disabled tests
-        - options: an optparse or argparse options object, used for subsuites
-        - values : keys and values to filter on (e.g. `os = linux mac`)
+        :param exists: filter out non-existing tests (default True)
+        :param disabled: whether to return disabled tests (default True)
+        :param values: keys and values to filter on (e.g. `os = linux mac`)
+        :param filters: list of filters to apply to the tests
+        :returns: list of test objects that were not filtered out
         """
         tests = [i.copy() for i in self.tests] # shallow copy
 
-        # Conditional subsuites are specified using:
-        #    subsuite = foo,condition
-        # where 'foo' is the subsuite name, and 'condition' is the same type of
-        # condition used for skip-if.  If the condition doesn't evaluate to true,
-        # the subsuite designation will be removed from the test.
-        #
-        # Look for conditional subsuites, and replace them with the subsuite itself
-        # (if the condition is true), or nothing.
-        for test in tests:
-            subsuite = test.get('subsuite', '')
-            if ',' in subsuite:
-                try:
-                    subsuite, condition = subsuite.split(',')
-                except ValueError:
-                    raise ParseError("subsuite condition can't contain commas")
-                # strip any comments from the condition
-                condition = condition.split('#')[0]
-                matched = parse(condition, **values)
-                if matched:
-                    test['subsuite'] = subsuite
-                else:
-                    test['subsuite'] = ''
-
-        # Filter on current subsuite
-        if options:
-            if hasattr(options, 'subsuite') and options.subsuite:
-                tests = [test for test in tests if options.subsuite == test['subsuite']]
-            else:
-                tests = [test for test in tests if not test['subsuite']]
-
-        # mark all tests as passing unless indicated otherwise
+        # mark all tests as passing
         for test in tests:
             test['expected'] = test.get('expected', 'pass')
 
-        # ignore tests that do not exist
+        # make a copy so original doesn't get modified
+        fltrs = self.filters[:]
         if exists:
-            missing = self.check_missing(tests)
-            tests = [test for test in tests if test not in missing]
-
-        # filter by tags
-        self.filter(values, tests)
+            if self.strict:
+                self.check_missing(tests)
+            else:
+                fltrs.append(_exists)
 
-        # ignore disabled tests if specified
         if not disabled:
-            tests = [test for test in tests
-                     if not 'disabled' in test]
+            fltrs.append(enabled)
 
-        # return active tests
-        return tests
+        if filters:
+            fltrs += filters
+
+        for fn in fltrs:
+            tests = fn(tests, values)
+        return list(tests)
 
     def test_paths(self):
         return [test['path'] for test in self.active_tests()]
--- a/testing/mozbase/manifestparser/setup.py
+++ b/testing/mozbase/manifestparser/setup.py
@@ -1,16 +1,16 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this file,
 # You can obtain one at http://mozilla.org/MPL/2.0/.
 
 from setuptools import setup
 
 PACKAGE_NAME = "manifestparser"
-PACKAGE_VERSION = '0.9'
+PACKAGE_VERSION = '1.0'
 
 setup(name=PACKAGE_NAME,
       version=PACKAGE_VERSION,
       description="Library to create and manage test manifests",
       long_description="see http://mozbase.readthedocs.org/",
       classifiers=[], # Get strings from http://pypi.python.org/pypi?%3Aaction=list_classifiers
       keywords='mozilla manifests',
       author='Mozilla Automation and Testing Team',
--- a/testing/mozbase/manifestparser/tests/manifest.ini
+++ b/testing/mozbase/manifestparser/tests/manifest.ini
@@ -1,9 +1,10 @@
 # test manifest for manifestparser
 [test_expressionparser.py]
 [test_manifestparser.py]
 [test_testmanifest.py]
 [test_read_ini.py]
 [test_convert_directory.py]
+[test_filters.py]
 
 [test_convert_symlinks.py]
 disabled = https://bugzilla.mozilla.org/show_bug.cgi?id=920938
new file mode 100644
--- /dev/null
+++ b/testing/mozbase/manifestparser/tests/test_filters.py
@@ -0,0 +1,156 @@
+#!/usr/bin/env python
+
+from copy import deepcopy
+import os
+import unittest
+
+from manifestparser import TestManifest
+from manifestparser.filters import (
+    subsuite,
+    skip_if,
+    run_if,
+    fail_if,
+    enabled,
+    exists,
+    filterlist,
+)
+
+here = os.path.dirname(os.path.abspath(__file__))
+
+
+class FilterList(unittest.TestCase):
+    """Test filterlist datatype"""
+
+    def test_data_model(self):
+        foo = lambda x, y: x
+        bar = lambda x, y: x
+        baz = lambda x, y: x
+        fl = filterlist()
+
+        fl.extend([foo, bar])
+        self.assertEquals(len(fl), 2)
+        self.assertTrue(foo in fl)
+
+        fl.append(baz)
+        self.assertEquals(fl[2], baz)
+
+        fl.remove(baz)
+        self.assertFalse(baz in fl)
+
+        item = fl.pop()
+        self.assertEquals(item, bar)
+
+        self.assertEquals(fl.index(foo), 0)
+
+        del fl[0]
+        self.assertFalse(foo in fl)
+        with self.assertRaises(IndexError):
+            fl[0]
+
+    def test_add_non_callable_to_set(self):
+        fl = filterlist()
+        with self.assertRaises(TypeError):
+            fl.append('foo')
+
+    def test_add_duplicates_to_set(self):
+        foo = lambda x, y: x
+        bar = lambda x, y: x
+        sub = subsuite('foo')
+        fl = filterlist([foo, bar, sub])
+        self.assertEquals(len(fl), 3)
+        self.assertEquals(fl[0], foo)
+
+        with self.assertRaises(ValueError):
+            fl.append(foo)
+
+        with self.assertRaises(ValueError):
+            fl.append(subsuite('bar'))
+
+    def test_filters_run_in_order(self):
+        a = lambda x, y: x
+        b = lambda x, y: x
+        c = lambda x, y: x
+        d = lambda x, y: x
+        e = lambda x, y: x
+        f = lambda x, y: x
+
+        fl = filterlist([a, b])
+        fl.append(c)
+        fl.extend([d, e])
+        fl += [f]
+        self.assertEquals([i for i in fl], [a, b, c, d, e, f])
+
+
+class BuiltinFilters(unittest.TestCase):
+    """Test the built-in filters"""
+
+    tests = (
+        { "name": "test0" },
+        { "name": "test1", "skip-if": "foo == 'bar'" },
+        { "name": "test2", "run-if": "foo == 'bar'" },
+        { "name": "test3", "fail-if": "foo == 'bar'" },
+        { "name": "test4", "disabled": "some reason" },
+        { "name": "test5", "subsuite": "baz" },
+        { "name": "test6", "subsuite": "baz,foo == 'bar'" })
+
+    def test_skip_if(self):
+        tests = deepcopy(self.tests)
+        tests = list(skip_if(tests, {}))
+        self.assertEquals(len(tests), len(self.tests))
+
+        tests = deepcopy(self.tests)
+        tests = list(skip_if(tests, {'foo': 'bar'}))
+        self.assertNotIn(self.tests[1], tests)
+
+    def test_run_if(self):
+        tests = deepcopy(self.tests)
+        tests = list(run_if(tests, {}))
+        self.assertNotIn(self.tests[2], tests)
+
+        tests = deepcopy(self.tests)
+        tests = list(run_if(tests, {'foo': 'bar'}))
+        self.assertEquals(len(tests), len(self.tests))
+
+    def test_fail_if(self):
+        tests = deepcopy(self.tests)
+        tests = list(fail_if(tests, {}))
+        self.assertNotIn('expected', tests[3])
+
+        tests = deepcopy(self.tests)
+        tests = list(fail_if(tests, {'foo': 'bar'}))
+        self.assertEquals(tests[3]['expected'], 'fail')
+
+    def test_enabled(self):
+        tests = deepcopy(self.tests)
+        tests = list(enabled(tests, {}))
+        self.assertNotIn(self.tests[4], tests)
+
+    def test_subsuite(self):
+        sub1 = subsuite()
+        sub2 = subsuite('baz')
+
+        tests = deepcopy(self.tests)
+        tests = list(sub1(tests, {}))
+        self.assertNotIn(self.tests[5], tests)
+        self.assertEquals(tests[-1]['name'], 'test6')
+
+        tests = deepcopy(self.tests)
+        tests = list(sub2(tests, {}))
+        self.assertEquals(len(tests), 1)
+        self.assertIn(self.tests[5], tests)
+
+    def test_subsuite_condition(self):
+        sub1 = subsuite()
+        sub2 = subsuite('baz')
+
+        tests = deepcopy(self.tests)
+
+        tests = list(sub1(tests, {'foo': 'bar'}))
+        self.assertNotIn(self.tests[5], tests)
+        self.assertNotIn(self.tests[6], tests)
+
+        tests = deepcopy(self.tests)
+        tests = list(sub2(tests, {'foo': 'bar'}))
+        self.assertEquals(len(tests), 2)
+        self.assertEquals(tests[0]['name'], 'test5')
+        self.assertEquals(tests[1]['name'], 'test6')
--- a/testing/mozbase/manifestparser/tests/test_testmanifest.py
+++ b/testing/mozbase/manifestparser/tests/test_testmanifest.py
@@ -1,15 +1,17 @@
 #!/usr/bin/env python
 
 import os
 import shutil
 import tempfile
 import unittest
+
 from manifestparser import TestManifest, ParseError
+from manifestparser.filters import subsuite
 
 here = os.path.dirname(os.path.abspath(__file__))
 
 
 class TestTestManifest(unittest.TestCase):
     """Test the Test Manifest"""
 
     def test_testmanifest(self):
@@ -21,21 +23,21 @@ class TestTestManifest(unittest.TestCase
         self.assertEqual([i['name'] for i in manifest.active_tests(os='linux', disabled=False, exists=False)],
                          ['fleem', 'linuxtest'])
 
         # Look for existing tests.  There is only one:
         self.assertEqual([i['name'] for i in manifest.active_tests()],
                          ['fleem'])
 
         # You should be able to expect failures:
-        last_test = manifest.active_tests(exists=False, toolkit='gtk2')[-1]
-        self.assertEqual(last_test['name'], 'linuxtest')
-        self.assertEqual(last_test['expected'], 'pass')
-        last_test = manifest.active_tests(exists=False, toolkit='cocoa')[-1]
-        self.assertEqual(last_test['expected'], 'fail')
+        last = manifest.active_tests(exists=False, toolkit='gtk2')[-1]
+        self.assertEqual(last['name'], 'linuxtest')
+        self.assertEqual(last['expected'], 'pass')
+        last = manifest.active_tests(exists=False, toolkit='cocoa')[-1]
+        self.assertEqual(last['expected'], 'fail')
 
     def test_missing_paths(self):
         """
         Test paths that don't exist raise an exception in strict mode.
         """
         tempdir = tempfile.mkdtemp()
 
         missing_path = os.path.join(here, 'missing-path.ini')
@@ -56,57 +58,55 @@ class TestTestManifest(unittest.TestCase
         self.assertEqual(len(manifest.tests), 8)
         names = [i['name'] for i in manifest.tests]
         self.assertFalse('test_0202_app_launch_apply_update_dirlocked.js' in names)
 
     def test_manifest_subsuites(self):
         """
         test subsuites and conditional subsuites
         """
-        class AttributeDict(dict):
-            def __getattr__(self, attr):
-                return self[attr]
-            def __setattr__(self, attr, value):
-                self[attr] = value
-
         relative_path = os.path.join(here, 'subsuite.ini')
         manifest = TestManifest(manifests=(relative_path,))
         info = {'foo': 'bar'}
-        options = {'subsuite': 'bar'}
 
         # 6 tests total
-        self.assertEquals(len(manifest.active_tests(exists=False, **info)), 6)
+        tests = manifest.active_tests(exists=False, **info)
+        self.assertEquals(len(tests), 6)
 
         # only 3 tests for subsuite bar when foo==bar
-        self.assertEquals(len(manifest.active_tests(exists=False,
-                                                    options=AttributeDict(options),
-                                                    **info)), 3)
+        tests = manifest.active_tests(exists=False,
+                                      filters=[subsuite('bar')],
+                                      **info)
+        self.assertEquals(len(tests), 3)
 
-        options = {'subsuite': 'baz'}
-        other = {'something': 'else'}
         # only 1 test for subsuite baz, regardless of conditions
-        self.assertEquals(len(manifest.active_tests(exists=False,
-                                                    options=AttributeDict(options),
-                                                    **info)), 1)
-        self.assertEquals(len(manifest.active_tests(exists=False,
-                                                    options=AttributeDict(options),
-                                                    **other)), 1)
+        other = {'something': 'else'}
+        tests = manifest.active_tests(exists=False,
+                                      filters=[subsuite('baz')],
+                                      **info)
+        self.assertEquals(len(tests), 1)
+        tests = manifest.active_tests(exists=False,
+                                      filters=[subsuite('baz')],
+                                      **other)
+        self.assertEquals(len(tests), 1)
 
         # 4 tests match when the condition doesn't match (all tests except
         # the unconditional subsuite)
         info = {'foo': 'blah'}
-        options = {'subsuite': None}
-        self.assertEquals(len(manifest.active_tests(exists=False,
-                                                    options=AttributeDict(options),
-                                                    **info)), 5)
+        tests = manifest.active_tests(exists=False,
+                                      filters=[subsuite()],
+                                      **info)
+        self.assertEquals(len(tests), 5)
 
         # test for illegal subsuite value
         manifest.tests[0]['subsuite'] = 'subsuite=bar,foo=="bar",type="nothing"'
-        self.assertRaises(ParseError, manifest.active_tests, exists=False,
-                          options=AttributeDict(options), **info)
+        with self.assertRaises(ParseError):
+            manifest.active_tests(exists=False,
+                                  filters=[subsuite('foo')],
+                                  **info)
 
     def test_none_and_empty_manifest(self):
         """
         Test TestManifest for None and empty manifest, see
         https://bugzilla.mozilla.org/show_bug.cgi?id=1087682
         """
         none_manifest = TestManifest(manifests=None, strict=False)
         self.assertEqual(len(none_manifest.test_paths()), 0)