Merge lp://qastaging/~3v1n0/unity/avoid-duplicate-icons into lp://qastaging/unity

Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Didier Roche-Tolomelli
Approved revision: no longer in the source branch.
Merged at revision: 2345
Proposed branch: lp://qastaging/~3v1n0/unity/avoid-duplicate-icons
Merge into: lp://qastaging/unity
Diff against target: 226 lines (+84/-17)
6 files modified
plugins/unityshell/src/BamfLauncherIcon.cpp (+2/-1)
plugins/unityshell/src/LauncherController.cpp (+4/-3)
tests/autopilot/autopilot/emulators/bamf.py (+10/-0)
tests/autopilot/autopilot/emulators/unity/launcher.py (+13/-11)
tests/autopilot/autopilot/tests/__init__.py (+6/-2)
tests/autopilot/autopilot/tests/test_launcher.py (+49/-0)
To merge this branch: bzr merge lp://qastaging/~3v1n0/unity/avoid-duplicate-icons
Reviewer Review Type Date Requested Status
Michal Hruby (community) Approve
Tim Penhey (community) Approve
Marco Trevisan (Treviño) Pending
Thomi Richards Pending
Review via email: mp+103611@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2012-04-26.

Commit message

We need to flag a bamf view that we control using an icon using the "unity-seen" qdata, or unity will duplicate it on bamfdaemon respawn. (LP: #928912)

Description of the change

We need to flag a bamf view that we control using an icon using the "unity-seen" qdata, or unity will duplicate it on bamfdaemon respawn.

AP tests included.

This MP was superseeded to use thomi's test fixes from lp:~thomir/unity/avoid-duplicate-icons

To post a comment you must log in.
Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote : Posted in a previous version of this proposal

Hi,

Please try and replace the call() lines with this:

call("kill `pidof %s`" % (app['process-name']), shell=True)

Don't use pkill -f - it's dangerous. Once making these changes, please ensure that all the tests still pass.

130 + self.start_app("Calculator")
131 + self.start_app("System Settings")
132 + os.spawnlp(os.P_NOWAIT, "xterm", "xterm", "-title", "Autopilot XTerm", "-e", "sh")
133 + self.addCleanup(call, ["killall", "xterm"])
134 +
135 + # FIXME bamf emulator should wait until a window is open
136 + sleep(1)
137 + [xterm_win] = [w for w in self.bamf.get_open_windows() if w.name == "Autopilot XTerm"]
138 + self.assertTrue(xterm_win.is_focused)

Please put all this in a method called "start_test_apps or something similar, then call it from your test.

154 + same_desktop = [i for i in bamf_icons if i.desktop_file == icon.desktop_file]
155 + self.assertThat(len(same_desktop), Equals(1))

Can you use 'get_icon_by_desktop_file' on the launcher model, to avoid this?

157 + same_appid = [i for i in bamf_icons if i.application_id == icon.application_id]
158 + self.assertThat(len(same_appid), Equals(1))

Similar thing here... we should probable add something to the launcher model like this:

def get_icon_by_filter(self, **kwargs):
        """Gets a launcher icon that matches the filter specified.

        Returns None if there is no such launcher icon.
        """
        icons = self.get_children_by_type(SimpleLauncherIcon, **kwargs)
        if len(icons):
            return icons[0]

        return None

review: Needs Fixing
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote : Posted in a previous version of this proposal

+1

review: Approve
Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote : Posted in a previous version of this proposal

This has a few problems:

1) I can't verify this without the libbamf fix (remember this isn't my branch, I'm just fixing the AP tests).
2) (possibly the same as above) - unity keeps the xclock icon around forever.

review: Needs Fixing
Revision history for this message
Tim Penhey (thumper) wrote :

Looks like the requested changes were done.

review: Approve
Revision history for this message
Unity Merger (unity-merger) wrote :

No commit message specified.

Revision history for this message
Michal Hruby (mhr3) wrote :

The unity side looks safe, tests pass with the not-yet-merged bamf branch.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.