Merge lp:~zyga/checkbox/no-resource-limits into lp:checkbox

[Zygmunt Krynicki]

[Sylvain Pineau (sylvain-pineau)]

Thanks for the doc update, it really helps understanding what's possible with resources expressions now. I even didn't realized that you could also introduce imported resources, that's even better.

+1

review: Approve

[Sylvain Pineau (sylvain-pineau)]

Attempt to merge into lp:checkbox failed due to conflicts:

text conflict in plainbox/docs/changelog.rst

[lp:~zyga/checkbox/no-resource-limits]

4030. By Zygmunt Krynicki on 2015-09-25

plainbox: add support for multiple resource requirements

This patch essentially allows a requirement program to use more than one
resource. In practice this unlocks a host of capabilities where we can
freely combine manifests, syfs data, udev data and anything else that
makes sense in a given context and turn that into a resource expression.

The primary consumer of this is the TPM provider that has some
non-trivial logic in requirement programs. This logic allows us to
construct a flat lost of jobs to run and thanks to the more complicated
expressions, get them to effectively implement and if-the-else
navigation among jobs so that regardless of what state the TPM hardware
is at the beginning of the test, we can correctly navigate the user
through the testing process.

Technically the ResourceNodeVisitor now collects a list of names as it
traverses the program AST. The MultipleResourcesReferenced exception is
gone entirely. The ResourceExpression class has an API change, as
.resource_id becomes a .resource_id_list. There is a similar change in
_resource_alias_list but this API is private.

There are surprisingly few users of resource_id. Most of the work was in
the controller module (ctrl.py) where we now look at all the resources
to convert them to dependencies. There are a few odd test cases for job
definition units and one utility script that generates .dot diagrams.

The most important note is, that multiple resources should be
discouraged for resource jobs that produce many resource records. The
complexity of evaluating a resource expression over N resources is the
product of the number of resource records in each resource. It's safe to
say that it works best for resource jobs with just one record.

At a later stage we may implement some simple low-hanging optimizations
where resource computation like res.attr == "value" can be converted
from O(N) to O(1) per evaluation at an O(N) one-time cost.

Signed-off-by: Zygmunt Krynicki <email address hidden>

=== modified file 'plainbox/docs/changelog.rst'
--- plainbox/docs/changelog.rst	2015-09-25 07:28:31 +0000
+++ plainbox/docs/changelog.rst	2015-09-25 08:41:19 +0000
@@ -23,6 +23,20 @@
   attachment jobs will automatically pull in their log-generating cousins and
   will run at the right time no matter what happens.
 
+* Plainbox now allows more than one resource object to be used in a resource
+  expression. This can be used to construct resource expressions that combine
+  facts from multiple sources (e.g. the manifest resource with something else).
+
+  As an **important** implementation limitation please remember that the
+  complexity of such resource programs is proportional to the product of the
+  number of resource objects associated with each resource in an expression.
+  In practice it is not advised to use resource objects with more than a few
+  resource records associated with them. This is just an implementation detail
+  that can be lifted in subsequent versions.
+
+  Examples of the usage of this feature can be found in the TPM (Trusted
+  Platform Module) provider.
+
 .. _version_0_23:
 
 Plainbox 0.23 (QA Testing)

=== modified file 'plainbox/docs/dev/resources.rst'
--- plainbox/docs/dev/resources.rst	2014-09-16 12:53:41 +0000
+++ plainbox/docs/dev/resources.rst	2015-09-25 08:41:19 +0000
@@ -69,14 +69,12 @@
 
 Anything else is rejected as an invalid resource expression.
 
-In addition to that, each resource expression must use exactly one variable,
+In addition to that, each resource expression must use at least one variable,
 which must be used like an object with attributes. The name of that variable
-must correspond to the name of the job that generates resources. Attempts to
-use more than one variable or to not use any variables are detected early and
-rejected as invalid resource expressions.
-
-The name of the variable determines which resource group to use. It must match
-the name of the job that generates such resources.
+must correspond to the name of the job that generates resources. You can use
+the ``imports`` field (at a job definition level) to rename a resource job to
+be compatible with the identifier syntax. It can also be used to refer to
+resources from another namespace.
 
 In the examples elsewhere in this page the  ``package`` resources are generated
 by the ``package`` job. Plainbox uses this to know which resources to try but
@@ -112,41 +110,32 @@
 non-exhaustive, it only contains issues that we came across found not to work
 in practice.
 
