Merge lp:~sylvain-pineau/checkbox/nested_testplans into lp:checkbox

[Sylvain Pineau]

The nested test plans feature is ready for review.
Instead of a CEP, I'll work on updating the documentation in a separate MR

To use and test this new field in your testplans just add:

nested_part:
    testplan_id # if it belongs to the same namespace
    2013.foo.bar::second_testplan_id # to include jobs from another project namespace

Nota: jobs will follow the nested parts ordering but all "include:" jobs of the "master" testplan if this list is non empty will come first.

Tested with both checkbox-cli and checkbox-converged, additional tests ( the ones coming from the nested parts) are properly included.

[Paul Larson (pwlars)]

Looks good, and unit tests pass. +1

review: Approve

[Sylvain Pineau (sylvain-pineau)]

Let's land it, thanks for the review.

review: Approve

=== modified file 'plainbox/plainbox/impl/commands/inv_special.py'
--- plainbox/plainbox/impl/commands/inv_special.py	2015-09-25 09:13:53 +0000
+++ plainbox/plainbox/impl/commands/inv_special.py	2016-06-09 20:31:24 +0000
@@ -57,7 +57,11 @@
         # specified just operate on the whole set. The ns.special check
         # prevents people starting plainbox from accidentally running _all_
         # jobs without prompting.
-        if ns.special is not None and not ns.include_pattern_list:
+        if (
+            ns.special is not None and
+            not ns.include_pattern_list and
+            not ns.test_plan
+        ):
             matching_job_list = job_list
         return matching_job_list
 

