Code review comment for lp://qastaging/~3v1n0/unity/hud-invalid-icon-fix

Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

Hi,

Great branch - a few small issues with the AP test though:

99 + self.keybinding("dash/reveal", 0.1)
100 + sleep(1)

Please use self.dash.ensure_visible() for this - it's safer, since it doesn't require you to sleep() after calling it.

105 + if self.hud.is_locked_launcher:
106 + hud_launcher_icon = self.get_hud_launcher_icon()
107 + self.assertThat(hud_launcher_icon.icon_name, Equals(calc.icon))
108 + else:
109 + hud_embedded_icon = self.hud.get_embedded_icon()
110 + self.assertThat(hud_embedded_icon.icon_name, Equals(calc.icon))

This seems to be something we're going to be doing a lot, so please just add a property to the Hud class that gets you the correct icon in all situations. THen these 6 lines become one:

self.assertThat(self.hud.icon.name, Equals(calc.icon))

... or something similar.

114 + self.start_app("Calculator")
115 + calctools = self.get_app_instances("Calculator")
116 + self.assertThat(len(calctools), GreaterThan(0))
117 + calc = calctools[0]
118 + self.assertTrue(calc.is_active)

This is ugly - and it's my fault. Please could you patch self.start_app(...) so it returns the BamfApplication instance? It shouldn't be hard to do, since we launch the application in the Bamf Emulator. THen you can replace all that code with this:

calc = self.start_app("Calculator")

Cheers,

review: Needs Fixing

« Back to merge proposal