-Exactly one variable per expression
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-Each resource expression must refer to exactly one variable. This is a side
-effect of the way the evaluator works. It basically bind one object (a
-particular resource) to that variable and evaluates the expression.
-
-The expression parser / syntax analyzer identifies expressions with this
-problem early and rejects them with an appropriate error message. Here are
-some examples of hypothetical expressions that exhibit this problem.
-
-"I want to have mplayer and an audio device so that I can play some sounds"::
-
-    device.category == "AUDIO" and package.name == "mplayer"
-
-To work around this, split the expression to two separate expressions. The
-evaluator will put an implicit ``and`` between them and it will do exactly what
-you intended::
-
-    device.category == "AUDIO"
-    package.name == "mplayer"
-
-"I want to always run this test"::
-
-    True
-
-To work around this, simply remove the requirement program entirely!
-
-"I want to never run this test"::
-
-    False
-
-To work around this remove this job from the selection. You may also use a
-special resource that produces one constant value, and check that it is equal
-to something different.
+Joins are not optimized
+^^^^^^^^^^^^^^^^^^^^^^^
+
+Starting with plainbox 0.24, a resource expression can use more than one
+resource object (resource job) at the same time. This allows the use of joins
+as the whole expression is evaluated over the cartesian product of all the
+resource records. This operation is not optimized, you can think of it as a
+JOIN that is performed on a database without any indices.
+
+Let's look at a practical example::
+
+    package.name == desired_package.name
+
+Here, two resource jobs would run. The classic *package* resource (that
+produces, typically, a great number of resource records, one for each package
+installed on the system) and a hypothetical *desired_package* resource (for
+this example let's pretend that it is a simple constant resource that just
+contains one object). Here, this operation is not any worse than before because
+``size(desired_package) * size(package)`` is not any larger. If, however,
+*desired_package* was on the same order as *package* (approximately a thousand
+resource objects). Then the computational cost of evaluating that expression
+would be quadratic.
+
+In general, the cost, assuming all resources have the same order, is
+exponential with the number of distinct resource jobs referenced by the
+expression.
 
 Exactly one resource bound to a variable at once
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

=== modified file 'plainbox/plainbox/impl/commands/inv_special.py'
--- plainbox/plainbox/impl/commands/inv_special.py	2014-11-18 17:20:58 +0000
+++ plainbox/plainbox/impl/commands/inv_special.py	2015-09-25 08:41:19 +0000
@@ -103,9 +103,10 @@
             prog = job.get_resource_program()
             if ns.dot_resources and prog is not None:
                 for expression in prog.expression_list:
-                    print('\t"{}" [shape=ellipse,color=blue];'.format(
-                        expression.resource_id))
-                    print('\t"{}" -> "{}" [style=dashed, label="{}"];'.format(
-                        job.id, expression.resource_id,
-                        expression.text.replace('"', "'")))
+                    for resource_id in expression.resource_id_list:
+                        print('\t"{}" [shape=ellipse,color=blue];'.format(
+                            resource_id))
+                        print('\t"{}" -> "{}" [style=dashed, label="{}"];'.format(
+                            job.id, resource_id,
+                            expression.text.replace('"', "'")))
         print("}")

=== modified file 'plainbox/plainbox/impl/ctrl.py'
--- plainbox/plainbox/impl/ctrl.py	2015-09-25 07:28:31 +0000
+++ plainbox/plainbox/impl/ctrl.py	2015-09-25 08:41:19 +0000
@@ -161,36 +161,41 @@
             try:
                 prog.evaluate_or_raise(session_state.resource_map)
             except ExpressionCannotEvaluateError as exc:
-                # Lookup the related job (the job that provides the
-                # resources needed by the expression that cannot be
-                # evaluated)
-                related_job = session_state.job_state_map[
-                    exc.expression.resource_id].job
-                # Add A PENDING_RESOURCE inhibitor as we are unable to
-                # determine if the resource requirement is met or not. This
-                # can happen if the resource job did not ran for any reason
-                # (it can either be prevented from running by normal means
-                # or simply be on the run_list but just was not executed
-                # yet).
-                inhibitor = JobReadinessInhibitor(
-                    cause=InhibitionCause.PENDING_RESOURCE,
-                    related_job=related_job,
-                    related_expression=exc.expression)
-                inhibitors.append(inhibitor)
+                for resource_id in exc.expression.resource_id_list:
+                    if session_state.job_state_map[resource_id].result.outcome == 'pass':
+                        continue
+                    # Lookup the related job (the job that provides the
+                    # resources needed by the expression that cannot be
+                    # evaluated)
+                    related_job = session_state.job_state_map[resource_id].job
+                    # Add A PENDING_RESOURCE inhibitor as we are unable to
+                    # determine if the resource requirement is met or not. This
+                    # can happen if the resource job did not ran for any reason
+                    # (it can either be prevented from running by normal means
+                    # or simply be on the run_list but just was not executed
+                    # yet).
+                    inhibitor = JobReadinessInhibitor(
+                        cause=InhibitionCause.PENDING_RESOURCE,
+                        related_job=related_job,
+                        related_expression=exc.expression)
+                    inhibitors.append(inhibitor)
             except ExpressionFailedError as exc:
-                # Lookup the related job (the job that provides the
-                # resources needed by the expression that failed)
-                related_job = session_state.job_state_map[
-                    exc.expression.resource_id].job
-                # Add a FAILED_RESOURCE inhibitor as we have all the data
-                # to run the requirement program but it simply returns a
-                # non-True value. This typically indicates a missing
-                # software package or necessary hardware.
-                inhibitor = JobReadinessInhibitor(
-                    cause=InhibitionCause.FAILED_RESOURCE,
-                    related_job=related_job,
-                    related_expression=exc.expression)
-                inhibitors.append(inhibitor)
+                # When expressions fail then all the associated resources are
+                # marked as failed since we don't want to get into the analysis
+                # of logic expressions to know any "better".
+                for resource_id in exc.expression.resource_id_list:
+                    # Lookup the related job (the job that provides the
+                    # resources needed by the expression that failed)
+                    related_job = session_state.job_state_map[resource_id].job
+                    # Add a FAILED_RESOURCE inhibitor as we have all the data
+                    # to run the requirement program but it simply returns a
+                    # non-True value. This typically indicates a missing
+                    # software package or necessary hardware.
+                    inhibitor = JobReadinessInhibitor(
+                        cause=InhibitionCause.FAILED_RESOURCE,
+                        related_job=related_job,
+                        related_expression=exc.expression)
+                    inhibitors.append(inhibitor)
         # Check if all job dependencies ran successfully
         for dep_id in sorted(job.get_direct_dependencies()):
             dep_job_state = session_state.job_state_map[dep_id]

=== modified file 'plainbox/plainbox/impl/resource.py'
--- plainbox/plainbox/impl/resource.py	2015-05-18 16:59:31 +0000
+++ plainbox/plainbox/impl/resource.py	2015-09-25 08:41:19 +0000
@@ -27,6 +27,7 @@
 """
 
 import ast
+import itertools
 import logging
 
 from plainbox.i18n import gettext as _
@@ -67,9 +68,13 @@
     enough data to provide rich and meaningful error messages to the operator.
     """
 
+    def __init__(self, expression, resource_id):
+        self.expression = expression
+        self.resource_id = resource_id
+
     def __str__(self):
         return _("expression {!r} needs unavailable resource {!r}").format(
-            self.expression.text, self.expression.resource_id)
+            self.expression.text, self.resource_id)
 
 
 class Resource:
@@ -220,8 +225,11 @@
         """
         A set() of resource ids that are needed to evaluate this program
         """
-        return set((expression.resource_id
-                    for expression in self._expression_list))
+        ids = set()
+        for expression in self._expression_list:
+            for resource_id in expression.resource_id_list:
+                ids.add(resource_id)
+        return ids
 
     def evaluate_or_raise(self, resource_map):
         """
@@ -238,12 +246,16 @@
         """
         # First check if we have all required resources
         for expression in self._expression_list:
-            if expression.resource_id not in resource_map:
-                raise ExpressionCannotEvaluateError(expression)
+            for resource_id in expression.resource_id_list:
+                if resource_id not in resource_map:
+                    raise ExpressionCannotEvaluateError(
+                        expression, resource_id)
         # Then evaluate all expressions
         for expression in self._expression_list:
-            result = expression.evaluate(
-                resource_map[expression.resource_id])
+            result = expression.evaluate(*[
+                resource_map[resource_id]
+                for resource_id in expression.resource_id_list
+            ])
             if not result:
                 raise ExpressionFailedError(expression)
         return True
