From 340b41afe17b8c40c19a596ff05e07b067f8e876 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20=27sil2100=27=20Zemczak?= Date: Tue, 14 Feb 2023 14:22:43 +0100 Subject: [PATCH] Save state file in-between runs to mark which packages have already been tested on the given clouds. --- britney.conf | 3 ++ britney2/policies/cloud.py | 68 +++++++++++++++++++++++++++++++++++--- tests/test_cloud.py | 46 ++++++++++++++++++++++++-- 3 files changed, 111 insertions(+), 6 deletions(-) diff --git a/britney.conf b/britney.conf index 40fd523..d27e968 100644 --- a/britney.conf +++ b/britney.conf @@ -143,6 +143,9 @@ CLOUD_ENABLE = no # A directory to store Cloud test results and logs. Is created at the start of # each policy run and deleted after test results are parsed. CLOUD_WORK_DIR = cloud_tests +# Path to a persistent state file, not to re-run cloud tests for packages that +# have already been tested. +CLOUD_STATE_FILE = cloud_state # Who to notify regarding test failures CLOUD_FAILURE_EMAILS = cpc@canonical.com # Who to notify regarding test errors diff --git a/britney2/policies/cloud.py b/britney2/policies/cloud.py index b19fbae..2c49d75 100644 --- a/britney2/policies/cloud.py +++ b/britney2/policies/cloud.py @@ -50,6 +50,7 @@ Regards, Ubuntu Release Team. """ class CloudPolicy(BasePolicy): PACKAGE_SET_FILE = "cloud_package_set" + STATE_FILE = "cloud_state" DEFAULT_EMAILS = ["cpc@canonical.com"] TEST_LOG_FILE = "CTF.log" @@ -68,6 +69,9 @@ class CloudPolicy(BasePolicy): self.work_dir = getattr(self.options, "cloud_work_dir", "cloud_tests") self.failure_emails = getattr(self.options, "cloud_failure_emails", self.DEFAULT_EMAILS) self.error_emails = getattr(self.options, "cloud_error_emails", self.DEFAULT_EMAILS) + self.state_filename = getattr(self.options, "cloud_state_file", self.STATE_FILE) + + self.state = {} adt_ppas = getattr(self.options, "adt_ppas", "") if not isinstance(adt_ppas, list): @@ -88,6 +92,7 @@ class CloudPolicy(BasePolicy): super().initialise(britney) self.package_set = self._retrieve_cloud_package_set_for_series(self.options.series) + self._load_state() def apply_src_policy_impl(self, policy_info, item, source_data_tdist, source_data_srcdist, excuse): if item.package not in self.package_set: @@ -103,7 +108,8 @@ class CloudPolicy(BasePolicy): self.failures = {} self.errors = {} - self._run_cloud_tests(item.package, self.options.series, self.sources, self.source_type) + self._run_cloud_tests(item.package, source_data_srcdist.version, self.options.series, + self.sources, self.source_type) if len(self.failures) > 0 or len(self.errors) > 0: self._send_emails_if_needed(item.package, source_data_srcdist.version, self.options.series) @@ -117,6 +123,54 @@ class CloudPolicy(BasePolicy): self._cleanup_work_directory() return PolicyVerdict.PASS + def _mark_tests_run(self, package, version, series, source_type, cloud): + """Mark the selected package version as already tested. + This takes which cloud we're testing into consideration. + + :param package The name of the package to test + :param version Version of the package + :param series The Ubuntu codename for the series (e.g. jammy) + :param source_type Either 'archive' or 'ppa' + :param cloud The name of the cloud being tested (e.g. azure) + """ + if cloud not in self.state: + self.state[cloud] = {} + if source_type not in self.state[cloud]: + self.state[cloud][source_type] = {} + if series not in self.state[cloud][source_type]: + self.state[cloud][source_type][series] = {} + self.state[cloud][source_type][series][package] = version + + self._save_state() + + def _check_if_tests_run(self, package, version, series, source_type, cloud): + """Check if tests were already run for the given package version. + This takes which cloud we're testing into consideration. + + :param package The name of the package to test + :param version Version of the package + :param series The Ubuntu codename for the series (e.g. jammy) + :param source_type Either 'archive' or 'ppa' + :param cloud The name of the cloud being tested (e.g. azure) + """ + try: + return self.state[cloud][source_type][series][package] == version + except KeyError: + return False + + def _load_state(self): + """Load the save state of which packages have already been tested.""" + if os.path.exists(self.state_filename): + with open(self.state_filename, encoding="utf-8") as data: + self.state = json.load(data) + self.logger.info("Loaded cloud policy state file %s" % self.state_filename) + + def _save_state(self): + """Save which packages have already been tested.""" + with open(self.state_filename, "w", encoding="utf-8") as data: + json.dump(self.state, data) + self.logger.info("Saved cloud policy state file %s" % self.state_filename) + def _retrieve_cloud_package_set_for_series(self, series): """Retrieves a set of packages for the given series in which cloud tests should be run. @@ -134,16 +188,17 @@ class CloudPolicy(BasePolicy): return package_set - def _run_cloud_tests(self, package, series, sources, source_type): + def _run_cloud_tests(self, package, version, series, sources, source_type): """Runs any cloud tests for the given package. Nothing is returned but test failures and errors are stored in instance variables. :param package The name of the package to test + :param version Version of the package :param series The Ubuntu codename for the series (e.g. jammy) :param sources List of sources where the package should be installed from (e.g. [proposed] or PPAs) :param source_type Either 'archive' or 'ppa' """ - self._run_azure_tests(package, series, sources, source_type) + self._run_azure_tests(package, version, series, sources, source_type) def _send_emails_if_needed(self, package, version, series): """Sends email(s) if there are test failures and/or errors @@ -168,14 +223,18 @@ class CloudPolicy(BasePolicy): self.logger.info("Cloud Policy: Sending error email for {}, to {}".format(package, emails)) self._send_email(emails, message) - def _run_azure_tests(self, package, series, sources, source_type): + def _run_azure_tests(self, package, version, series, sources, source_type): """Runs Azure's required package tests. :param package The name of the package to test + :param version Version of the package :param series The Ubuntu codename for the series (e.g. jammy) :param sources List of sources where the package should be installed from (e.g. [proposed] or PPAs) :param source_type Either 'archive' or 'ppa' """ + if self._check_if_tests_run(package, version, series, source_type, "azure"): + return + urn = self._retrieve_urn(series) self.logger.info("Cloud Policy: Running Azure tests for: {} in {}".format(package, series)) @@ -204,6 +263,7 @@ class CloudPolicy(BasePolicy): results_file_paths = self._find_results_files(r"TEST-NetworkTests-[0-9]*.xml") self._parse_xunit_test_results("Azure", results_file_paths) self._store_extra_test_result_info(self, package) + self._mark_tests_run(package, version, series, source_type, "azure") def _retrieve_urn(self, series): """Retrieves an URN from the configuration options based on series. diff --git a/tests/test_cloud.py b/tests/test_cloud.py index bbd8595..9d5f79a 100644 --- a/tests/test_cloud.py +++ b/tests/test_cloud.py @@ -7,6 +7,7 @@ # (at your option) any later version. import os +import json import pathlib import sys from types import SimpleNamespace @@ -35,7 +36,8 @@ class T(unittest.TestCase): verbose = False, cloud_source = "zazzy-proposed", cloud_source_type = "archive", - cloud_azure_zazzy_urn = "fake-urn-value" + cloud_azure_zazzy_urn = "fake-urn-value", + cloud_state_file = "/tmp/test_state.json" ) self.policy = CloudPolicy(self.fake_options, {}) self.policy._setup_work_directory() @@ -43,6 +45,46 @@ class T(unittest.TestCase): def tearDown(self): self.policy._cleanup_work_directory() + @patch("britney2.policies.cloud.CloudPolicy._store_extra_test_result_info") + @patch("britney2.policies.cloud.CloudPolicy._parse_xunit_test_results") + @patch("subprocess.run") + def test_run_cloud_tests_state_handling(self, mock_run, mock_xunit, mock_extra): + """Cloud tests should save state and not re-run tests for packages + already tested.""" + expected_state = { + "azure": { + "archive": { + "zazzy": { + "chromium-browser": "55.0" + } + } + } + } + with open(self.policy.options.cloud_state_file, "w") as file: + json.dump(expected_state, file) + self.policy._load_state() + + # Package already tested, no tests should run + self.policy._run_cloud_tests("chromium-browser", "55.0", "zazzy", ["proposed"], "archive") + self.assertDictEqual(expected_state, self.policy.state) + mock_run.assert_not_called() + + # A new package appears, tests should run + expected_state["azure"]["archive"]["zazzy"]["hello"] = "2.10" + self.policy._run_cloud_tests("hello", "2.10", "zazzy", ["proposed"], "archive") + self.assertDictEqual(expected_state, self.policy.state) + mock_run.assert_called() + + # A new version of existing package, tests should run + expected_state["azure"]["archive"]["zazzy"]["chromium-browser"] = "55.1" + self.policy._run_cloud_tests("chromium-browser", "55.1", "zazzy", ["proposed"], "archive") + self.assertDictEqual(expected_state, self.policy.state) + self.assertEqual(mock_run.call_count, 2) + + # Make sure the state was saved properly + with open(self.policy.options.cloud_state_file, "r") as file: + self.assertDictEqual(expected_state, json.load(file)) + @patch("britney2.policies.cloud.CloudPolicy._run_cloud_tests") def test_run_cloud_tests_called_for_package_in_manifest(self, mock_run): """Cloud tests should run for a package in the cloud package set. @@ -55,7 +97,7 @@ class T(unittest.TestCase): ) mock_run.assert_called_once_with( - "chromium-browser", "jammy", ["proposed"], "archive" + "chromium-browser", "55.0", "jammy", ["proposed"], "archive" ) @patch("britney2.policies.cloud.CloudPolicy._run_cloud_tests")