Code review comment for lp:~3v1n0/unity/alt+tab-scroll-wheel

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

Looks good, but with a few things that need fixing.

1) The docstrings for next_icon_mouse and previous_icon_mouse should say: "icon" instead of "application".

2) If possible, we should use constsants to define the mouse buttons in unity. These must be defined in X somewhere?

3)Your tests need to be split up, like this:

107 + def test_switcher_scroll_next(self):
108 + self.set_timeout_setting(False)
109 + sleep(1)
110 +
111 + self.switcher.initiate()
112 + sleep(.2)
113 +
114 + start = self.switcher.get_selection_index()
115 + self.switcher.next_icon_mouse()
116 + sleep(.2)
117 +
118 + end = self.switcher.get_selection_index()
119 + self.assertThat(start, NotEquals(0))
120 + self.assertThat(end, Equals(start + 1))
121 +

def test_switcher_ignores_fast_wheel_down_events(self):
122 + # Quickly repeatead events should be ignored (except the first)
123 + start = self.switcher.get_selection_index()
124 + self.switcher.next_icon_mouse()
125 + self.switcher.next_icon_mouse()
126 + sleep(.2)
127 +
128 + end = self.switcher.get_selection_index()
129 + self.assertThat(end, Equals(start + 1))
130 +
131 + self.switcher.terminate()
132 +

133 + def test_switcher_scroll_prev(self):
134 + self.set_timeout_setting(False)
135 + sleep(1)
136 +
137 + self.switcher.initiate()
138 + sleep(.2)
139 +
140 + start = self.switcher.get_selection_index()
141 + self.switcher.previous_icon_mouse()
142 + sleep(.2)
143 +
144 + end = self.switcher.get_selection_index()
145 + self.assertThat(start, NotEquals(0))
146 + self.assertThat(end, Equals(start - 1))
147 +

def test_switcher_ignores_fast_wheel_down_events(self):
148 + # Quickly repeatead events should be ignored (except the first)
149 + start = self.switcher.get_selection_index()
150 + self.switcher.previous_icon_mouse()
151 + self.switcher.previous_icon_mouse()
152 + sleep(.2)
153 +
154 + end = self.switcher.get_selection_index()
155 + self.assertThat(end, Equals(start - 1))
156 +
157 + self.switcher.terminate()
158 + self.set_timeout_setting(True)
159 +

review: Needs Fixing

« Back to merge proposal