@@ -378,16 +390,24 @@
 
     def __init__(self):
         """
-        Initialize a ResourceNodeVisitor with empty set of ids_seen
+        Initialize a ResourceNodeVisitor with empty trace of seen identifiers
         """
-        self._ids_seen = set()
+        self._ids_seen_set = set()
+        self._ids_seen_list = []
 
     @property
-    def ids_seen(self):
+    def ids_seen_set(self):
         """
         set() of ast.Name().id values seen
         """
-        return self._ids_seen
+        return self._ids_seen_set
+
+    @property
+    def ids_seen_list(self):
+        """
+        list() of ast.Name().id values seen
+        """
+        return self._ids_seen_list
 
     def visit_Name(self, node):
         """
@@ -397,7 +417,9 @@
         ast.Name(). It records the node identifier and calls _check_node()
         """
         self._check_node(node)
-        self._ids_seen.add(node.id)
+        if node.id not in self._ids_seen_set:
+            self._ids_seen_set.add(node.id)
+            self._ids_seen_list.append(node.id)
 
     def visit_Call(self, node):
         """
@@ -470,15 +492,6 @@
         return _("expression did not reference any resources")
 
 
-class MultipleResourcesReferenced(ResourceProgramError):
-    """
-    Exception raised when an expression references multiple resources.
-    """
-
-    def __str__(self):
-        return _("expression referenced multiple resources")
-
-
 class ResourceSyntaxError(ResourceProgramError):
 
     def __str__(self):
@@ -502,20 +515,22 @@
         May raise ResourceProgramError
         """
         self._implicit_namespace = implicit_namespace
-        self._resource_alias = self._analyze(text)
+        self._resource_alias_list = self._analyze(text)
+        self._resource_id_list = []
         if imports is None:
             imports = ()
         # Respect any import statements.
         # They always take priority over anything we may know locally
-        for imported_resource_id, imported_alias in imports:
-            if imported_alias == self._resource_alias:
-                self._resource_id = imported_resource_id
-                break
-        else:
-            self._resource_id = self._resource_alias
+        for resource_alias in self._resource_alias_list:
+            for imported_resource_id, imported_alias in imports:
+                if imported_alias == resource_alias:
+                    self._resource_id_list.append(imported_resource_id)
+                    break
+            else:
+                self._resource_id_list.append(resource_alias)
         self._text = text
         self._lambda = eval("lambda {}: {}".format(
-            self._resource_alias, self._text))
+            ', '.join(self._resource_alias_list), self._text))
 
     def __str__(self):
         return self._text
@@ -541,7 +556,7 @@
         return self._text
 
     @property
-    def resource_id(self):
+    def resource_id_list(self):
         """
         The id of the resource this expression depends on
 
@@ -549,13 +564,15 @@
         valid python identifier and it is always (ideally) a fully-qualified
         job identifier.
         """
-        if "::" not in self._resource_id and self._implicit_namespace:
-            return "{}::{}".format(self._implicit_namespace, self._resource_id)
-        else:
-            return self._resource_id
+        return [
+            "{}::{}".format(self._implicit_namespace, resource_id)
+            if "::" not in resource_id and self._implicit_namespace
+            else resource_id
+            for resource_id in self._resource_id_list
+        ]
 
     @property
-    def resource_alias(self):
+    def resource_alias_list(self):
         """
         The alias of the resource object this expression operates on
 
@@ -563,7 +580,7 @@
         python identifier. The alias is either the partial identifier of the
         resource job or an arbitrary identifier, as used by the job definition.
         """
-        return self._resource_alias
+        return self._resource_alias_list
 
     @property
     def implicit_namespace(self):
@@ -572,7 +589,7 @@
         """
         return self._implicit_namespace
 
-    def evaluate(self, resource_list):
+    def evaluate(self, *resource_list_list):
         """
         Evaluate the expression against a list of resources
 
@@ -580,13 +597,16 @@
         id in the expression. The return value is True if any of the attempts
         return a true value, otherwise the result is False.
         """
+        for resource_list in resource_list_list:
+            for resource in resource_list:
+                if not isinstance(resource, Resource):
+                    raise TypeError(
+                        "Each resource must be a Resource instance")
         # Try each resource in sequence.
-        for resource in resource_list:
-            if not isinstance(resource, Resource):
-                raise TypeError("Each resource must be a Resource instance")
+        for resource_pack in itertools.product(*resource_list_list):
             # Attempt to evaluate the code with the current resource
             try:
-                result = self._lambda(resource)
+                result = self._lambda(*resource_pack)
             except Exception as exc:
                 # Treat any exception as a non-fatal error
                 #
@@ -594,7 +614,8 @@
                 # why they happen.  We could do deeper validation this way.
                 logger.debug(
                     _("Exception in requirement expression %r (with %s=%r):"
-                      " %r"), self._text, self._resource_id, resource, exc)
+                      " %r"),
+                    self._text, self._resource_id_list, resource, exc)
                 continue
             # Treat any true result as a success
             if result:
@@ -622,12 +643,10 @@
         visitor = ResourceNodeVisitor()
         visitor.visit(node)
         # Bail if the expression is not using exactly one resource id
-        if len(visitor.ids_seen) == 0:
+        if len(visitor.ids_seen_list) == 0:
             raise NoResourcesReferenced()
-        elif len(visitor.ids_seen) == 1:
-            return list(visitor.ids_seen)[0]
         else:
-            raise MultipleResourcesReferenced()
+            return list(visitor.ids_seen_list)
 
 
 def parse_imports_stmt(imports):

=== modified file 'plainbox/plainbox/impl/test_resource.py'
--- plainbox/plainbox/impl/test_resource.py	2014-08-08 16:35:49 +0000
+++ plainbox/plainbox/impl/test_resource.py	2015-09-25 08:41:19 +0000
@@ -31,7 +31,6 @@
 from plainbox.impl.resource import ExpressionCannotEvaluateError
 from plainbox.impl.resource import ExpressionFailedError
 from plainbox.impl.resource import FakeResource
-from plainbox.impl.resource import MultipleResourcesReferenced
 from plainbox.impl.resource import NoResourcesReferenced
 from plainbox.impl.resource import Resource
 from plainbox.impl.resource import ResourceExpression
@@ -59,7 +58,7 @@
 
     def test_smoke(self):
         expression = ResourceExpression('resource.attr == "value"')
-        exc = ExpressionCannotEvaluateError(expression)
+        exc = ExpressionCannotEvaluateError(expression, 'resource')
         self.assertIs(exc.expression, expression)
         self.assertEqual(str(exc), (
             "expression 'resource.attr == \"value\"' needs unavailable"
@@ -183,11 +182,6 @@
 
 class ResourceProgramErrorTests(TestCase):
 
-    def test_multiple(self):
-        exc = MultipleResourcesReferenced()
-        self.assertEqual(
-            str(exc), "expression referenced multiple resources")
-
     def test_none(self):
         exc = NoResourcesReferenced()
         self.assertEqual(
@@ -209,13 +203,15 @@
 
     def test_smoke(self):
         visitor = ResourceNodeVisitor()
-        self.assertEqual(visitor.ids_seen, set())
+        self.assertEqual(visitor.ids_seen_set, set())
+        self.assertEqual(visitor.ids_seen_list, [])
 
     def test_ids_seen(self):
         visitor = ResourceNodeVisitor()
         node = ast.parse("package.name == 'fwts' and package.version == '1.2'")
         visitor.visit(node)
-        self.assertEqual(visitor.ids_seen, {'package'})
+        self.assertEqual(visitor.ids_seen_set, {'package'})
+        self.assertEqual(visitor.ids_seen_list, ['package'])
 
     def test_name_assignment_disallowed(self):
         visitor = ResourceNodeVisitor()
@@ -289,45 +285,49 @@
         text = "package.name == 'fwts'"
         expr = ResourceExpression(text)
         self.assertEqual(expr.text, text)
-        self.assertEqual(expr.resource_id, "package")
+        self.assertEqual(expr.resource_id_list, ["package"])
         self.assertEqual(expr.implicit_namespace, None)
 
     def test_namespace_support(self):
         text = "package.name == 'fwts'"
         expr = ResourceExpression(text, "2014.com.canonical")
         self.assertEqual(expr.text, text)
-        self.assertEqual(expr.resource_id, "2014.com.canonical::package")
+        self.assertEqual(expr.resource_id_list,
+                         ["2014.com.canonical::package"])
         self.assertEqual(expr.implicit_namespace, "2014.com.canonical")
 
     def test_imports_support(self):
         text = "package.name == 'fwts'"
         expr1 = ResourceExpression(text, "2014.com.example")
         self.assertEqual(expr1.text, text)
-        self.assertEqual(expr1.resource_id, "2014.com.example::package")
+        self.assertEqual(expr1.resource_id_list, ["2014.com.example::package"])
         self.assertEqual(expr1.implicit_namespace, "2014.com.example")
         expr2 = ResourceExpression(text, "2014.com.example", imports=())
         self.assertEqual(expr2.text, text)
-        self.assertEqual(expr2.resource_id, "2014.com.example::package")
+        self.assertEqual(expr2.resource_id_list, ["2014.com.example::package"])
         self.assertEqual(expr2.implicit_namespace, "2014.com.example")
         expr3 = ResourceExpression(
             text, "2014.com.example", imports=[
                 ('2014.com.canonical::package', 'package')])
         self.assertEqual(expr3.text, text)
-        self.assertEqual(expr3.resource_id, "2014.com.canonical::package")
+        self.assertEqual(expr3.resource_id_list,
+                         ["2014.com.canonical::package"])
         self.assertEqual(expr3.implicit_namespace, "2014.com.example")
 
     def test_smoke_bad(self):
         self.assertRaises(ResourceSyntaxError, ResourceExpression, "barf'")
         self.assertRaises(CodeNotAllowed, ResourceExpression, "a = 5")
         self.assertRaises(NoResourcesReferenced, ResourceExpression, "5 < 10")
-        self.assertRaises(MultipleResourcesReferenced,
-                          ResourceExpression, "a.foo == 1 and b.bar == 2")
+
+    def test_multiple_resources(self):
+        expr = ResourceExpression("a.foo == 1 and b.bar == 2")
+        self.assertEqual(expr.resource_id_list, ["a", "b"])
 
     def test_evaluate_no_namespaces(self):
         self.assertFalse(ResourceExpression("whatever").evaluate([]))
 
     def test_evaluate_normal(self):
-        # NOTE: the actual expr.resource_id is irrelevant for this test
+        # NOTE: the actual expr.resource_id_list is irrelevant for this test
         expr = ResourceExpression("obj.a == 2")
         self.assertTrue(
             expr.evaluate([
@@ -340,7 +340,7 @@
                 Resource({'a': 1}), Resource({'a': 3})]))
 
     def test_evaluate_exception(self):
-        # NOTE: the actual expr.resource_id is irrelevant for this test
+        # NOTE: the actual expr.resource_id_list is irrelevant for this test
         expr = ResourceExpression("obj.a == 2")
         self.assertFalse(expr.evaluate([Resource()]))
 
@@ -362,12 +362,12 @@
         self.assertEqual(len(self.prog.expression_list), 2)
         self.assertEqual(self.prog.expression_list[0].text,
                          "package.name == 'fwts'")
-        self.assertEqual(self.prog.expression_list[0].resource_id,
-                         "package")
+        self.assertEqual(self.prog.expression_list[0].resource_id_list,
+                         ["package"])
         self.assertEqual(self.prog.expression_list[1].text,
                          "platform.arch in ('i386', 'amd64')")
-        self.assertEqual(self.prog.expression_list[1].resource_id,
-                         "platform")
+        self.assertEqual(self.prog.expression_list[1].resource_id_list,
+                         ["platform"])
 
     def test_required_resources(self):
         self.assertEqual(self.prog.required_resources,

=== modified file 'plainbox/plainbox/impl/unit/test_job.py'
--- plainbox/plainbox/impl/unit/test_job.py	2015-09-25 08:10:57 +0000
+++ plainbox/plainbox/impl/unit/test_job.py	2015-09-25 08:41:19 +0000
@@ -1110,13 +1110,6 @@
         observed = job.get_resource_dependencies()
         self.assertEqual(expected, observed)
 
-    def test_resource_parsing_broken(self):
-        job = JobDefinition({
-            'id': 'id',
-            'plugin': 'plugin',
-            'requires': "foo.bar == bar"})
-        self.assertRaises(Exception, job.get_resource_dependencies)
-
     def test_checksum_smoke(self):
         job1 = JobDefinition({
             'id': 'id',
@@ -1410,5 +1403,5 @@
                 "manifest.has_thunderbolt == 'True'\n"),
         }, provider=provider)
         prog = job.get_resource_program()
-        self.assertEqual(prog.expression_list[-1].resource_id,
-                         '2013.com.canonical.plainbox::manifest')
+        self.assertEqual(prog.expression_list[-1].resource_id_list,
+                         ['2013.com.canonical.plainbox::manifest'])