=== modified file 'plainbox/plainbox/impl/session/assistant.py'
--- plainbox/plainbox/impl/session/assistant.py	2016-05-24 19:38:24 +0000
+++ plainbox/plainbox/impl/session/assistant.py	2016-06-09 20:31:24 +0000
@@ -1059,11 +1059,9 @@
         """
         UsageExpectation.of(self).enforce()
         test_plan = self._manager.test_plans[0]
-        potential_job_list = select_jobs(
-            self._context.state.job_list,
-            [test_plan.get_qualifier(), test_plan.get_mandatory_qualifier()])
         return list(set(
-            test_plan.get_effective_category_map(potential_job_list).values()))
+            test_plan.get_effective_category_map(
+                self._context.state.run_list).values()))
 
     @raises(UnexpectedMethodCall)
     def get_mandatory_jobs(self) -> 'Iterable[str]':
@@ -1432,7 +1430,7 @@
         UsageExpectation.of(self).enforce()
         exporter = self._manager.create_exporter(exporter_id, option_list,
                                                  strict=False)
-        
+
         # LP:1585326 maintain isoformat but removing ':' chars that cause
         # issues when copying files.
         isoformat = "%Y-%m-%dT%H.%M.%S.%f"

=== modified file 'plainbox/plainbox/impl/session/manager.py'
--- plainbox/plainbox/impl/session/manager.py	2016-04-29 08:36:20 +0000
+++ plainbox/plainbox/impl/session/manager.py	2016-06-09 20:31:24 +0000
@@ -550,8 +550,8 @@
             repo = SessionStorageRepository(tmp)
             if provider_list is None:
                 provider_list = get_providers()
+            try:
                 manager = cls.create(repo=repo)
-            try:
                 manager.add_local_device_context()
                 device_context = manager.default_device_context
                 for provider in provider_list:

=== modified file 'plainbox/plainbox/impl/unit/test_testplan.py'
--- plainbox/plainbox/impl/unit/test_testplan.py	2015-09-25 06:48:54 +0000
+++ plainbox/plainbox/impl/unit/test_testplan.py	2016-06-09 20:31:24 +0000
@@ -33,6 +33,7 @@
 from plainbox.impl.secure.qualifiers import OperatorMatcher
 from plainbox.impl.secure.qualifiers import PatternMatcher
 from plainbox.impl.unit.testplan import TestPlanUnit
+from plainbox.impl.unit.testplan import TestPlanUnitSupport
 from plainbox.vendor import mock
 
 
@@ -133,7 +134,7 @@
 
     def test_category_override__normal(self):
         unit = TestPlanUnit({
-            'category-overrides': 'value',
+            'category_overrides': 'value',
         }, provider=self.provider)
         self.assertEqual(unit.category_overrides, 'value')
 
@@ -385,3 +386,126 @@
         }, provider=self.provider)
         self.assertEqual(unit.get_bootstrap_job_ids(),
                          set(['ns::Foo', 'ns::Bar']))
+
+
+class TestNestedTestPlan(TestCase):
+
+    def setUp(self):
+        self.provider1 = mock.Mock(name='provider1', spec_set=IProvider1)
+        self.provider1.namespace = 'ns1'
+        self.provider2 = mock.Mock(name='provider2', spec_set=IProvider1)
+        self.provider2.namespace = 'ns2'
+        self.tp1 = TestPlanUnit({
+            'id': 'tp1',
+            'unit': 'test-plan',
+            'name': 'An example test plan 1',
+            'include': 'Foo',
+            'nested_part': 'tp2'
+        }, provider=self.provider1)
+        self.tp2 = TestPlanUnit({
+            'id': 'tp2',
+            'unit': 'test-plan',
+            'name': 'An example test plan 2',
+            'include': 'Bar',
+            'mandatory_include': 'Baz',
+            'bootstrap_include': 'Qux'
+        }, provider=self.provider1)
+        self.tp3 = TestPlanUnit({
+            'id': 'tp3',
+            'unit': 'test-plan',
+            'name': 'An example test plan 3',
+            'include': '# nothing\n',
+            'nested_part': 'tp2',
+            'certification_status_overrides': 'apply blocker to Bar'
+        }, provider=self.provider1)
+        self.tp4 = TestPlanUnit({
+            'id': 'tp4',
+            'unit': 'test-plan',
+            'name': 'An example test plan 4',
+            'include': '# nothing\n',
+            'nested_part': (
+                'tp2\n'
+                'tp5\n'
+                )
+        }, provider=self.provider1)
+        self.tp5 = TestPlanUnit({
+            'id': 'tp5',
+            'unit': 'test-plan',
+            'name': 'An example test plan 5',
+            'include': 'Baz2',
+        }, provider=self.provider1)
+        self.tp6 = TestPlanUnit({
+            'id': 'tp6',
+            'unit': 'test-plan',
+            'name': 'An example test plan 6',
+            'include': 'Foo',
+            'nested_part': 'ns2::tp7'
+        }, provider=self.provider1)
+        self.tp7 = TestPlanUnit({
+            'id': 'tp7',
+            'unit': 'test-plan',
+            'name': 'An example test plan 7',
+            'include': 'Bar'
+        }, provider=self.provider2)
+        self.provider1.unit_list = []
+        self.provider2.unit_list = [self.tp7]
+        self.tp7.provider_list = [self.provider1, self.provider2]
+        for i in range(1, 7):
+            tp = getattr(self, 'tp{}'.format(i))
+            tp.provider_list = [self.provider1, self.provider2]
+            self.provider1.unit_list.append(tp)
+
+    def test_nested_tesplan__qualifiers(self):
+        qual_list = self.tp1.get_qualifier().get_primitive_qualifiers()
+        mandatory_qual_list = \
+            self.tp1.get_mandatory_qualifier().get_primitive_qualifiers()
+        bootstrap_qual_list = \
+            self.tp1.get_bootstrap_qualifier().get_primitive_qualifiers()
+        self.assertEqual(qual_list[0].field, 'id')
+        self.assertIsInstance(qual_list[0].matcher, OperatorMatcher)
+        self.assertEqual(qual_list[0].matcher.value, 'ns1::Foo')
+        self.assertEqual(qual_list[0].inclusive, True)
+        self.assertEqual(qual_list[1].field, 'id')
+        self.assertIsInstance(qual_list[1].matcher, OperatorMatcher)
+        self.assertEqual(qual_list[1].matcher.value, 'ns1::Qux')
+        self.assertEqual(qual_list[1].inclusive, False)
+        self.assertEqual(qual_list[2].field, 'id')
+        self.assertIsInstance(qual_list[2].matcher, OperatorMatcher)
+        self.assertEqual(qual_list[2].matcher.value, 'ns1::Bar')
+        self.assertEqual(qual_list[2].inclusive, True)
+        self.assertEqual(mandatory_qual_list[0].field, 'id')
+        self.assertIsInstance(mandatory_qual_list[0].matcher, OperatorMatcher)
+        self.assertEqual(mandatory_qual_list[0].matcher.value, 'ns1::Baz')
+        self.assertEqual(mandatory_qual_list[0].inclusive, True)
+        self.assertEqual(bootstrap_qual_list[0].field, 'id')
+        self.assertIsInstance(bootstrap_qual_list[0].matcher, OperatorMatcher)
+        self.assertEqual(bootstrap_qual_list[0].matcher.value, 'ns1::Qux')
+        self.assertEqual(bootstrap_qual_list[0].inclusive, True)
+
+    def test_nested_tesplan__certification_status_override(self):
+        support = TestPlanUnitSupport(self.tp3)
+        self.assertEqual(
+            support.override_list,
+            [('^ns1::Bar$', [('certification_status', 'blocker')])])
+
+    def test_nested_tesplan__multiple_parts(self):
+        qual_list = self.tp4.get_qualifier().get_primitive_qualifiers()
+        self.assertEqual(qual_list[1].field, 'id')
+        self.assertIsInstance(qual_list[1].matcher, OperatorMatcher)
+        self.assertEqual(qual_list[1].matcher.value, 'ns1::Bar')
+        self.assertEqual(qual_list[1].inclusive, True)
+        self.assertEqual(qual_list[3].field, 'id')
+        self.assertIsInstance(qual_list[3].matcher, OperatorMatcher)
+        self.assertEqual(qual_list[3].matcher.value, 'ns1::Baz2')
+        self.assertEqual(qual_list[3].inclusive, True)
+
+    def test_nested_tesplan__multiple_namespaces(self):
+        qual_list = self.tp6.get_qualifier().get_primitive_qualifiers()
+        self.assertEqual(qual_list[0].field, 'id')
+        self.assertIsInstance(qual_list[0].matcher, OperatorMatcher)
+        self.assertEqual(qual_list[0].matcher.value, 'ns1::Foo')
+        self.assertEqual(qual_list[0].inclusive, True)
+        self.assertEqual(qual_list[1].field, 'id')
+        self.assertIsInstance(qual_list[1].matcher, OperatorMatcher)
+        self.assertEqual(qual_list[1].matcher.value, 'ns2::Bar')
+        self.assertEqual(qual_list[1].inclusive, True)

=== modified file 'plainbox/plainbox/impl/unit/testplan.py'
--- plainbox/plainbox/impl/unit/testplan.py	2016-05-20 13:54:59 +0000
+++ plainbox/plainbox/impl/unit/testplan.py	2016-06-09 20:31:24 +0000
@@ -24,6 +24,7 @@
 import logging
 import operator
 import re
+from functools import lru_cache
 
 from plainbox.i18n import gettext as _
 from plainbox.impl.secure.qualifiers import CompositeQualifier
@@ -237,16 +238,33 @@
         return self.get_record_value('exclude')
 
     @property
+    def nested_part(self):
+        return self.get_record_value('nested_part')
+
+    @property
     def icon(self):
         return self.get_record_value('icon')
 
     @property
     def category_overrides(self):
-        return self.get_record_value('category-overrides')
+        return self.get_record_value('category_overrides')
 
     @property
     def certification_status_overrides(self):
-        return self.get_record_value('certification-status-overrides')
+        return self.get_record_value('certification_status_overrides')
+
+    @property
+    def provider_list(self):
+        """
+        List of provider to used when calling get_throwaway_manager().
+        Meant to be used by unit tests only.
+        """
+        if hasattr(self, "_provider_list"):
+            return self._provider_list
+
+    @provider_list.setter
+    def provider_list(self, value):
+        self._provider_list = value
 
     @property
     def estimated_duration(self):
@@ -299,21 +317,51 @@
     def get_bootstrap_job_ids(self):
         """Compute and return a set of job ids from bootstrap_include field."""
         job_ids = set()
-        if self.bootstrap_include is None:
-            return job_ids
-
-        class V(Visitor):
-
-            def visit_Text_node(visitor, node: Text):
-                job_ids.add(self.qualify_id(node.text))
-
-            def visit_Error_node(visitor, node: Error):
-                logger.warning(_(
-                    "unable to parse bootstrap_include: %s"), node.msg)
-
-        V().visit(WordList.parse(self.bootstrap_include))
+        if self.bootstrap_include is not None:
+
+            class V(Visitor):
+
+                def visit_Text_node(visitor, node: Text):
+                    job_ids.add(self.qualify_id(node.text))
+
+                def visit_Error_node(visitor, node: Error):
+                    logger.warning(_(
+                        "unable to parse bootstrap_include: %s"), node.msg)
+
+            V().visit(WordList.parse(self.bootstrap_include))
+        for tp_unit in self.get_nested_part():
+            job_ids |= tp_unit.get_bootstrap_job_ids()
         return job_ids
 
+    @lru_cache(maxsize=None)
+    def get_nested_part(self):
+        """Compute and return a set of test plan ids from nested_part field."""
+        nested_parts = []
+        if self.nested_part is not None:
+            from plainbox.impl.session import SessionManager
+            with SessionManager.get_throwaway_manager(self.provider_list) as m:
+                context = m.default_device_context
+                testplan_ids = []
+
+                class V(Visitor):
+
+                    def visit_Text_node(visitor, node: Text):
+                        testplan_ids.append(self.qualify_id(node.text))
+
+                    def visit_Error_node(visitor, node: Error):
+                        logger.warning(_(
+                            "unable to parse nested_part: %s"), node.msg)
+
+                V().visit(WordList.parse(self.nested_part))
+                for tp_id in testplan_ids:
+                    try:
+                        nested_parts.append(context.get_unit(tp_id, 'test plan'))
+                    except KeyError:
+                        logger.warning(_(
+                            "unable to find nested part: %s"), tp_id)
+        return nested_parts
+
+    @lru_cache(maxsize=None)
     def get_qualifier(self):
         """
         Convert this test plan to an equivalent qualifier for job selection
