Checkbox

Merge lp://qastaging/~brendan-donegan/checkbox/bug1314516_checkbox-gui into lp://qastaging/checkbox

bug1314516_checkbox-gui
Merge into trunk

Proposed by Brendan Donegan on 2014-05-07

Status:

Merged

Approved by:

Zygmunt Krynicki on 2014-05-07

Approved revision:

2979

Merged at revision:

2981

Proposed branch:

lp://qastaging/~brendan-donegan/checkbox/bug1314516_checkbox-gui

Merge into:

lp://qastaging/checkbox

Diff against target:

87 lines (+14/-15)

2 files modified

checkbox-gui/checkbox-gui/testitemmodel.cpp (+12/-13)
plainbox/plainbox/impl/job.py (+2/-2)

To merge this branch:

bzr merge lp://qastaging/~brendan-donegan/checkbox/bug1314516_checkbox-gui

Medium

Fix Released

Link a bug report

Reviewer	Date Requested	Status
Zygmunt Krynicki (community)	2014-05-07	Approve on 2014-05-07
Brendan Donegan (community)		Needs Resubmitting on 2014-05-07
Review via email: mp+218608@code.qastaging.launchpad.net

Description of the change

This branch updates the GUI to use the summary field of a job in all cases in the UI, falling back to the partial_id of the job if the summary is not available. For local jobs it will display the description where the summary is not available.

Revision history for this message

Zygmunt Krynicki (zyga) wrote on 2014-05-07:

Please split that to two commits, one per project

review: Needs Fixing

Revision history for this message

Zygmunt Krynicki (zyga) wrote on 2014-05-07:

I also wonder if plainbox tests pass with those changes, perhaps we just have spotty coverage

lp://qastaging/~brendan-donegan/checkbox/bug1314516_checkbox-gui updated on 2014-05-07

2978. By Brendan Donegan on 2014-05-07

plainbox: If summary does not exist, use partial_id

Signed-off-by: Brendan Donegan <email address hidden>

2979. By Brendan Donegan on 2014-05-07

checkbox-gui: Display summary where possible, otherwise partial_id

To make the UI of plainbox based tools consistent, let's use summary when
displaying jobs. However these is currently no guarantee summary is there
for any particular job, so we need to fall back to using partial_id.

To keep behaviour consistent, local jobs should fall back to using
description instead of partial_id.

Signed-off-by: Brendan Donegan <email address hidden>

Revision history for this message

Brendan Donegan (brendan-donegan) wrote on 2014-05-07:

Rebased. Overall coverage is 52%, but in e.g. job.py it's 91%, so there might be room for improvement, but it's not that bad.

review: Needs Resubmitting

Revision history for this message

Zygmunt Krynicki (zyga) wrote on 2014-05-07:

26 + variant = *iface->properties.find("summary");
27 + if (variant.isValid() && variant.canConvert(QMetaType::QString) ) {
28 + testname = variant.toString();
29 + }
30 +

When would that be false? We always return something for summary

review: Needs Information

Revision history for this message

Brendan Donegan (brendan-donegan) wrote on 2014-05-07:

That's just the pattern used in the current code - it's probably better to be defensive, don't you think?

review: Needs Resubmitting

Revision history for this message

Zygmunt Krynicki (zyga) wrote on 2014-05-07:

Ok, I see, I was just wondering if there was something else to it.

Revision history for this message

Zygmunt Krynicki (zyga) wrote on 2014-05-07: