From dd4be74de291ba7d7654ce43e470991de446973a Mon Sep 17 00:00:00 2001 From: Hook25 Date: Mon, 23 Jun 2025 18:25:17 +0200 Subject: [PATCH 01/13] Initial jsonification --- .../checkbox_ng/launcher/controller.py | 25 +++++++---- .../plainbox/impl/session/remote_assistant.py | 42 ++++++++++++++++++- 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/checkbox-ng/checkbox_ng/launcher/controller.py b/checkbox-ng/checkbox_ng/launcher/controller.py index 90aa8a07b7..3b85758552 100644 --- a/checkbox-ng/checkbox_ng/launcher/controller.py +++ b/checkbox-ng/checkbox_ng/launcher/controller.py @@ -455,7 +455,8 @@ def start_session(self): configuration["normal_user"] = self._normal_user try: _logger.info("remote: Starting new session.") - tps = self.sa.start_session(configuration) + tps = self.sa.start_session_json(configuration) + tps = json.loads(tps) if self.sa.sideloaded_providers: _logger.warning("Agent is using sideloaded providers") except RuntimeError as exc: @@ -604,7 +605,7 @@ def select_tp(self, tp): _logger.error('The test plan "%s" is not available!', tp) raise SystemExit(1) self._is_bootstrapping = True - bs_todo = self.sa.get_bootstrapping_todo_list() + bs_todo = json.loads(self.sa.get_bootstrapping_todo_list_json()) for job_no, job_id in enumerate(bs_todo, start=1): print( self.C.header( @@ -616,13 +617,14 @@ def select_tp(self, tp): self.sa.run_bootstrapping_job(job_id) self.wait_for_job() self._is_bootstrapping = False - self.jobs = self.sa.finish_bootstrap() + self.jobs = json.loads(self.sa.finish_bootstrap_json()) def _strtobool(self, val): return val.lower() in ("y", "yes", "t", "true", "on", "1") def _save_manifest(self, interactive): - manifest_repr = self.sa.get_manifest_repr() + manifest_repr = self.sa.get_manifest_repr_json() + manifest_repr = json.loads(manifest_repr) if not manifest_repr: _logger.info("Skipping saving of the manifest") return @@ -644,7 +646,7 @@ def _save_manifest(self, interactive): to_save_manifest = { conf["id"]: conf["value"] for conf in all_preconf } - self.sa.save_manifest(to_save_manifest) + self.sa.save_manifest_json(json.dumps(to_save_manifest)) def select_jobs(self, all_jobs): if self.launcher.get_value("test selection", "forced"): @@ -652,7 +654,9 @@ def select_jobs(self, all_jobs): self._save_manifest(interactive=False) else: _logger.info("controller: Selecting jobs.") - reprs = json.loads(self.sa.get_jobs_repr(all_jobs)) + reprs = json.loads( + self.sa.get_jobs_repr_json(json.dumps(all_jobs)) + ) wanted_set = CategoryBrowser( "Choose tests to run on your system:", reprs ).run() @@ -662,7 +666,7 @@ def select_jobs(self, all_jobs): # the original list chosen_jobs = [job for job in all_jobs if job in wanted_set] _logger.debug("controller: Selected jobs: %s", chosen_jobs) - self.sa.modify_todo_list(chosen_jobs) + self.sa.modify_todo_list_json(json.dumps(chosen_jobs)) self._save_manifest(interactive=True) self.sa.finish_job_selection() @@ -760,7 +764,7 @@ def run_jobs(self, resumed_ongoing_session_info=None): ): self._handle_last_job_after_resume(resumed_ongoing_session_info) _logger.info("controller: Running jobs.") - jobs = self.sa.get_session_progress() + jobs = json.loads(self.sa.get_session_progress_json()) _logger.debug( "controller: Jobs to be run:\n%s", "\n".join([" " + job for job in jobs]), @@ -768,7 +772,9 @@ def run_jobs(self, resumed_ongoing_session_info=None): total_num = len(jobs["done"]) + len(jobs["todo"]) jobs_repr = json.loads( - self.sa.get_jobs_repr(jobs["todo"], len(jobs["done"])) + self.sa.get_jobs_repr_json( + json.dumps(jobs["todo"]), len(jobs["done"]) + ) ) self._run_jobs(jobs_repr, total_num) @@ -901,6 +907,7 @@ def _maybe_manual_rerun_jobs(self): return True def _run_jobs(self, jobs_repr, total_num=0): + raise SystemExit("Done bootstrapping") for job in jobs_repr: job_state = self.sa.get_job_state(job["id"]) self.sa.note_metadata_starting_job(job, job_state) diff --git a/checkbox-ng/plainbox/impl/session/remote_assistant.py b/checkbox-ng/plainbox/impl/session/remote_assistant.py index 3089b8e2e3..75bfffc157 100644 --- a/checkbox-ng/plainbox/impl/session/remote_assistant.py +++ b/checkbox-ng/plainbox/impl/session/remote_assistant.py @@ -284,6 +284,12 @@ def prepare_extra_env(self): return extra_env + @allowed_when(Idle) + def start_session_json(self, configuration): + import json + + return json.dumps(self.start_session(configuration)) + @allowed_when(Idle) def start_session(self, configuration): self._reset_sa() @@ -338,6 +344,14 @@ def start_session(self, configuration): filtered_tps = set() for filter in self._launcher.get_value("test plan", "filter"): filtered_tps.update(fnmatch.filter(tps, filter)) + launcher_tp_unit = self._launcher.get_value("test plan", "unit") + if ( + self._launcher.get_value("test plan", "forced") + and launcher_tp_unit + ): + # this seems useless (as if the test plan selection is forced, + # the test selection screen won't be shown) + filtered_tps = {launcher_tp_unit} filtered_tps = list(filtered_tps) response = zip( filtered_tps, @@ -347,7 +361,8 @@ def start_session(self, configuration): self._available_testplans = sorted( response, key=lambda x: x[1] ) # sorted by name - return self._available_testplans + self._available_testplans = list(self._available_testplans) + return list(self._available_testplans) def select_test_plan(self, test_plan_id): return self._sa.select_test_plan(test_plan_id) @@ -370,10 +385,17 @@ def prepare_bootstrapping(self, test_plan_id): # we can make this funciton return nothing return False + @allowed_when(Started) + def get_bootstrapping_todo_list_json(self): + return json.dumps(self.get_bootstrapping_todo_list()) + @allowed_when(Started) def get_bootstrapping_todo_list(self): return self._sa.get_bootstrap_todo_list() + def finish_bootstrap_json(self): + return json.dumps(self.finish_bootstrap()) + def finish_bootstrap(self): self._sa.finish_bootstrap() self._state = Bootstrapped @@ -385,12 +407,22 @@ def finish_bootstrap(self): ) return self._sa.get_static_todo_list() + def get_manifest_repr_json(self): + return json.dumps(self.get_manifest_repr()) + def get_manifest_repr(self): return self._sa.get_manifest_repr() + def save_manifest_json(self, manifest_answers): + manifest_answers = json.loads(manifest_answers) + return json.dumps(self.save_manifest(manifest_answers)) + def save_manifest(self, manifest_answers): return self._sa.save_manifest(manifest_answers) + def modify_todo_list_json(self, chosen_jobs): + self._sa.use_alternate_selection(json.loads(chosen_jobs)) + def modify_todo_list(self, chosen_jobs): self._sa.use_alternate_selection(chosen_jobs) @@ -582,10 +614,14 @@ def terminate(self): if self.terminate_cb: self.terminate_cb() + def get_session_progress_json(self): + return json.dumps(self.get_session_progress()) + def get_session_progress(self): """Return list of completed and not completed jobs in a dict.""" _logger.debug("get_session_progress()") + print("Called get_session_progress") return { "done": self._sa.get_dynamic_done_list(), "todo": self._sa.get_dynamic_todo_list(), @@ -641,6 +677,10 @@ def get_job_result(self, job_id): def get_job_state(self, job_id): return self._sa.get_job_state(job_id) + def get_jobs_repr_json(self, job_ids, offset=0): + job_ids = json.loads(job_ids) + return self.get_jobs_repr(job_ids, offset) + def get_jobs_repr(self, job_ids, offset=0): """ Translate jobs into a {'field': 'val'} representations. From d5a9fceb7ff5128911ac351c49467b45b0a873b9 Mon Sep 17 00:00:00 2001 From: Hook25 Date: Tue, 24 Jun 2025 15:46:36 +0200 Subject: [PATCH 02/13] More json apis Minor: Also poll sooner on first try --- .../checkbox_ng/launcher/controller.py | 25 +++++------- checkbox-ng/checkbox_ng/launcher/stages.py | 14 ++++--- .../plainbox/impl/session/remote_assistant.py | 39 +++++++++++++++++-- 3 files changed, 54 insertions(+), 24 deletions(-) diff --git a/checkbox-ng/checkbox_ng/launcher/controller.py b/checkbox-ng/checkbox_ng/launcher/controller.py index 3b85758552..691c9a8cbf 100644 --- a/checkbox-ng/checkbox_ng/launcher/controller.py +++ b/checkbox-ng/checkbox_ng/launcher/controller.py @@ -455,7 +455,7 @@ def start_session(self): configuration["normal_user"] = self._normal_user try: _logger.info("remote: Starting new session.") - tps = self.sa.start_session_json(configuration) + tps = self.sa.start_session_json(json.dumps(configuration)) tps = json.loads(tps) if self.sa.sideloaded_providers: _logger.warning("Agent is using sideloaded providers") @@ -714,18 +714,7 @@ def finish_session(self): stack.callback(signal.signal, signal.SIGINT, tmp_sig) self._export_results() # let's see if any of the jobs failed, if so, let's return an error code of 1 - job_state_map = ( - self._sa.manager.default_device_context._state._job_state_map - ) - failing_outcomes = ( - IJobResult.OUTCOME_FAIL, - IJobResult.OUTCOME_CRASH, - ) - self._has_anything_failed = any( - job.result.outcome in failing_outcomes - for job in job_state_map.values() - ) - + self._has_anything_failed = self.sa.has_any_job_failed() self.sa.finalize_session() return False @@ -796,9 +785,12 @@ def resume_interacting(self, interaction): def wait_for_job(self, dont_finish=False): _logger.info("controller: Waiting for job to finish.") + polling_backoff = [0, 0.1, 0.2, 0.5] + polling_i = 0 while True: state, payload = self.sa.monitor_job() if payload and not self._is_bootstrapping: + polling_i = 0 for line in payload.splitlines(): if line.startswith("stderr"): SimpleUI.red_text(line[6:]) @@ -807,7 +799,8 @@ def wait_for_job(self, dont_finish=False): else: SimpleUI.black_text(line[6:]) if state == "running": - time.sleep(0.5) + time.sleep(polling_backoff[polling_i]) + polling_i = min(polling_i + 1, len(polling_backoff)) while True: res = select.select([sys.stdin], [], [], 0) if not res[0]: @@ -907,10 +900,10 @@ def _maybe_manual_rerun_jobs(self): return True def _run_jobs(self, jobs_repr, total_num=0): - raise SystemExit("Done bootstrapping") for job in jobs_repr: job_state = self.sa.get_job_state(job["id"]) - self.sa.note_metadata_starting_job(job, job_state) + # Note: job_state is a remote object, no need to json encode it + self.sa.note_metadata_starting_job_json(json.dumps(job), job_state) SimpleUI.header( _("Running job {} / {}").format( job["num"], total_num, fill="-" diff --git a/checkbox-ng/checkbox_ng/launcher/stages.py b/checkbox-ng/checkbox_ng/launcher/stages.py index 5272f1f175..8ddc695f27 100644 --- a/checkbox-ng/checkbox_ng/launcher/stages.py +++ b/checkbox-ng/checkbox_ng/launcher/stages.py @@ -453,9 +453,13 @@ def _override_exporting(self, export_fn): self._export_fn = export_fn def _prepare_stock_report(self, report): + try: + ConfigurationType = self.sa.configuration_type() + except AttributeError: + ConfigurationType = Configuration new_origin = "stock_reports" if report == "text": - additional_config = Configuration.from_text( + additional_config = ConfigurationType.from_text( textwrap.dedent( """ [exporter:text] @@ -473,7 +477,7 @@ def _prepare_stock_report(self, report): ) self.sa.config.update_from_another(additional_config, new_origin) elif report == "certification": - additional_config = Configuration.from_text( + additional_config = ConfigurationType.from_text( textwrap.dedent( """ [exporter:tar] @@ -489,7 +493,7 @@ def _prepare_stock_report(self, report): ) self.sa.config.update_from_another(additional_config, new_origin) elif report == "certification-staging": - additional_config = Configuration.from_text( + additional_config = ConfigurationType.from_text( textwrap.dedent( """ [exporter:tar] @@ -526,7 +530,7 @@ def _prepare_stock_report(self, report): transport = {exporter}_file """ ) - additional_config = Configuration.from_text( + additional_config = ConfigurationType.from_text( template.format(exporter=exporter, path=path), new_origin ) self.sa.config.update_from_another( @@ -550,7 +554,7 @@ def _prepare_stock_report(self, report): transport = {exporter}_file """ ) - additional_config = Configuration.from_text( + additional_config = ConfigurationType.from_text( template.format(exporter=exporter, path=path), new_origin ) self.sa.config.update_from_another(additional_config, new_origin) diff --git a/checkbox-ng/plainbox/impl/session/remote_assistant.py b/checkbox-ng/plainbox/impl/session/remote_assistant.py index 75bfffc157..6fc6281b10 100644 --- a/checkbox-ng/plainbox/impl/session/remote_assistant.py +++ b/checkbox-ng/plainbox/impl/session/remote_assistant.py @@ -175,6 +175,11 @@ def _reset_sa(self): self.session_change_lock.acquire(blocking=False) self.session_change_lock.release() + def note_metadata_starting_job_json(self, job, job_state): + # job_state is a netref, it lives on this (agent) side! + job = json.loads(job) + return self.note_metadata_starting_job(job, job_state) + def note_metadata_starting_job(self, job, job_state): self._sa.note_metadata_starting_job(job, job_state) @@ -186,6 +191,12 @@ def session_change_lock(self): def config(self): return self._sa.config + def configuration_type(self): + return Configuration + + def get_config_json(self): + return json.dumps(self._sa.config.sections) + def update_app_blob(self, app_blob): self._sa.update_app_blob(app_blob) @@ -286,9 +297,7 @@ def prepare_extra_env(self): @allowed_when(Idle) def start_session_json(self, configuration): - import json - - return json.dumps(self.start_session(configuration)) + return json.dumps(self.start_session(json.loads(configuration))) @allowed_when(Idle) def start_session(self, configuration): @@ -627,6 +636,17 @@ def get_session_progress(self): "todo": self._sa.get_dynamic_todo_list(), } + def finish_job_json(self, result=None): + if result: + result = json.loads(result) + result = self.finish_job(result) + return json.dumps( + { + "tr_outcome": result.tr_outcome(), + "outcome_color": result.outcome_color_ansi(), + } + ) + def finish_job(self, result=None): # assert the thread completed self.session_change_lock.acquire(blocking=False) @@ -847,6 +867,19 @@ def resume_by_id(self, session_id=None, overwrite_result_dict={}): self._state = TestsSelected + def has_any_job_failed(self): + job_state_map = ( + self.manager.default_device_context._state._job_state_map + ) + failing_outcomes = ( + IJobResult.OUTCOME_FAIL, + IJobResult.OUTCOME_CRASH, + ) + return any( + job.result.outcome in failing_outcomes + for job in job_state_map.values() + ) + def finalize_session(self): self._sa.finalize_session() self._reset_sa() From cf9524507833814ee0fc66cc6f1e6fcd4a05ff2d Mon Sep 17 00:00:00 2001 From: Hook25 Date: Tue, 24 Jun 2025 16:41:20 +0200 Subject: [PATCH 03/13] Increase chunk size greately This is done because every call we pay 1 roundtrip, that may be 100ms and this, only to get a pretty bar going through the screen, I feel like 58 tiks are enough out of 8Mb, we don't need 580 --- checkbox-ng/checkbox_ng/launcher/controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkbox-ng/checkbox_ng/launcher/controller.py b/checkbox-ng/checkbox_ng/launcher/controller.py index 691c9a8cbf..46bcaab3d5 100644 --- a/checkbox-ng/checkbox_ng/launcher/controller.py +++ b/checkbox-ng/checkbox_ng/launcher/controller.py @@ -836,7 +836,7 @@ def local_export(self, exporter_id, transport, options=()): _logger.info("controller: Exporting locally'") rf = self.sa.cache_report(exporter_id, options) exported_stream = SpooledTemporaryFile(max_size=102400, mode="w+b") - chunk_size = 16384 + chunk_size = 160 * 1024 with tqdm( total=rf.tell(), unit="B", From 762d7e6b80380dd925888e44c239ef3426f84eea Mon Sep 17 00:00:00 2001 From: Hook25 Date: Wed, 25 Jun 2025 15:01:40 +0200 Subject: [PATCH 04/13] Document RemoteAssistant better with a docstr Minor: remove old debug stuff wrongly committed --- .../plainbox/impl/session/remote_assistant.py | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/checkbox-ng/plainbox/impl/session/remote_assistant.py b/checkbox-ng/plainbox/impl/session/remote_assistant.py index 6fc6281b10..293efde5a3 100644 --- a/checkbox-ng/plainbox/impl/session/remote_assistant.py +++ b/checkbox-ng/plainbox/impl/session/remote_assistant.py @@ -27,6 +27,8 @@ from contextlib import suppress from tempfile import SpooledTemporaryFile from threading import Thread, Lock +from enum import Enum + from plainbox.impl.config import Configuration from plainbox.impl.execution import UnifiedRunner from plainbox.impl.session.assistant import SessionAssistant @@ -141,9 +143,18 @@ def outcome(self): class RemoteSessionAssistant: - """Remote execution enabling wrapper for the SessionAssistant""" + """ + This is the main API surface for controller-agent communication + + Code in this class runs in the agent. Returning mutable types or receiving + mutable types as parameter from any of these functions creates an implicit + remote API (as any function/attribute used on the returned value will + result in a remote API call) and should therefore be avoided. + Favour creating a JSON API version of the function that returns the same + object but JSON encoded. + """ - REMOTE_API_VERSION = 13 + REMOTE_API_VERSION = 14 def __init__(self, cmd_callback): _logger.debug("__init__()") @@ -353,14 +364,6 @@ def start_session(self, configuration): filtered_tps = set() for filter in self._launcher.get_value("test plan", "filter"): filtered_tps.update(fnmatch.filter(tps, filter)) - launcher_tp_unit = self._launcher.get_value("test plan", "unit") - if ( - self._launcher.get_value("test plan", "forced") - and launcher_tp_unit - ): - # this seems useless (as if the test plan selection is forced, - # the test selection screen won't be shown) - filtered_tps = {launcher_tp_unit} filtered_tps = list(filtered_tps) response = zip( filtered_tps, @@ -371,7 +374,7 @@ def start_session(self, configuration): response, key=lambda x: x[1] ) # sorted by name self._available_testplans = list(self._available_testplans) - return list(self._available_testplans) + return self._available_testplans def select_test_plan(self, test_plan_id): return self._sa.select_test_plan(test_plan_id) @@ -630,7 +633,6 @@ def get_session_progress(self): """Return list of completed and not completed jobs in a dict.""" _logger.debug("get_session_progress()") - print("Called get_session_progress") return { "done": self._sa.get_dynamic_done_list(), "todo": self._sa.get_dynamic_todo_list(), From b1df6a25d3857cf60da3ff421634d4f9452410a2 Mon Sep 17 00:00:00 2001 From: Hook25 Date: Wed, 25 Jun 2025 17:48:49 +0200 Subject: [PATCH 05/13] Off by 1 error on the polling delay --- checkbox-ng/checkbox_ng/launcher/controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkbox-ng/checkbox_ng/launcher/controller.py b/checkbox-ng/checkbox_ng/launcher/controller.py index 46bcaab3d5..08f28cc61a 100644 --- a/checkbox-ng/checkbox_ng/launcher/controller.py +++ b/checkbox-ng/checkbox_ng/launcher/controller.py @@ -800,7 +800,7 @@ def wait_for_job(self, dont_finish=False): SimpleUI.black_text(line[6:]) if state == "running": time.sleep(polling_backoff[polling_i]) - polling_i = min(polling_i + 1, len(polling_backoff)) + polling_i = min(polling_i + 1, len(polling_backoff) - 1) while True: res = select.select([sys.stdin], [], [], 0) if not res[0]: From cc6b50baf7105244815764315b4aef26892f8e98 Mon Sep 17 00:00:00 2001 From: Hook25 Date: Mon, 30 Jun 2025 12:55:20 +0200 Subject: [PATCH 06/13] Cache attributes of job interactions Minor: forgot to bump remote api version --- .../checkbox_ng/launcher/controller.py | 31 +++++++++---------- .../plainbox/impl/session/remote_assistant.py | 2 +- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/checkbox-ng/checkbox_ng/launcher/controller.py b/checkbox-ng/checkbox_ng/launcher/controller.py index 08f28cc61a..01b9050c74 100644 --- a/checkbox-ng/checkbox_ng/launcher/controller.py +++ b/checkbox-ng/checkbox_ng/launcher/controller.py @@ -916,14 +916,13 @@ def _run_jobs(self, jobs_repr, total_num=0): next_job = False while next_job is False: for interaction in self.sa.run_job(job["id"]): - if interaction.kind == "purpose": - SimpleUI.description( - _("Purpose:"), interaction.message - ) - elif interaction.kind == "description": - SimpleUI.description( - _("Description:"), interaction.message - ) + # interaction is a netref, cache attributes here + kind = interaction.kind + message = interaction.message + if kind == "purpose": + SimpleUI.description(_("Purpose:"), message) + elif kind == "description": + SimpleUI.description(_("Description:"), message) if job["command"] is None: cmd = "run" else: @@ -937,8 +936,8 @@ def _run_jobs(self, jobs_repr, total_num=0): raise SystemExit("Session paused by the user") self.sa.remember_users_response(cmd) self.wait_for_job(dont_finish=True) - elif interaction.kind in "steps": - SimpleUI.description(_("Steps:"), interaction.message) + elif kind in "steps": + SimpleUI.description(_("Steps:"), message) if job["command"] is None: cmd = "run" else: @@ -951,12 +950,10 @@ def _run_jobs(self, jobs_repr, total_num=0): self.sa.remember_users_response(cmd) raise SystemExit("Session paused by the user") self.sa.remember_users_response(cmd) - elif interaction.kind == "verification": + elif kind == "verification": self.wait_for_job(dont_finish=True) - if interaction.message: - SimpleUI.description( - _("Verification:"), interaction.message - ) + if message: + SimpleUI.description(_("Verification:"), message) JobAdapter = namedtuple("job_adapter", ["command"]) job_lite = JobAdapter(job["command"]) try: @@ -976,14 +973,14 @@ def _run_jobs(self, jobs_repr, total_num=0): interaction.extra._builder.get_result(), ) break - elif interaction.kind == "comment": + elif kind == "comment": new_comment = input( SimpleUI.C.BLUE( _("Please enter your comments:") + "\n" ) ) self.sa.remember_users_response(new_comment + "\n") - elif interaction.kind == "skip": + elif kind == "skip": if ( job_state.effective_certification_status == "blocker" diff --git a/checkbox-ng/plainbox/impl/session/remote_assistant.py b/checkbox-ng/plainbox/impl/session/remote_assistant.py index 293efde5a3..c5b22993fa 100644 --- a/checkbox-ng/plainbox/impl/session/remote_assistant.py +++ b/checkbox-ng/plainbox/impl/session/remote_assistant.py @@ -154,7 +154,7 @@ class RemoteSessionAssistant: object but JSON encoded. """ - REMOTE_API_VERSION = 14 + REMOTE_API_VERSION = 15 def __init__(self, cmd_callback): _logger.debug("__init__()") From bedbb96d174cfc931de59ead7c0039873c6b659b Mon Sep 17 00:00:00 2001 From: Hook25 Date: Mon, 30 Jun 2025 16:03:11 +0200 Subject: [PATCH 07/13] Adjust tests to the new api version --- .../checkbox_ng/launcher/test_controller.py | 22 ++++++++----------- .../checkbox_ng/launcher/test_stages.py | 1 + 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/checkbox-ng/checkbox_ng/launcher/test_controller.py b/checkbox-ng/checkbox_ng/launcher/test_controller.py index fd2955c564..2443108024 100644 --- a/checkbox-ng/checkbox_ng/launcher/test_controller.py +++ b/checkbox-ng/checkbox_ng/launcher/test_controller.py @@ -18,6 +18,7 @@ import socket +import json from unittest import TestCase, mock from functools import partial @@ -180,13 +181,8 @@ def test_finish_session_all_pass(self): """ self_mock = mock.MagicMock() - mock_job_state_map = { - "job1": mock.MagicMock(result=mock.MagicMock(outcome="pass")), - "job2": mock.MagicMock(result=mock.MagicMock(outcome="pass")), - } - self_mock.manager.default_device_context._state._job_state_map = ( - mock_job_state_map - ) + self_mock.sa.has_any_job_failed.return_value = False + RemoteController.finish_session(self_mock) self.assertFalse(self_mock._has_anything_failed) @@ -1037,14 +1033,14 @@ def test_start_session_success(self): "normal_user": True, } - self_mock.sa.start_session.return_value = "session_started" + self_mock.sa.start_session_json.return_value = '["testplan1"]' tps = RemoteController.start_session(self_mock) - self_mock.sa.start_session.assert_called_once_with( - expected_configuration + self_mock.sa.start_session_json.assert_called_once_with( + json.dumps(expected_configuration) ) - self.assertEqual(tps, "session_started") + self.assertEqual(tps, ["testplan1"]) def test_start_session_with_sideloaded_providers(self): self_mock = mock.MagicMock() @@ -1052,7 +1048,7 @@ def test_start_session_with_sideloaded_providers(self): self_mock._normal_user = True self_mock.sa.sideloaded_providers = True - self_mock.sa.start_session.return_value = "session_started" + self_mock.sa.start_session_json.return_value = '["testplan1"]' RemoteController.start_session(self_mock) @@ -1060,7 +1056,7 @@ def test_start_session_runtime_error(self): self_mock = mock.MagicMock() self_mock._launcher_text = "launcher_example" self_mock._normal_user = True - self_mock.sa.start_session.side_effect = RuntimeError( + self_mock.sa.start_session_json.side_effect = RuntimeError( "Failed to start session" ) diff --git a/checkbox-ng/checkbox_ng/launcher/test_stages.py b/checkbox-ng/checkbox_ng/launcher/test_stages.py index 5e957a8a7a..b788c53232 100644 --- a/checkbox-ng/checkbox_ng/launcher/test_stages.py +++ b/checkbox-ng/checkbox_ng/launcher/test_stages.py @@ -161,6 +161,7 @@ def test__prepare_stock_report_submission_files(self, makedirs): @mock.patch("os.makedirs") def test__prepare_stock_report_submission_json(self, makedirs): self_mock = mock.MagicMock() + self_mock.sa.configuration_type.side_effect = AttributeError self_mock._get_submission_file_path = partial( ReportsStage._get_submission_file_path, self_mock ) From 0df42e33cbbcb1f04bd0c93f7661dad4b755eab5 Mon Sep 17 00:00:00 2001 From: Hook25 Date: Tue, 1 Jul 2025 08:36:08 +0200 Subject: [PATCH 08/13] Test all new json apis --- .../impl/session/test_remote_assistant.py | 265 +++++++++++++++--- 1 file changed, 230 insertions(+), 35 deletions(-) diff --git a/checkbox-ng/plainbox/impl/session/test_remote_assistant.py b/checkbox-ng/plainbox/impl/session/test_remote_assistant.py index affe9ddb7d..bd4b1d6bdf 100644 --- a/checkbox-ng/plainbox/impl/session/test_remote_assistant.py +++ b/checkbox-ng/plainbox/impl/session/test_remote_assistant.py @@ -17,7 +17,9 @@ # You should have received a copy of the GNU General Public License # along with Checkbox. If not, see . +import json from os.path import exists +from functools import partial from unittest import TestCase, mock @@ -31,13 +33,14 @@ from plainbox.impl.secure.sudo_broker import is_passwordless_sudo from plainbox.impl.session import remote_assistant +from plainbox.impl.session.remote_assistant import RemoteSessionAssistant from plainbox.impl.session.assistant import SessionAssistant class RemoteAssistantTests(TestCase): def test_allowed_when_ok(self): self_mock = mock.MagicMock() - allowed_when = remote_assistant.RemoteSessionAssistant.allowed_when + allowed_when = RemoteSessionAssistant.allowed_when @allowed_when(remote_assistant.Idle) def allowed(self, *args): ... @@ -47,7 +50,7 @@ def allowed(self, *args): ... def test_allowed_when_fail(self): self_mock = mock.MagicMock() - allowed_when = remote_assistant.RemoteSessionAssistant.allowed_when + allowed_when = RemoteSessionAssistant.allowed_when @allowed_when(remote_assistant.Idle) def not_allowed(self, *args): ... @@ -61,7 +64,7 @@ def not_allowed(self, *args): ... def test__reset_sa(self, is_passwordless_sudo_mock, init_mock): init_mock.return_value = None # RSA constructor calls _reset_sa, which in turns creates a new SA - rsa = remote_assistant.RemoteSessionAssistant(lambda: None) + rsa = RemoteSessionAssistant(lambda: None) self.assertEqual(init_mock.call_count, 1) @mock.patch("plainbox.impl.session.remote_assistant.guess_normal_user") @@ -75,11 +78,20 @@ def test_start_session_with_launcher(self, mock_filter, mock_gnu): rsa = mock.Mock() rsa.get_test_plans.return_value = [mock.Mock()] rsa._state = remote_assistant.Idle + + def get_test_plan_mock(name): + to_r = mock.Mock() + to_r.name = name + return to_r + + rsa._sa.get_test_plan.side_effect = get_test_plan_mock + rsa.start_session = partial(RemoteSessionAssistant.start_session, rsa) with mock.patch("plainbox.impl.config.Configuration.from_text") as cm: cm.return_value = Configuration() - tps = remote_assistant.RemoteSessionAssistant.start_session( - rsa, extra_cfg + tps = RemoteSessionAssistant.start_session_json( + rsa, json.dumps(extra_cfg) ) + tps = json.loads(tps) self.assertEqual(tps[0][0][1], "tp") @mock.patch("plainbox.impl.session.remote_assistant.guess_normal_user") @@ -95,9 +107,7 @@ def test_start_session_without_launcher(self, mock_filter, mock_gnu): rsa._state = remote_assistant.Idle with mock.patch("plainbox.impl.config.Configuration.from_text") as cm: cm.return_value = Configuration() - tps = remote_assistant.RemoteSessionAssistant.start_session( - rsa, extra_cfg - ) + tps = RemoteSessionAssistant.start_session(rsa, extra_cfg) self.assertEqual(tps[0][0][1], "tp") def test_resume_by_id_with_session_id(self): @@ -110,7 +120,7 @@ def test_resume_by_id_with_session_id(self): mock_meta.app_blob = b'{"launcher": "", "testplan_id": "tp_id"}' rsa.resume_session.return_value = mock_meta - remote_assistant.RemoteSessionAssistant.resume_by_id(rsa, "session_id") + RemoteSessionAssistant.resume_by_id(rsa, "session_id") self.assertEqual(rsa._state, "testsselected") def test_resume_by_id_bad_session_id(self): @@ -123,7 +133,7 @@ def test_resume_by_id_bad_session_id(self): mock_meta.app_blob = b'{"launcher": "", "testplan_id": "tp_id"}' rsa.resume_session.return_value = mock_meta - remote_assistant.RemoteSessionAssistant.resume_by_id(rsa, "bad_id") + RemoteSessionAssistant.resume_by_id(rsa, "bad_id") self.assertEqual(rsa._state, "idle") def test_resume_by_id_without_session_id(self): @@ -136,7 +146,7 @@ def test_resume_by_id_without_session_id(self): mock_meta.app_blob = b'{"launcher": "", "testplan_id": "tp_id"}' rsa.resume_session.return_value = mock_meta - remote_assistant.RemoteSessionAssistant.resume_by_id(rsa) + RemoteSessionAssistant.resume_by_id(rsa) self.assertEqual(rsa._state, "testsselected") @mock.patch("plainbox.impl.session.remote_assistant.load_configs") @@ -167,7 +177,7 @@ def test_resume_by_id_with_result_file_ok(self, mock_load_configs): ), ): os_path_exists_mock.return_value = True - remote_assistant.RemoteSessionAssistant.resume_by_id(rsa) + RemoteSessionAssistant.resume_by_id(rsa) mjr = MemoryJobResult( { @@ -207,7 +217,7 @@ def test_resume_by_id_with_result_file_garbage_outcome( ), ): os_path_exists_mock.return_value = True - remote_assistant.RemoteSessionAssistant.resume_by_id(rsa) + RemoteSessionAssistant.resume_by_id(rsa) mjr = MemoryJobResult( { @@ -244,7 +254,7 @@ def test_resume_by_id_with_result_no_file_noreturn( "noreturn" } - remote_assistant.RemoteSessionAssistant.resume_by_id(rsa) + RemoteSessionAssistant.resume_by_id(rsa) mjr = MemoryJobResult( { @@ -282,7 +292,7 @@ def test_resume_by_id_with_result_no_file_normal(self, mock_load_configs): os_path_exists_mock.return_value = False rsa._sa.get_job.return_value.get_flag_set.return_value = {} - remote_assistant.RemoteSessionAssistant.resume_by_id(rsa) + RemoteSessionAssistant.resume_by_id(rsa) mjr = MemoryJobResult( { @@ -322,7 +332,7 @@ def test_resume_by_id_with_result_no_file_already_set( os_path_exists_mock.return_value = False rsa._sa.get_job.return_value.get_flag_set.return_value = {} - remote_assistant.RemoteSessionAssistant.resume_by_id(rsa) + RemoteSessionAssistant.resume_by_id(rsa) mjr = MemoryJobResult( { @@ -354,7 +364,7 @@ def test_resume_by_id_with_result_file_not_json(self, mock_load_configs): with mock.patch("builtins.open", mock.mock_open(read_data="!@!")): with mock.patch("os.path.exists", os_path_exists_mock): os_path_exists_mock.return_value = True - remote_assistant.RemoteSessionAssistant.resume_by_id(rsa) + RemoteSessionAssistant.resume_by_id(rsa) mjr = MemoryJobResult( { @@ -369,9 +379,7 @@ def test_remember_users_response_quit(self): self_mock = mock.MagicMock() self_mock._state = remote_assistant.Interacting - remote_assistant.RemoteSessionAssistant.remember_users_response( - self_mock, "quit" - ) + RemoteSessionAssistant.remember_users_response(self_mock, "quit") self.assertTrue(self_mock.abandon_session.called) @@ -379,9 +387,7 @@ def test_remember_users_response_rollback(self): self_mock = mock.MagicMock() self_mock._state = remote_assistant.Interacting - remote_assistant.RemoteSessionAssistant.remember_users_response( - self_mock, "rollback" - ) + RemoteSessionAssistant.remember_users_response(self_mock, "rollback") self.assertEqual(self_mock._state, remote_assistant.TestsSelected) @@ -389,35 +395,224 @@ def test_remember_users_response_run(self): self_mock = mock.MagicMock() self_mock._state = remote_assistant.Interacting - remote_assistant.RemoteSessionAssistant.remember_users_response( - self_mock, "run" - ) + RemoteSessionAssistant.remember_users_response(self_mock, "run") self.assertEqual(self_mock._state, remote_assistant.Running) def test_note_metadata_starting_job(self): self_mock = mock.MagicMock() - remote_assistant.RemoteSessionAssistant.note_metadata_starting_job( - self_mock, mock.MagicMock(), mock.MagicMock() + note_metadata_starting_job = partial( + RemoteSessionAssistant.note_metadata_starting_job, + self_mock, + ) + self_mock.note_metadata_starting_job = note_metadata_starting_job + RemoteSessionAssistant.note_metadata_starting_job_json( + self_mock, "{}", mock.MagicMock() ) self.assertTrue(self_mock._sa.note_metadata_starting_job.called) def test_abandon_session(self): self_mock = mock.MagicMock() - remote_assistant.RemoteSessionAssistant.abandon_session(self_mock) + RemoteSessionAssistant.abandon_session(self_mock) self.assertTrue(self_mock._reset_sa.called) def test_delete_sessions(self): self_mock = mock.MagicMock() - remote_assistant.RemoteSessionAssistant.delete_sessions(self_mock, []) + RemoteSessionAssistant.delete_sessions(self_mock, []) self.assertTrue(self_mock._sa.delete_sessions.called) def test_get_resumable_sessions(self): self_mock = mock.MagicMock() - remote_assistant.RemoteSessionAssistant.get_resumable_sessions( + RemoteSessionAssistant.get_resumable_sessions(self_mock) + self.assertTrue(self_mock._sa.get_resumable_sessions.called) + + def test_configuration_type(self): + # This is used to allow the controller to create netref configurations. + conf_type = RemoteSessionAssistant.configuration_type(mock.MagicMock()) + self.assertEqual(conf_type, remote_assistant.Configuration) + + def test_bootstrapping_todo_list(self): + self_mock = mock.MagicMock() + self_mock._state = remote_assistant.Started + self_mock.get_bootstrapping_todo_list = partial( + RemoteSessionAssistant.get_bootstrapping_todo_list, self_mock + ) + self_mock._sa.get_bootstrap_todo_list.return_value = [ + "job1", + "job2", + ] + + job_list_str = RemoteSessionAssistant.get_bootstrapping_todo_list_json( self_mock ) - self.assertTrue(self_mock._sa.get_resumable_sessions.called) + + self.assertTrue(self_mock._sa.get_bootstrap_todo_list.called) + self.assertEqual(["job1", "job2"], json.loads(job_list_str)) + + def test_finish_bootstrap_json(self): + self_mock = mock.MagicMock() + self_mock.finish_bootstrap = partial( + RemoteSessionAssistant.finish_bootstrap, self_mock + ) + self_mock._sa.get_static_todo_list.return_value = static_todo_list = [ + "test_{}".format(x) for x in range(10) + ] + to_r = {x: mock.MagicMock() for x in static_todo_list} + + def get_job_state(id): + return to_r[id] + + self_mock._sa.get_job_state = get_job_state + + def get_value(top, key): + assert top == "ui" + if key == "auto_retry": + return True + elif key == "max_attempts": + return 10 + assert False, "Undefined key" + + self_mock._launcher.get_value = get_value + + bootstrapped_todo = json.loads( + RemoteSessionAssistant.finish_bootstrap_json(self_mock) + ) + + self.assertEqual(self_mock._state, remote_assistant.Bootstrapped) + self.assertEqual(bootstrapped_todo, static_todo_list) + self.assertTrue( + all( + to_r[x].attempts == get_value("ui", "max_attempts") + for x in bootstrapped_todo + ) + ) + + def test_get_manifest_repr_json(self): + self_mock = mock.MagicMock() + self_mock.get_manifest_repr = partial( + RemoteSessionAssistant.get_manifest_repr, self_mock + ) + self_mock._sa.get_manifest_repr.return_value = {"manifest1": True} + + manifest = RemoteSessionAssistant.get_manifest_repr_json(self_mock) + + self.assertEqual(json.loads(manifest), {"manifest1": True}) + + def test_modify_todo_list_json(self): + self_mock = mock.MagicMock() + self_mock.modify_todo_list = partial( + RemoteSessionAssistant.modify_todo_list, self_mock + ) + chosen_jobs_list = ["job1", "job2", "job3"] + chosen_jobs_json = json.dumps(chosen_jobs_list) + + RemoteSessionAssistant.modify_todo_list_json( + self_mock, chosen_jobs_json + ) + + self.assertTrue(self_mock._sa.use_alternate_selection.assert_called) + + def test_finish_job_json_with_result(self): + self_mock = mock.MagicMock() + mock_result_obj = mock.MagicMock() + mock_result_obj.tr_outcome.return_value = "PASS" + mock_result_obj.outcome_color_ansi.return_value = "[green]PASS[/green]" + self_mock.finish_job.return_value = mock_result_obj + + input_result_data = { + "outcome": "PASS", + "comments": "Completed successfully", + } + input_result_json = json.dumps(input_result_data) + + response_json = RemoteSessionAssistant.finish_job_json( + self_mock, input_result_json + ) + + self_mock.finish_job.assert_called_once_with(input_result_data) + expected_response = { + "tr_outcome": "PASS", + "outcome_color": "[green]PASS[/green]", + } + self.assertEqual(json.loads(response_json), expected_response) + + def test_finish_job_json_without_result(self): + self_mock = mock.MagicMock() + mock_result_obj = mock.MagicMock() + mock_result_obj.tr_outcome.return_value = "FAIL" + mock_result_obj.outcome_color_ansi.return_value = "[red]FAIL[/red]" + self_mock.finish_job.return_value = mock_result_obj + + response_json = RemoteSessionAssistant.finish_job_json(self_mock) + + self_mock.finish_job.assert_called_once_with(None) + expected_response = { + "tr_outcome": "FAIL", + "outcome_color": "[red]FAIL[/red]", + } + self.assertEqual(json.loads(response_json), expected_response) + + def test_finish_job_json_handles_none_result_from_finish_job(self): + self_mock = mock.MagicMock() + self_mock.finish_job.return_value = None + + response = RemoteSessionAssistant.finish_job_json(self_mock) + + self_mock.finish_job.assert_called_once_with(None) + self.assertIsNone(response) + + def test_has_any_job_failed_is_true_if_a_job_failed(self): + self_mock = mock.MagicMock() + + passing_job = mock.MagicMock() + passing_job.result.outcome = IJobResult.OUTCOME_PASS + + failing_job = mock.MagicMock() + failing_job.result.outcome = IJobResult.OUTCOME_FAIL + + self_mock.manager.default_device_context._state._job_state_map = { + "job1": passing_job, + "job2": failing_job, + } + + self.assertTrue(RemoteSessionAssistant.has_any_job_failed(self_mock)) + + def test_has_any_job_failed_is_true_if_a_job_crashed(self): + self_mock = mock.MagicMock() + + passing_job = mock.MagicMock() + passing_job.result.outcome = IJobResult.OUTCOME_PASS + + crashing_job = mock.MagicMock() + crashing_job.result.outcome = IJobResult.OUTCOME_CRASH + + self_mock.manager.default_device_context._state._job_state_map = { + "job1": passing_job, + "job2": crashing_job, + } + + self.assertTrue(RemoteSessionAssistant.has_any_job_failed(self_mock)) + + def test_has_any_job_failed_is_false_if_no_jobs_failed(self): + self_mock = mock.MagicMock() + + passing_job_1 = mock.MagicMock() + passing_job_1.result.outcome = IJobResult.OUTCOME_PASS + + passing_job_2 = mock.MagicMock() + passing_job_2.result.outcome = IJobResult.OUTCOME_PASS + + self_mock.manager.default_device_context._state._job_state_map = { + "job1": passing_job_1, + "job2": passing_job_2, + } + + self.assertFalse(RemoteSessionAssistant.has_any_job_failed(self_mock)) + + def test_has_any_job_failed_is_false_for_empty_job_map(self): + self_mock = mock.MagicMock() + self_mock.manager.default_device_context._state._job_state_map = {} + self.assertFalse(RemoteSessionAssistant.has_any_job_failed(self_mock)) class RemoteAssistantFinishJobTests(TestCase): @@ -433,7 +628,7 @@ def test_no_result_after_auto_resume(self, MockJobResultBuilder): mock_builder = MockJobResultBuilder.return_value mock_builder.get_result.return_value = IJobResult.OUTCOME_PASS - result = remote_assistant.RemoteSessionAssistant.finish_job(self.rsa) + result = RemoteSessionAssistant.finish_job(self.rsa) self.rsa._sa.use_job_result.assert_called_with("job_id", "pass") self.assertEqual(result, IJobResult.OUTCOME_PASS) @@ -451,7 +646,7 @@ def test_no_result_with_be(self): self.rsa._be.wait().get_result = wait_get_result_res wait_get_result_res.return_value = IJobResult.OUTCOME_PASS - result = remote_assistant.RemoteSessionAssistant.finish_job(self.rsa) + result = RemoteSessionAssistant.finish_job(self.rsa) self.assertTrue(self.rsa._be.wait.called) self.assertTrue(self.rsa._be.wait().get_result) @@ -467,7 +662,7 @@ def test_no_result_with_be_but_no_builder(self, MockJobResultBuilder): mock_builder = MockJobResultBuilder.return_value mock_builder.get_result.return_value = IJobResult.OUTCOME_PASS - result = remote_assistant.RemoteSessionAssistant.finish_job(self.rsa) + result = RemoteSessionAssistant.finish_job(self.rsa) self.rsa._sa.use_job_result.assert_called_with("job_id", "pass") self.assertEqual(result, IJobResult.OUTCOME_PASS) From afb8ae138b336f109b980013d588e4974fe6918a Mon Sep 17 00:00:00 2001 From: Hook25 Date: Tue, 1 Jul 2025 08:37:24 +0200 Subject: [PATCH 09/13] Fix bug in finish_job_json on None result Minor: remove unused API Minor: json api should call non-json API --- .../plainbox/impl/session/remote_assistant.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/checkbox-ng/plainbox/impl/session/remote_assistant.py b/checkbox-ng/plainbox/impl/session/remote_assistant.py index c5b22993fa..4fe6f836cc 100644 --- a/checkbox-ng/plainbox/impl/session/remote_assistant.py +++ b/checkbox-ng/plainbox/impl/session/remote_assistant.py @@ -205,9 +205,6 @@ def config(self): def configuration_type(self): return Configuration - def get_config_json(self): - return json.dumps(self._sa.config.sections) - def update_app_blob(self, app_blob): self._sa.update_app_blob(app_blob) @@ -433,7 +430,7 @@ def save_manifest(self, manifest_answers): return self._sa.save_manifest(manifest_answers) def modify_todo_list_json(self, chosen_jobs): - self._sa.use_alternate_selection(json.loads(chosen_jobs)) + self.modify_todo_list(json.loads(chosen_jobs)) def modify_todo_list(self, chosen_jobs): self._sa.use_alternate_selection(chosen_jobs) @@ -642,12 +639,14 @@ def finish_job_json(self, result=None): if result: result = json.loads(result) result = self.finish_job(result) - return json.dumps( - { - "tr_outcome": result.tr_outcome(), - "outcome_color": result.outcome_color_ansi(), - } - ) + if result is not None: + return json.dumps( + { + "tr_outcome": result.tr_outcome(), + "outcome_color": result.outcome_color_ansi(), + } + ) + return def finish_job(self, result=None): # assert the thread completed From 3bca223e80a2f0293ca0d4f25919c906f6e8861a Mon Sep 17 00:00:00 2001 From: Hook25 Date: Tue, 1 Jul 2025 08:43:21 +0200 Subject: [PATCH 10/13] Wrong function called --- checkbox-ng/plainbox/impl/session/test_remote_assistant.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkbox-ng/plainbox/impl/session/test_remote_assistant.py b/checkbox-ng/plainbox/impl/session/test_remote_assistant.py index bd4b1d6bdf..5d5ac43a22 100644 --- a/checkbox-ng/plainbox/impl/session/test_remote_assistant.py +++ b/checkbox-ng/plainbox/impl/session/test_remote_assistant.py @@ -510,7 +510,7 @@ def test_modify_todo_list_json(self): self_mock, chosen_jobs_json ) - self.assertTrue(self_mock._sa.use_alternate_selection.assert_called) + self.assertTrue(self_mock._sa.use_alternate_selection.called) def test_finish_job_json_with_result(self): self_mock = mock.MagicMock() From d519ce5c35661a3a36d10f29fd2ec55774b72f8f Mon Sep 17 00:00:00 2001 From: Hook25 Date: Tue, 1 Jul 2025 10:00:59 +0200 Subject: [PATCH 11/13] Test modified controller functions --- .../checkbox_ng/launcher/test_controller.py | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+) diff --git a/checkbox-ng/checkbox_ng/launcher/test_controller.py b/checkbox-ng/checkbox_ng/launcher/test_controller.py index 2443108024..54d77ad2e4 100644 --- a/checkbox-ng/checkbox_ng/launcher/test_controller.py +++ b/checkbox-ng/checkbox_ng/launcher/test_controller.py @@ -1063,6 +1063,148 @@ def test_start_session_runtime_error(self): with self.assertRaises(SystemExit) as _: RemoteController.start_session(self_mock) + def test__save_manifest_no_repr(self): + self_mock = mock.MagicMock() + self_mock.sa.get_manifest_repr_json.return_value = "{}" + RemoteController._save_manifest(self_mock, False) + + @mock.patch("checkbox_ng.launcher.controller.ManifestBrowser") + def test__save_manifest_interactive(self, manifest_browser_mock): + self_mock = mock.MagicMock() + manifest_repr = {"Question 1": [{"id": "conf1", "value": "val1"}]} + self_mock.sa.get_manifest_repr_json.return_value = json.dumps( + manifest_repr + ) + to_save_manifest = {"conf1": "new_val"} + manifest_browser_mock.return_value.run.return_value = to_save_manifest + + RemoteController._save_manifest(self_mock, True) + + manifest_browser_mock.assert_called_once_with( + "System Manifest:", manifest_repr + ) + self_mock.sa.save_manifest_json.assert_called_once_with( + json.dumps(to_save_manifest) + ) + + def test__save_manifest_non_interactive(self): + self_mock = mock.MagicMock() + manifest_repr = { + "Question 1": [ + {"id": "conf1", "value": "val1"}, + {"id": "conf2", "value": "val2"}, + ], + "Question 2": [{"id": "conf3", "value": "val3"}], + } + self_mock.sa.get_manifest_repr_json.return_value = json.dumps( + manifest_repr + ) + + RemoteController._save_manifest(self_mock, False) + + expected_to_save = { + "conf1": "val1", + "conf2": "val2", + "conf3": "val3", + } + self_mock.sa.save_manifest_json.assert_called_once_with( + json.dumps(expected_to_save) + ) + + def test_select_jobs_forced_with_manifest(self): + self_mock = mock.MagicMock() + self_mock.launcher.get_value.return_value = True + self_mock.launcher.manifest = True + + RemoteController.select_jobs(self_mock, []) + + self.assertTrue(self_mock._save_manifest.called) + self.assertTrue(self_mock.sa.finish_job_selection.called) + self.assertFalse(self_mock.sa.get_jobs_repr_json.called) + + @mock.patch("checkbox_ng.launcher.controller.CategoryBrowser") + def test_select_jobs_interactive_modified(self, category_browser_mock): + self_mock = mock.MagicMock() + self_mock.launcher.get_value.return_value = False + all_jobs = ["job1", "job2", "job3"] + self_mock.sa.get_jobs_repr_json.return_value = json.dumps(all_jobs) + wanted_set = {"job1", "job3"} + category_browser_mock.return_value.run.return_value = wanted_set + + RemoteController.select_jobs(self_mock, all_jobs) + + self.assertTrue(category_browser_mock.called) + self.assertTrue(self_mock.sa.modify_todo_list_json.called) + self.assertTrue(self_mock._save_manifest.called) + self.assertTrue(self_mock.sa.finish_job_selection.called) + + @mock.patch("checkbox_ng.launcher.controller.CategoryBrowser") + def test_select_jobs_interactive_not_modified(self, category_browser_mock): + self_mock = mock.MagicMock() + self_mock.launcher.get_value.return_value = False + all_jobs = ["job1", "job2", "job3"] + self_mock.sa.get_jobs_repr_json.return_value = json.dumps(all_jobs) + wanted_set = {"job1", "job2", "job3"} + category_browser_mock.return_value.run.return_value = wanted_set + + RemoteController.select_jobs(self_mock, all_jobs) + + self.assertTrue(category_browser_mock.called) + self.assertFalse(self_mock.sa.modify_todo_list_json.called) + self.assertTrue(self_mock._save_manifest.called) + self.assertTrue(self_mock.sa.finish_job_selection.called) + + @mock.patch("time.sleep") + def test_wait_for_job_finishes_on_non_running_state(self, sleep_mock): + self_mock = mock.MagicMock() + self_mock.sa.monitor_job.side_effect = [("completed", None)] + + RemoteController.wait_for_job(self_mock, dont_finish=False) + + self.assertTrue(self_mock.finish_job.called) + self.assertFalse(sleep_mock.called) + + @mock.patch("select.select") + @mock.patch("time.sleep") + def test_wait_for_job_sleeps_increasingly(self, sleep_mock, select_mock): + self_mock = mock.MagicMock() + select_mock.return_value = ([], [], []) + self_mock.sa.monitor_job.side_effect = [ + ("running", None), + ("running", None), + ("running", None), + ("running", None), + ("running", None), + ("finished", None), + ] + + RemoteController.wait_for_job(self_mock) + + self.assertTrue(sleep_mock.called) + calls = sleep_mock.call_args_list + self.assertEqual(calls[0], mock.call(0)) + self.assertEqual(calls[1], mock.call(0.1)) + self.assertEqual(calls[2], mock.call(0.2)) + self.assertEqual(calls[3], mock.call(0.5)) + self.assertEqual(calls[4], mock.call(0.5)) + self.assertTrue(self_mock.finish_job.called) + + @mock.patch("checkbox_ng.launcher.controller.SimpleUI") + def test_wait_for_job_with_payload(self, simple_ui_mock): + self_mock = mock.MagicMock() + self_mock._is_bootstrapping = False + payload = "stdout:OK\nstderr:ERROR" + self_mock.sa.monitor_job.side_effect = [ + ("finished", payload), + ] + + RemoteController.wait_for_job(self_mock) + + self.assertTrue(self_mock.sa.monitor_job.called) + self.assertTrue(simple_ui_mock.green_text.called) + self.assertTrue(simple_ui_mock.red_text.called) + self.assertTrue(self_mock.finish_job.called) + class IsHostnameALoopbackTests(TestCase): @mock.patch("socket.gethostbyname") From 731f26da29a21e06e0a2ff3991a2e67f4dad382b Mon Sep 17 00:00:00 2001 From: Hook25 Date: Wed, 9 Jul 2025 14:31:11 +0200 Subject: [PATCH 12/13] Document why we use the api to fetch the Configuration type --- checkbox-ng/checkbox_ng/launcher/stages.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/checkbox-ng/checkbox_ng/launcher/stages.py b/checkbox-ng/checkbox_ng/launcher/stages.py index 8ddc695f27..2c0905ede2 100644 --- a/checkbox-ng/checkbox_ng/launcher/stages.py +++ b/checkbox-ng/checkbox_ng/launcher/stages.py @@ -454,6 +454,11 @@ def _override_exporting(self, export_fn): def _prepare_stock_report(self, report): try: + # This function is called by both remote and local. Local sa + # doesn't have this API so it will fallback to the "normal" + # Configuration type. In remote we get the agent Configuration type + # because the object is passed and mostly used by the agent, so + # creating this object as a netref saves a lot of rpyc calls ConfigurationType = self.sa.configuration_type() except AttributeError: ConfigurationType = Configuration From 0ff8f2451e753d6662d3a3128fbbd494c1dd190a Mon Sep 17 00:00:00 2001 From: Hook25 Date: Wed, 9 Jul 2025 14:31:53 +0200 Subject: [PATCH 13/13] Correct double bump done by mistake Turns out, 13 + 1 = 14, not 15 --- checkbox-ng/plainbox/impl/session/remote_assistant.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkbox-ng/plainbox/impl/session/remote_assistant.py b/checkbox-ng/plainbox/impl/session/remote_assistant.py index 4fe6f836cc..942a2dd709 100644 --- a/checkbox-ng/plainbox/impl/session/remote_assistant.py +++ b/checkbox-ng/plainbox/impl/session/remote_assistant.py @@ -154,7 +154,7 @@ class RemoteSessionAssistant: object but JSON encoded. """ - REMOTE_API_VERSION = 15 + REMOTE_API_VERSION = 14 def __init__(self, cmd_callback): _logger.debug("__init__()")