@@ -326,8 +374,11 @@
         qual_list.extend(self._gen_qualifiers('include', self.include, True))
         qual_list.extend(self._gen_qualifiers('exclude', self.exclude, False))
         qual_list.extend([self.get_bootstrap_qualifier(excluding=True)])
+        for tp_unit in self.get_nested_part():
+            qual_list.extend([tp_unit.get_qualifier()])
         return CompositeQualifier(qual_list)
 
+    @lru_cache(maxsize=None)
     def get_mandatory_qualifier(self):
         """
         Convert this test plan to an equivalent qualifier for job selection
@@ -339,20 +390,24 @@
         qual_list = []
         qual_list.extend(
             self._gen_qualifiers('include', self.mandatory_include, True))
+        for tp_unit in self.get_nested_part():
+            qual_list.extend([tp_unit.get_mandatory_qualifier()])
         return CompositeQualifier(qual_list)
 
+    @lru_cache(maxsize=None)
     def get_bootstrap_qualifier(self, excluding=False):
         """
         Convert this test plan to an equivalent qualifier for job selection
         """
         qual_list = []
-        if self.bootstrap_include is None:
-            return CompositeQualifier(qual_list)
-        field_origin = self.origin.just_line().with_offset(
-            self.field_offset_map['bootstrap_include'])
-        qual_list = [FieldQualifier(
-            'id', OperatorMatcher(operator.eq, target_id), field_origin,
-            not excluding) for target_id in self.get_bootstrap_job_ids()]
+        if self.bootstrap_include is not None:
+            field_origin = self.origin.just_line().with_offset(
+                self.field_offset_map['bootstrap_include'])
+            qual_list = [FieldQualifier(
+                'id', OperatorMatcher(operator.eq, target_id), field_origin,
+                not excluding) for target_id in self.get_bootstrap_job_ids()]
+        for tp_unit in self.get_nested_part():
+            qual_list.extend([tp_unit.get_bootstrap_qualifier(excluding)])
         return CompositeQualifier(qual_list)
 
     def _gen_qualifiers(self, field_name, field_value, inclusive):
@@ -547,6 +602,7 @@
             mandatory_include = 'mandatory_include'
             bootstrap_include = 'bootstrap_include'
             exclude = 'exclude'
+            nested_part = 'nested_part'
             estimated_duration = 'estimated_duration'
             icon = 'icon'
             category_overrides = 'category-overrides'
@@ -602,6 +658,9 @@
             fields.exclude: [
                 NonEmptyPatternIntersectionValidator,
             ],
+            fields.nested_part: [
+                NonEmptyPatternIntersectionValidator,
+            ],
             fields.estimated_duration: [
                 UntranslatableFieldValidator,
                 TemplateInvariantFieldValidator,
@@ -649,17 +708,17 @@
     Some examples of how that works, given this test plan:
         >>> testplan = TestPlanUnit({
         ...     'include': '''
-        ...         job-a certification-status=blocker, category-id=example
-        ...         job-b certification-status=non-blocker
+        ...         job-a certification_status=blocker, category-id=example
+        ...         job-b certification_status=non-blocker
         ...         job-c
         ...     ''',
         ...     'exclude': '''
         ...         job-[x-z]
         ...     ''',
-        ...     'category-overrides': '''
+        ...     'category_overrides': '''
         ...         apply other-example to job-[bc]
         ...     ''',
-        ...     'certification-status-overrides': '''
+        ...     'certification_status_overrides': '''
         ...         apply not-part-of-certification to job-c
         ...     ''',
         ...     })
@@ -810,6 +869,8 @@
                 override_list.append((pattern, 'category_id', category_id))
 
         V().visit(OverrideFieldList.parse(testplan.category_overrides))
+        for tp_unit in testplan.get_nested_part():
+            override_list.extend(self._get_category_overrides(tp_unit))
         return override_list
 
     def _get_blocker_status_overrides(
@@ -824,20 +885,21 @@
         is the overridden value.
         """
         override_list = []
