Merge ~sylvain-pineau/checkbox-ng:no_resource_no_templates into checkbox-ng:master

Proposed by Sylvain Pineau
Status: Merged
Approved by: Sylvain Pineau
Approved revision: 49f6aa4c42948470322bf32bcb5e5168ba6d2721
Merge reported by: Sylvain Pineau
Merged at revision: 49f6aa4c42948470322bf32bcb5e5168ba6d2721
Proposed branch: ~sylvain-pineau/checkbox-ng:no_resource_no_templates
Merge into: checkbox-ng:master
Diff against target: 97 lines (+39/-35)
2 files modified
checkbox_ng/launcher/subcommands.py (+36/-33)
plainbox/impl/ctrl.py (+3/-2)
Reviewer Review Type Date Requested Status
Jonathan Cave (community) Approve
Sylvain Pineau (community) Approve
Review via email: mp+355777@code.launchpad.net

Description of the change

Fix the crash observed while running the sru test plan on pi2 where the graphics_card resource job returns nothing. Templates were still checked as checkbox then creates an empty record (to avoid another kind of crash ; IIRC "package.name == foo" checks on UC).

The proposed fix is to simply bypass instantiate all when such fake record is the only resource object known for this resource job.

Bonus a commit to handle KeyboardInterrrupt with the Run command.

To post a comment you must log in.
Revision history for this message
Jonathan Cave (jocave) wrote :

On the other MR that triggered this you say:

> The proposed solution fixes the crash and removes all those warnings about missing parameters.

So does this actually only remove the warnings when the fake record is the only resource object? i.e. there are still warnings if there is a real resource object but the key the job is using is missing? I think we want to keep those warnings as it helps debug job creation.

review: Needs Information
Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

Tested by adding a typo (s/name/foo)in the following job def:

unit: template
template-resource: fwts
plugin: shell
category_id: com.canonical.plainbox::firmware
id: firmware/fwts_{name}
estimated_duration: 1.2
requires: executable.name == 'fwts'
user: root
command: checkbox-support-fwts_test -t {foo} -l $PLAINBOX_SESSION_SHARE/fwts_{name}.log
_description: Run {name} test from Firmware Test Suite.
_summary: Run {name} test from Firmware Test Suite.

Checkbox still complains and issues the following warnings:

$ checkbox-cli run com.canonical.certification::client-cert-18-04
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
Ignoring firmware/fwts_{name} with missing template parameter foo
=========================[ Running Selected Test Plan ]=========================
-------------[ Running job 1 / 347. Estimated time left: unknown ]--------------
---------------[ Collect the hardware manifest (interactively) ]----------------
ID: com.canonical.plainbox::collect-manifest
Category: com.canonical.plainbox::uncategorised

Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

Landing this one to avoid failing SRU tests

review: Approve
Revision history for this message
Jonathan Cave (jocave) wrote :

Thanks for the extra info, the warnings are still present when required by the looks of it.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/checkbox_ng/launcher/subcommands.py b/checkbox_ng/launcher/subcommands.py
index 8dd3873..87e4698 100644
--- a/checkbox_ng/launcher/subcommands.py
+++ b/checkbox_ng/launcher/subcommands.py
@@ -756,40 +756,43 @@ class Run(Command, MainLoopStage):
756 self.ctx.args.non_interactive)756 self.ctx.args.non_interactive)
757757
758 def invoked(self, ctx):758 def invoked(self, ctx):
759 self._C = Colorizer()759 try:
760 self.ctx = ctx760 self._C = Colorizer()
761 ctx.sa = SessionAssistant(761 self.ctx = ctx
762 "com.canonical:checkbox-cli",762 ctx.sa = SessionAssistant(
763 self.get_cmd_version(),763 "com.canonical:checkbox-cli",
764 "0.99",764 self.get_cmd_version(),
765 ["restartable"],765 "0.99",
766 )766 ["restartable"],
767 self._configure_restart()767 )
768 try_selecting_providers(self.sa, '*')768 self._configure_restart()
769 self.sa.start_new_session('checkbox-run')769 try_selecting_providers(self.sa, '*')
770 tps = self.sa.get_test_plans()770 self.sa.start_new_session('checkbox-run')
771 self._configure_report()771 tps = self.sa.get_test_plans()
772 selection = ctx.args.PATTERN772 self._configure_report()
773 if len(selection) == 1 and selection[0] in tps:773 selection = ctx.args.PATTERN
774 self.ctx.sa.update_app_blob(json.dumps(774 if len(selection) == 1 and selection[0] in tps:
775 {'testplan_id': selection[0]}).encode("UTF-8"))775 self.ctx.sa.update_app_blob(json.dumps(
776 self.just_run_test_plan(selection[0])776 {'testplan_id': selection[0]}).encode("UTF-8"))
777 else:777 self.just_run_test_plan(selection[0])
778 self.ctx.sa.update_app_blob(json.dumps(778 else:
779 {}).encode("UTF-8"))779 self.ctx.sa.update_app_blob(json.dumps(
780 self.sa.hand_pick_jobs(selection)780 {}).encode("UTF-8"))
781 print(self.C.header(_("Running Selected Jobs")))781 self.sa.hand_pick_jobs(selection)
782 self._run_jobs(self.sa.get_dynamic_todo_list())782 print(self.C.header(_("Running Selected Jobs")))
783 # there might have been new jobs instantiated
784 while True:
785 self.sa.hand_pick_jobs(ctx.args.PATTERN)
786 todos = self.sa.get_dynamic_todo_list()
787 if not todos:
788 break
789 self._run_jobs(self.sa.get_dynamic_todo_list())783 self._run_jobs(self.sa.get_dynamic_todo_list())
790 self.sa.finalize_session()784 # there might have been new jobs instantiated
791 self._print_results()785 while True:
792 return 0 if self.sa.get_summary()['fail'] == 0 else 1786 self.sa.hand_pick_jobs(ctx.args.PATTERN)
787 todos = self.sa.get_dynamic_todo_list()
788 if not todos:
789 break
790 self._run_jobs(self.sa.get_dynamic_todo_list())
791 self.sa.finalize_session()
792 self._print_results()
793 return 0 if self.sa.get_summary()['fail'] == 0 else 1
794 except KeyboardInterrupt:
795 return 1
793796
794 def just_run_test_plan(self, tp_id):797 def just_run_test_plan(self, tp_id):
795 self.sa.select_test_plan(tp_id)798 self.sa.select_test_plan(tp_id)
diff --git a/plainbox/impl/ctrl.py b/plainbox/impl/ctrl.py
index 471dd5f..4aefe11 100644
--- a/plainbox/impl/ctrl.py
+++ b/plainbox/impl/ctrl.py
@@ -270,8 +270,9 @@ class CheckBoxSessionStateController(ISessionStateController):
270 or replace resource records.270 or replace resource records.
271 """271 """
272 self._parse_and_store_resource(session_state, job, result)272 self._parse_and_store_resource(session_state, job, result)
273 self._instantiate_templates(273 if session_state.resource_map[job.id] != [Resource({})]:
274 session_state, job, result, fake_resources)274 self._instantiate_templates(
275 session_state, job, result, fake_resources)
275276
276 def _parse_and_store_resource(self, session_state, job, result):277 def _parse_and_store_resource(self, session_state, job, result):
277 # NOTE: https://bugs.launchpad.net/checkbox/+bug/1297928278 # NOTE: https://bugs.launchpad.net/checkbox/+bug/1297928

Subscribers

People subscribed via source and target branches