Apply overrides even when using template id in test plan definition (…

…BugFix) (#1052)

* Apply overrides even when using template id in test plan definition

If an override is described next of a template id in a test plan
definition, this override is now applied to every jobs instantiated from
this template.

Fix #1046

* Refactor SessionDeviceContext._bulk_override_update

* Add unit tests for _override_update and _bulk_override_update

* Remove vendorized mock and refactor test_state.py

* Fix test to run with Python 3.5

checkbox-ng/plainbox/impl/session/state.py

@@ -548,20 +548,20 @@ def _invalidate_override_map(self, *args, **kwargs):
         self.invalidate_shared(self._CACHE_OVERRIDE_MAP)
 
     def _bulk_override_update(self):
-        # NOTE: there is an O(N) algorithm for that solves this but it is more
-        # complicated than I was able to write without a hard-copy reference
-        # that describes it. I will improve this method once I complete the
-        # required research.
         for job_state in self.state.job_state_map.values():
             job = job_state.job
-            for pattern, override_list in self.override_map.items():
-                if re.match(pattern, job.id):
-                    job_state.apply_overrides(override_list)
+            self._override_update(job)
 
     def _override_update(self, job):
+        """
+        Apply overrides to job if they are directly related or apply to the
+        template the job was instantiated from.
+        """
         job_state = self.state.job_state_map[job.id]
         for pattern, override_list in self.override_map.items():
-            if re.match(pattern, job.id):
+            if re.match(pattern, job.id) or (
+                job.template_id and re.match(pattern, job.template_id)
+            ):
                 job_state.apply_overrides(override_list)
 
     def _update_mandatory_job_list(self):

checkbox-ng/plainbox/impl/session/test_state.py

@@ -24,6 +24,9 @@
 from doctest import DocTestSuite
 from doctest import REPORT_NDIFF
 from unittest import TestCase
+from unittest.mock import MagicMock
+from unittest.mock import Mock
+from unittest.mock import patch
 
 from plainbox.abc import IJobResult
 from plainbox.impl.depmgr import DependencyDuplicateError
@@ -38,14 +41,14 @@
 from plainbox.impl.session import InhibitionCause
 from plainbox.impl.session import SessionState
 from plainbox.impl.session import UndesiredJobReadinessInhibitor
+from plainbox.impl.session.state import JobState
 from plainbox.impl.session.state import SessionDeviceContext
 from plainbox.impl.session.state import SessionMetaData
 from plainbox.impl.testing_utils import make_job
 from plainbox.impl.unit.job import JobDefinition
 from plainbox.impl.unit.category import CategoryUnit
 from plainbox.impl.unit.unit_with_id import UnitWithId
 from plainbox.suspend_consts import Suspend
-from plainbox.vendor import mock
 from plainbox.vendor.morris import SignalTestCase
 
 
@@ -464,9 +467,9 @@ def test_mandatory_jobs_are_first_in_run_list(self):
     def test_system_information_collection_called(self):
         getter = SessionState.system_information.__get__
 
-        self_mock = mock.MagicMock()
+        self_mock = MagicMock()
         self_mock._system_information = None
-        with mock.patch(
+        with patch(
             "plainbox.impl.session.state.collect_system_information"
         ) as collect_system_information_mock:
             return_value = getter(self_mock)
@@ -479,15 +482,15 @@ def test_system_information_collection_cached(self):
         getter = SessionState.system_information.__get__
         setter = SessionState.system_information.__set__
 
-        self_mock = mock.MagicMock()
+        self_mock = MagicMock()
         self_mock._system_information = None
-        with mock.patch(
+        with patch(
             "plainbox.impl.session.state.collect_system_information"
         ) as collect_system_information_mock:
             setter(self_mock, {"inxi": {}})
             self.assertFalse(collect_system_information_mock.called)
 
-        with mock.patch(
+        with patch(
             "plainbox.impl.session.state.collect_system_information"
         ) as collect_system_information_mock:
             return_value = getter(self_mock)
@@ -503,7 +506,7 @@ class SessionStateTrimTests(TestCase):
     def setUp(self):
         self.job_a = make_job("a")
         self.job_b = make_job("b")
-        self.origin = mock.Mock(name="origin", spec_set=Origin)
+        self.origin = Mock(name="origin", spec_set=Origin)
         self.session = SessionState([self.job_a, self.job_b])
 
     def test_trim_does_remove_jobs(self):
@@ -734,7 +737,7 @@ def test_normal_job_result_updates(self):
             InhibitionCause.UNDESIRED,
         )
 
-    @mock.patch("plainbox.impl.ctrl.logger")
+    @patch("plainbox.impl.ctrl.logger")
     def test_resource_job_with_broken_output(self, mock_logger):
         # This function checks how SessionState parses partially broken
         # resource jobs.  A JobResult with broken output is constructed below.
@@ -995,13 +998,13 @@ def test_app_id_kwarg_to_init(self):
 class SessionDeviceContextTests(SignalTestCase):
     def setUp(self):
         self.ctx = SessionDeviceContext()
-        self.provider = mock.Mock(name="provider", spec_set=Provider1)
-        self.unit = mock.Mock(name="unit", spec_set=UnitWithId)
+        self.provider = Mock(name="provider", spec_set=Provider1)
+        self.unit = Mock(name="unit", spec_set=UnitWithId)
         self.unit.provider = self.provider
         self.provider.unit_list = [self.unit]
         self.provider.problem_list = []
-        self.job = mock.Mock(name="job", spec_set=JobDefinition, siblings=None)
-        self.job.get_flag_set = mock.Mock(return_value=())
+        self.job = Mock(name="job", spec_set=JobDefinition, siblings=None)
+        self.job.get_flag_set = Mock(return_value=())
         self.job.Meta.name = "job"
 
     def test_smoke(self):
@@ -1218,3 +1221,33 @@ def test_on_job_removed__via_state(self):
         sig2 = self.assertSignalFired(self.ctx.state.on_unit_removed, self.job)
         sig3 = self.assertSignalFired(self.ctx.state.on_job_removed, self.job)
         self.assertSignalOrdering(sig1, sig2, sig3)
+
+    def test_override_update(self):
+        """
+        Check that JobState.apply_overrides is called if the override matches a
+        job id or the template_id this job has been instantiated from.
+        """
+        self_mock = MagicMock()
+        self_mock.override_map = {
+            "^test-tpl$": [("certification_status", "blocker")],
+            "^job1$": [("certification_status", "blocker")],
+        }
+        job1 = Mock(id="job1", template_id=None)
+        tpl_job = Mock(id="job2", template_id="test-tpl")
+        SessionDeviceContext._override_update(self_mock, job1)
+        self.assertTrue(
+            self_mock.state.job_state_map[job1.id].apply_overrides.called
+        )
+        SessionDeviceContext._override_update(self_mock, tpl_job)
+        self.assertTrue(
+            self_mock.state.job_state_map[tpl_job.id].apply_overrides.called
+        )
+
+    def test_bulk_override_update(self):
+        self_mock = MagicMock()
+        job_state1 = Mock(spec=JobState)
+        self_mock.state.job_state_map = {
+            "job1": job_state1,
+        }
+        SessionDeviceContext._bulk_override_update(self_mock)
+        self.assertTrue(self_mock._override_update.called)