-        if testplan.certification_status_overrides is None:
-            return override_list
-
-        class V(Visitor):
-
-            def visit_FieldOverride_node(self, node: FieldOverride):
-                blocker_status = node.value.text
-                pattern = r"^{}$".format(
-                    testplan.qualify_id(node.pattern.text))
-                override_list.append(
-                    (pattern, 'certification_status', blocker_status))
-
-        V().visit(OverrideFieldList.parse(
-            testplan.certification_status_overrides))
+        if testplan.certification_status_overrides is not None:
+
+            class V(Visitor):
+
+                def visit_FieldOverride_node(self, node: FieldOverride):
+                    blocker_status = node.value.text
+                    pattern = r"^{}$".format(
+                        testplan.qualify_id(node.pattern.text))
+                    override_list.append(
+                        (pattern, 'certification_status', blocker_status))
+
+            V().visit(OverrideFieldList.parse(
+                testplan.certification_status_overrides))
+        for tp_unit in testplan.get_nested_part():
+            override_list.extend(self._get_blocker_status_overrides(tp_unit))
         return override_list
 
     def _get_inline_overrides(
@@ -850,21 +912,22 @@
         subsequently packed into a tuple ``(pattern, field_value_list)``.
         """
         override_list = []
-        if testplan.include is None:
-            return override_list
-
-        class V(Visitor):
-
-            def visit_IncludeStmt_node(self, node: IncludeStmt):
-                if not node.overrides:
-                    return
-                pattern = r"^{}$".format(
-                    testplan.qualify_id(node.pattern.text))
-                field_value_list = [
-                    (override_exp.field.text.replace('-', '_'),
-                     override_exp.value.text)
-                    for override_exp in node.overrides]
-                override_list.append((pattern, field_value_list))
-
-        V().visit(IncludeStmtList.parse(testplan.include))
+        if testplan.include is not None:
+
+            class V(Visitor):
+
+                def visit_IncludeStmt_node(self, node: IncludeStmt):
+                    if not node.overrides:
+                        return
+                    pattern = r"^{}$".format(
+                        testplan.qualify_id(node.pattern.text))
+                    field_value_list = [
+                        (override_exp.field.text.replace('-', '_'),
+                         override_exp.value.text)
+                        for override_exp in node.overrides]
+                    override_list.append((pattern, field_value_list))
+
+            V().visit(IncludeStmtList.parse(testplan.include))
+        for tp_unit in testplan.get_nested_part():
+            override_list.extend(self._get_inline_overrides(tp_unit))
         return override_list