From 4b7b77671c0ae879f37c539c15ba54225d62e36c Mon Sep 17 00:00:00 2001 From: Pierre Equoy Date: Tue, 12 Mar 2024 17:27:56 +0800 Subject: [PATCH] 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 | 16 +++--- .../plainbox/impl/session/test_state.py | 57 +++++++++++++++---- 2 files changed, 53 insertions(+), 20 deletions(-) diff --git a/checkbox-ng/plainbox/impl/session/state.py b/checkbox-ng/plainbox/impl/session/state.py index 46eb54fbc..658772d84 100644 --- a/checkbox-ng/plainbox/impl/session/state.py +++ b/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): diff --git a/checkbox-ng/plainbox/impl/session/test_state.py b/checkbox-ng/plainbox/impl/session/test_state.py index 7f1dec63f..702fb822c 100644 --- a/checkbox-ng/plainbox/impl/session/test_state.py +++ b/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,6 +41,7 @@ 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 @@ -45,7 +49,6 @@ 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)