Merge ~jocave/checkbox-ng:modify-bootstrap-output into checkbox-ng:master

Proposed by Jonathan Cave
Status: Merged
Approved by: Jonathan Cave
Approved revision: 85100bbb2f16d744446bd3a22b8cb746d2dadb18
Merged at revision: a5237c26ea652a37eeb17fbd6e47bdffefa60b17
Proposed branch: ~jocave/checkbox-ng:modify-bootstrap-output
Merge into: checkbox-ng:master
Diff against target: 35 lines (+3/-18)
1 file modified
checkbox_ng/launcher/stages.py (+3/-18)
Reviewer Review Type Date Requested Status
Sylvain Pineau (community) Approve
Sheila Miguez (community) Approve
Review via email: mp+335385@code.launchpad.net

Description of the change

Usefulness of bootstrap job output is probably quite subjective, however I found these changes useful in identifying possible issues during this stage of test runs.

Having the job id available means you ensure presence of particular job and ordering. The use of the PianoUI provides are alert that something might be wrong that could effect jobs in main body of the test run.

Estimated duration is almost useless in my experience.

Depends on:
https://code.launchpad.net/~jocave/plainbox/+git/plainbox/+merge/335384

To post a comment you must log in.
Revision history for this message
Sheila Miguez (codersquid) wrote :

Sounds reasonable.

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

Your patch highlights an existing bug, see below in the inline comment.

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

Since you're not using the estimated_time in the new version, let's completely remove those lines in _run_bootstrap_jobs():

        estimated_time = 0
        for job_id in jobs_to_run:
            job = self.sa.get_job(job_id)
            if (job.estimated_duration is not None and
                    estimated_time is not None):
                estimated_time += job.estimated_duration
            else:
                estimated_time = None

[...]

            if (job.estimated_duration is not None and
                    estimated_time is not None):
                estimated_time -= job.estimated_duration

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

Reviewed in light of good point that estimated_duration code could be removed. Last commit simplifies things a bit.

Let me know if these commits should be squashed.

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

Let's keep your branch history, I think it helps. Clear to land!

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/stages.py b/checkbox_ng/launcher/stages.py
index 93f4a8c..2836520 100644
--- a/checkbox_ng/launcher/stages.py
+++ b/checkbox_ng/launcher/stages.py
@@ -223,27 +223,12 @@ class MainLoopStage(metaclass=abc.ABCMeta):
223 estimated_time -= job.estimated_duration223 estimated_time -= job.estimated_duration
224224
225 def _run_bootstrap_jobs(self, jobs_to_run):225 def _run_bootstrap_jobs(self, jobs_to_run):
226 estimated_time = 0
227 for job_id in jobs_to_run:
228 job = self.sa.get_job(job_id)
229 if (job.estimated_duration is not None and
230 estimated_time is not None):
231 estimated_time += job.estimated_duration
232 else:
233 estimated_time = None
234 for job_no, job_id in enumerate(jobs_to_run, start=1):226 for job_no, job_id in enumerate(jobs_to_run, start=1):
235 print(self.C.header(227 print(self.C.header(
236 _('Running bootstrapping job {} / {}. Estimated time left: {}').format(228 _('Bootstrap {} ({}/{})').format(
237 job_no, len(jobs_to_run),229 job_id, job_no, len(jobs_to_run), fill='-')))
238 seconds_to_human_duration(max(0, estimated_time))230 result_builder = self.sa.run_job(job_id, 'piano', False)
239 if estimated_time is not None else _("unknown")),
240 fill='-'))
241 job = self.sa.get_job(job_id)
242 result_builder = self.sa.run_job(job.id, 'silent', False)
243 self.sa.use_job_result(job_id, result_builder.get_result())231 self.sa.use_job_result(job_id, result_builder.get_result())
244 if (job.estimated_duration is not None and
245 estimated_time is not None):
246 estimated_time -= job.estimated_duration
247232
248 def _pick_action_cmd(self, action_list, prompt=None):233 def _pick_action_cmd(self, action_list, prompt=None):
249 return ActionUI(action_list, prompt).run()234 return ActionUI(action_list, prompt).run()

Subscribers

People subscribed via source and target branches