Unity

Merge lp://qastaging/~azzar1/unity/fix-778499 into lp://qastaging/unity

fix-778499
Merge into trunk

Proposed by Andrea Azzarone on 2012-04-24

Status:

Merged

Approved by:

Marco Trevisan (Treviño) on 2012-04-24

Approved revision:

no longer in the source branch.

Merged at revision:

2335

Proposed branch:

lp://qastaging/~azzar1/unity/fix-778499

Merge into:

lp://qastaging/unity

Diff against target:

121 lines (+37/-3)

5 files modified

plugins/unityshell/src/AbstractLauncherIcon.h (+1/-0)
plugins/unityshell/src/LauncherController.cpp (+6/-3)
plugins/unityshell/src/LauncherIcon.cpp (+6/-0)
tests/autopilot/autopilot/emulators/unity/launcher.py (+11/-0)
tests/autopilot/autopilot/tests/test_launcher.py (+13/-0)

To merge this branch:

bzr merge lp://qastaging/~azzar1/unity/fix-778499

Low

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Marco Trevisan (Treviño)		2012-04-24	Approve on 2012-04-24
Review via email: mp+103327@code.qastaging.launchpad.net

Commit message

Set a shortcut for the new apps on the launcher too.

Description of the change

== Problem ==
New apps on the launcher have no shortcut key until something is being closed.

== Fix ==
Call LauncherController::SortAndUpdate when a new bamf icon has been added to the launcher.

== Test ==
App test added.
./tools/autopilot run autopilot.tests.test_launcher.LauncherRevealTests.test_new_icon_has_the_shortcut

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2012-04-24:

Unity code looks good to me.

The test needs some tuning:

111 + desktop_file = self.KNOWN_APPS['Calculator']['desktop-file']
112 + if self.launcher.model.get_icon_by_desktop_id(desktop_file) != None:
113 + self.skip("Calculator icon is already on the launcher.")
114 +
115 + self.start_app('Calculator')
116 + icon = self.launcher.model.get_icon_by_desktop_id(desktop_file)
117 + self.assertThat(icon.shortcut, GreaterThan(0))

Instead of skipping the test when the calculator is already opened, just close it and you can just do:

self.close_all_app('Calculator')
calc = self.start_app('Calculator')
icon = self.launcher.model.get_icon_by_desktop_id(calc.desktop_file)
self.assertThat(icon.shortcut, Eventually(GreaterThan(0)))

(using eventually to avoid false-negatives)

review: Needs Fixing

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2012-04-24:

> self.close_all_app('Calculator')

Put this even before the "num_bamf_launcher_icons" check, so that it won't fail just for that icon ;)

Revision history for this message

Andrea Azzarone (azzar1) wrote on 2012-04-24:

> Unity code looks good to me.
>
> The test needs some tuning:
>
> 111 + desktop_file = self.KNOWN_APPS['Calculator']['desktop-file']
> 112 + if self.launcher.model.get_icon_by_desktop_id(desktop_file)
> != None:
> 113 + self.skip("Calculator icon is already on the launcher.")
> 114 +
> 115 + self.start_app('Calculator')
> 116 + icon =
> self.launcher.model.get_icon_by_desktop_id(desktop_file)
> 117 + self.assertThat(icon.shortcut, GreaterThan(0))
>
> Instead of skipping the test when the calculator is already opened, just close
> it and you can just do:
>
> self.close_all_app('Calculator')
> calc = self.start_app('Calculator')
> icon = self.launcher.model.get_icon_by_desktop_id(calc.desktop_file)
> self.assertThat(icon.shortcut, Eventually(GreaterThan(0)))
>
> (using eventually to avoid false-negatives)

If the Calculator is pinned, closing it will not remove the icon from the launcher.

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2012-04-24:

You're right... But you can still check for that...

Revision history for this message

Andrea Azzarone (azzar1) wrote on 2012-04-24:

Yeah but keep in mind that tests should be as clean as possible, and adding another if statement will add clutter to the test. But I will do as you want...

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2012-04-24:

Ok.. fair enough.

review: Approve