Merge lp:~nskaggs/ubuntu-calendar-app/fix-ap-flo into lp:ubuntu-calendar-app

Proposed by Nicholas Skaggs on 2014-05-17
Status: Merged
Approved by: Kunal Parmar on 2014-05-23
Approved revision: 283
Merged at revision: 284
Proposed branch: lp:~nskaggs/ubuntu-calendar-app/fix-ap-flo
Merge into: lp:ubuntu-calendar-app
Diff against target: 336 lines (+105/-49)
6 files modified
tests/autopilot/calendar_app/emulators.py (+61/-0)
tests/autopilot/calendar_app/tests/__init__.py (+9/-1)
tests/autopilot/calendar_app/tests/test_calendar.py (+3/-3)
tests/autopilot/calendar_app/tests/test_dayview.py (+6/-6)
tests/autopilot/calendar_app/tests/test_monthview.py (+10/-10)
tests/autopilot/calendar_app/tests/test_yearview.py (+16/-29)
To merge this branch: bzr merge lp:~nskaggs/ubuntu-calendar-app/fix-ap-flo
Reviewer Review Type Date Requested Status
Kunal Parmar 2014-05-17 Approve on 2014-05-23
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve on 2014-05-23
Review via email: mp+219925@code.launchpad.net

Commit message

Description of the change

This change cleans up the calendar app tests, fixes the apparmor issue

To post a comment you must log in.
275. By Nicholas Skaggs on 2014-05-17

flake8 fixes

276. By Nicholas Skaggs on 2014-05-17

migrate emulator to function object

Nicholas Skaggs (nskaggs) wrote :

Whelp, I moved this to under /home/phablet and it's still whining. What gives?

May 17 01:16:59 ubuntu-phablet kernel: [10350.034301] type=1400 audit(1400289419.632:2623): apparmor="DENIED" operation="mkdir" parent=1655 profile="com.ubuntu.calendar_calendar_0.4.271" name="/home/phablet/tmp8vw2onxo/.config/" pid=26208 comm="qmlscene" requested_mask="c" denied_mask="c" fsuid=32011 ouid=32011

277. By Nicholas Skaggs on 2014-05-17

restore old swipe behavoir

278. By Nicholas Skaggs on 2014-05-19

apparmor fix; switch /tmp source dir

Jamie Strandboge (jdstrand) wrote :

The merge is using:
    temp_dir_fixture = fixtures.TempDir(os.environ['XDG_RUNTIME_DIR'])

but why wouldn't it use:
    temp_dir_fixture = fixtures.TempDir(os.environ['TMPDIR'])

TMPDIR is set in click packaging as well, and seems somewhat more appropriate.

Jamie Strandboge (jdstrand) wrote :

From discussions on irc, if you are changing around the XDG variables, you need to coordinate that with apparmor policy. This could be done in with the addon dbus accesses for autopilot.

Current existing apparmor is:
  # Allow writes to various (application-specific) XDG directories
  owner @{HOME}/.cache/@{APP_PKGNAME}/ rw, # subdir of XDG_CACHE_HOME
  owner @{HOME}/.cache/@{APP_PKGNAME}/** mrwkl,
  owner @{HOME}/.config/@{APP_PKGNAME}/ rw, # subdir of XDG_CONFIG_HOME
  owner @{HOME}/.config/@{APP_PKGNAME}/** mrwkl,
  owner @{HOME}/.local/share/@{APP_PKGNAME}/ rw, # subdir of XDG_DATA_HOME
  owner @{HOME}/.local/share/@{APP_PKGNAME}/** mrwklix,
  owner /{,var/}run/user/*/confined/@{APP_PKGNAME}/ rw, # subdir of XDG_RUNTIME_DIR
  owner /{,var/}run/user/*/confined/@{APP_PKGNAME}/** mrwkl,

So if you changed these XDG_* directories in the environment to something else, you should create addon policy. Eg:

If set these like so:
XDG_CACHE_HOME=$HOME/fake/.cache
XDG_CONFIG_HOME=$HOME/fake/.config
XDG_DATA_HOME=$HOME/fake/.local/share
XDG_RUNTIME_DIR=$XDG_RUNTIME_DIR/fake

Then need to add to autopilot add-on policy:
  # Allow writes to various (application-specific) XDG directories
  owner @{HOME}/fake/.cache/@{APP_PKGNAME}/ rw, # subdir of XDG_CACHE_HOME
  owner @{HOME}/fake/.cache/@{APP_PKGNAME}/** mrwkl,
  owner @{HOME}/fake/.config/@{APP_PKGNAME}/ rw, # subdir of XDG_CONFIG_HOME
  owner @{HOME}/fake/.config/@{APP_PKGNAME}/** mrwkl,
  owner @{HOME}/fake/.local/share/@{APP_PKGNAME}/ rw, # subdir of XDG_DATA_HOME
  owner @{HOME}/fake/.local/share/@{APP_PKGNAME}/** mrwklix,
  owner /{,var/}run/user/*/fake/confined/@{APP_PKGNAME}/ rw, # subdir of XDG_RUNTIME_DIR
  owner /{,var/}run/user/*/fake/confined/@{APP_PKGNAME}/** mrwkl,

The above is untested, but should work.

All that said, it is (still) my opinion that using an alternate user is the best option.

Nicholas Skaggs (nskaggs) wrote :

Jamie, so if we wanted to implement this, we would need to hardcode the 'fake' directory we want to use as part of the policy. I was hoping to have this directory be dynamic and temporary; aka not pre-named. This directory would then need to be used by any test wishing to run; hmm.

Jamie Strandboge (jdstrand) wrote :

It is a matter of security that the user is not able to modify the apparmor path directly. It must be done via a root process. Currently the phablet-tools will run 'aa-clickhook --include=/usr/share/autopilot-touch/apparmor/click.rules -f' as root before running the tests. My thought was that you would add these 'fake' paths to /usr/share/autopilot-touch/apparmor/click.rules so that when the phablet test tool is run, the paths are added to the profile accordingly.

It is possible to use globs though. So we can have rules like this:
  # Allow writes to various (application-specific) XDG directories
  owner /run/user/[0-9]*/autopilot/*/.cache/@{APP_PKGNAME}/ rw, # subdir of XDG_CACHE_HOME
  owner /run/user/[0-9]*/autopilot/*/.cache/@{APP_PKGNAME}/** mrwkl,
  owner /run/user/[0-9]*/autopilot/*/.config/@{APP_PKGNAME}/ rw, # subdir of XDG_CONFIG_HOME
  owner /run/user/[0-9]*/autopilot/*/.config/@{APP_PKGNAME}/** mrwkl,
  owner /run/user/[0-9]*/autopilot/*/.local/share/@{APP_PKGNAME}/ rw, # subdir of XDG_DATA_HOME
  owner /run/user/[0-9]*/autopilot/*/.local/share/@{APP_PKGNAME}/** mrwklix,
  owner /run/user/[0-9]*/autopilot/*/confined/@{APP_PKGNAME}/ rw, # subdir of XDG_RUNTIME_DIR
  owner /run/user/[0-9]*/autopilot/*/confined/@{APP_PKGNAME}/** mrwkl,

This way you could do could create a temporary directory (eg /run/user/32011/autopilot/tmpA34Hhh/) and do something like:
XDG_CACHE_HOME=/run/user/32011/autopilot/tmpA34Hhh/.cache
XDG_CONFIG_HOME=/run/user/32011/autopilot/tmpA34Hhh/.config
XDG_DATA_HOME=/run/user/32011/autopilot/tmpA34Hhh/.local/share
XDG_RUNTIME_DIR=/run/user/32011/autopilot/tmpA34Hhh/

Would this work better?

279. By Nicholas Skaggs on 2014-05-22

apparmor fixes

Nicholas Skaggs (nskaggs) wrote :

/run/usr/32011/autopilot doesn't exist, but we can create

Using this path; os.path.join(os.environ['XDG_RUNTIME_DIR'], 'autopilot')

280. By Nicholas Skaggs on 2014-05-22

merge with trunk

281. By Nicholas Skaggs on 2014-05-22

flake8 fixes

282. By Nicholas Skaggs on 2014-05-22

fix func name

283. By Nicholas Skaggs on 2014-05-23

change path to follow new click rules

Kunal Parmar (pkunal-parmar) wrote :

looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/autopilot/calendar_app/emulators.py'
2--- tests/autopilot/calendar_app/emulators.py 2014-05-21 13:05:14 +0000
3+++ tests/autopilot/calendar_app/emulators.py 2014-05-23 08:12:33 +0000
4@@ -6,8 +6,10 @@
5 # by the Free Software Foundation.
6
7 """Calendar app autopilot emulators."""
8+from time import sleep
9
10 from autopilot.introspection import dbus
11+
12 from ubuntuuitoolkit import emulators as toolkit_emulators
13 from dateutil import tz
14
15@@ -79,6 +81,25 @@
16 except dbus.StateNotFoundError:
17 return None
18
19+ def safe_swipe_view(self, direction, view, date):
20+ """
21+ direction: direction to swip
22+ view: the view you are swiping against
23+ date: a function object of the view
24+ """
25+ timeout = 0
26+ before = date
27+ #try up to 3 times to swipe
28+ while timeout < 3 and date == before:
29+ self._swipe(direction, view)
30+ #check for up to 3 seconds after swipe for view
31+ #to have changed before trying again
32+ for x in range(0, 3):
33+ if date != before:
34+ break
35+ sleep(1)
36+ timeout += 1
37+
38 def swipe_view(self, direction, view, x_pad=0.15):
39 """Swipe the given view to left or right.
40
41@@ -122,3 +143,43 @@
42 utc = date.replace(tzinfo=tz.tzutc())
43 local = utc.astimezone(tz.tzlocal())
44 return local
45+
46+
47+class Page(toolkit_emulators.UbuntuUIToolkitEmulatorBase):
48+ """Autopilot helper for Pages."""
49+
50+ def __init__(self, *args):
51+ super(Page, self).__init__(*args)
52+ # XXX we need a better way to keep reference to the main view.
53+ # --elopio - 2014-01-31
54+ self.main_view = self.get_root_instance().select_single(MainView)
55+
56+ def drag_page_up(self):
57+ """Drag the given page up."""
58+ self._drag_page(direction='up')
59+
60+ def drag_page_down(self):
61+ """Drag the given page down."""
62+ self._drag_page(direction='down')
63+
64+ def _drag_page(self, direction):
65+ """Function to drag the page up/down."""
66+ self._wait_to_stop_moving()
67+
68+ x, y, w, h = self.globalRect
69+ start_x = stop_x = x + (w / 2)
70+ start_y = y + (h / 2)
71+
72+ if direction == "down":
73+ stop_y = start_y + h / 3
74+ self.pointing_device.drag(start_x, start_y, stop_x, stop_y)
75+ else:
76+ stop_y = start_y - h / 3
77+ self.pointing_device.drag(start_x, start_y, stop_x, stop_y)
78+
79+ self._wait_to_stop_moving()
80+
81+ def _wait_to_stop_moving(self):
82+ self.select_single(
83+ 'QQuickFlickable',
84+ objectName='animationContainer').moving.wait_for(False)
85
86=== modified file 'tests/autopilot/calendar_app/tests/__init__.py'
87--- tests/autopilot/calendar_app/tests/__init__.py 2014-05-08 17:04:33 +0000
88+++ tests/autopilot/calendar_app/tests/__init__.py 2014-05-23 08:12:33 +0000
89@@ -117,9 +117,17 @@
90 def _patch_home(self):
91 """ mock /home for testing purposes to preserve user data
92 """
93- temp_dir_fixture = fixtures.TempDir()
94+ #click requires apparmor profile, and writing to special dir
95+ #but the desktop can write to a traditional /tmp directory
96+ if self.test_type == 'click':
97+ temp_dir = os.path.join('~', 'autopilot', 'fakeenv')
98+ logger.debug(temp_dir)
99+ temp_dir_fixture = fixtures.TempDir(temp_dir)
100+ else:
101+ temp_dir_fixture = fixtures.TempDir()
102 self.useFixture(temp_dir_fixture)
103 temp_dir = temp_dir_fixture.path
104+ logger.debug(temp_dir)
105
106 #If running under xvfb, as jenkins does,
107 #xsession will fail to start without xauthority file
108
109=== modified file 'tests/autopilot/calendar_app/tests/test_calendar.py'
110--- tests/autopilot/calendar_app/tests/test_calendar.py 2014-04-29 15:43:44 +0000
111+++ tests/autopilot/calendar_app/tests/test_calendar.py 2014-05-23 08:12:33 +0000
112@@ -20,7 +20,7 @@
113
114 class TestMainView(CalendarTestCase):
115
116- def scroll_time_picker_to_time(self, picker, hours, minutes):
117+ def _scroll_time_picker_to_time(self, picker, hours, minutes):
118 # Scroll hours to selected value
119 scroller = picker.select_single("Scroller", objectName="hourScroller")
120 x = int(scroller.globalRect[0] + scroller.globalRect[2] / 2)
121@@ -68,7 +68,7 @@
122 start_time_field = self.main_view.get_event_start_time_field()
123 self.pointing_device.click_object(start_time_field)
124 picker = self.main_view.get_time_picker()
125- self.scroll_time_picker_to_time(picker, 12, 28)
126+ self._scroll_time_picker_to_time(picker, 12, 28)
127 ok = picker.select_single("Button", objectName="TimePickerOKButton")
128 self.pointing_device.click_object(ok)
129
130@@ -76,7 +76,7 @@
131 end_time_field = self.main_view.get_event_end_time_field()
132 self.pointing_device.click_object(end_time_field)
133 picker = self.main_view.get_time_picker()
134- self.scroll_time_picker_to_time(picker, 13, 38)
135+ self._scroll_time_picker_to_time(picker, 13, 38)
136 ok = picker.select_single("Button", objectName="TimePickerOKButton")
137 self.pointing_device.click_object(ok)
138
139
140=== modified file 'tests/autopilot/calendar_app/tests/test_dayview.py'
141--- tests/autopilot/calendar_app/tests/test_dayview.py 2014-05-21 12:35:25 +0000
142+++ tests/autopilot/calendar_app/tests/test_dayview.py 2014-05-23 08:12:33 +0000
143@@ -71,13 +71,13 @@
144
145 def test_show_next_days(self):
146 """It must be possible to show next days by swiping the view."""
147- self.change_days(1)
148+ self._change_days(1)
149
150 def test_show_previous_days(self):
151 """It must be possible to show previous days by swiping the view."""
152- self.change_days(-1)
153+ self._change_days(-1)
154
155- def change_days(self, direction):
156+ def _change_days(self, direction):
157 firstday = self.day_view.currentDay.datetime
158
159 for i in range(1, 5):
160@@ -92,8 +92,8 @@
161 expected_day = (firstday + datetime.timedelta(
162 days=(i * direction)))
163
164- self.assertThat(self.strip(current_day),
165- Equals(self.strip(expected_day)))
166+ self.assertThat(self._strip_time(current_day),
167+ Equals(self._strip_time(expected_day)))
168
169- def strip(self, date):
170+ def _strip_time(self, date):
171 return date.replace(hour=0, minute=0, second=0, microsecond=0)
172
173=== modified file 'tests/autopilot/calendar_app/tests/test_monthview.py'
174--- tests/autopilot/calendar_app/tests/test_monthview.py 2014-05-21 13:05:14 +0000
175+++ tests/autopilot/calendar_app/tests/test_monthview.py 2014-05-23 08:12:33 +0000
176@@ -32,9 +32,9 @@
177
178 self.month_view = self.main_view.get_month_view()
179
180- def change_month(self, delta):
181+ def _change_month(self, delta):
182 month_view = self.main_view.get_month_view()
183- sign = int(math.copysign(1, delta))
184+ direction = int(math.copysign(1, delta))
185
186 for _ in range(abs(delta)):
187 before = self.main_view.to_local_date(
188@@ -44,7 +44,7 @@
189 old_month = self.main_view.to_local_date(
190 month_view.currentMonth.datetime)
191
192- self.main_view.swipe_view(sign, month_view)
193+ self.main_view.swipe_view(direction, month_view)
194
195 month_after = self.main_view.to_local_date(
196 month_view.currentMonth.datetime)
197@@ -52,7 +52,7 @@
198 self.assertThat(lambda: month_after,
199 Eventually(NotEquals(old_month)))
200
201- after = before + relativedelta(months=sign)
202+ after = before + relativedelta(months=direction)
203
204 self.assertThat(lambda:
205 month_after.month,
206@@ -73,20 +73,20 @@
207 self.assertThat(lambda: local.year,
208 Eventually(Equals(today.year)))
209
210- def _test_go_to_today(self, delta):
211+ def _go_to_today(self, delta):
212 self._assert_today()
213- self.change_month(delta)
214+ self._change_month(delta)
215 self.main_view.open_toolbar().click_button("todaybutton")
216 self._assert_today()
217
218 def test_monthview_go_to_today_next_month(self):
219- self._test_go_to_today(1)
220+ self._go_to_today(1)
221
222 def test_monthview_go_to_today_prev_month(self):
223- self._test_go_to_today(-1)
224+ self._go_to_today(-1)
225
226 def test_monthview_go_to_today_next_year(self):
227- self._test_go_to_today(12)
228+ self._go_to_today(12)
229
230 def test_monthview_go_to_today_prev_year(self):
231- self._test_go_to_today(-12)
232+ self._go_to_today(-12)
233
234=== modified file 'tests/autopilot/calendar_app/tests/test_yearview.py'
235--- tests/autopilot/calendar_app/tests/test_yearview.py 2014-05-21 12:35:25 +0000
236+++ tests/autopilot/calendar_app/tests/test_yearview.py 2014-05-23 08:12:33 +0000
237@@ -33,7 +33,7 @@
238
239 self.year_view = self.main_view.get_year_view()
240
241- def get_current_year(self):
242+ def _get_current_year(self):
243 return self.year_view.select_single("QQuickGridView",
244 isCurrentItem=True)
245
246@@ -42,11 +42,13 @@
247
248 # TODO: the component indexed at 1 is the one currently displayed,
249 # investigate a way to validate this assumption visually.
250- year_grid = self.get_current_year()
251+ year_grid = self._get_current_year()
252 self.assertThat(year_grid, NotEquals(None))
253 months = year_grid.select_many("MonthComponent")
254 months.sort(key=lambda month: month.currentMonth)
255- self.assert_current_year_is_default_one(months[0])
256+ #check that current year is the default
257+ self.assertThat(self.main_view.get_year(months[0]),
258+ Equals(datetime.now().year))
259
260 february = months[1]
261 expected_month_name = self.main_view.get_month_name(february)
262@@ -74,8 +76,8 @@
263 def test_current_day_is_selected(self):
264 """The current day must be selected."""
265
266- current_month = self.current_month()
267- month_grid = current_month.select_single(objectName="monthGrid")
268+ _current_month = self._current_month()
269+ month_grid = _current_month.select_single(objectName="monthGrid")
270
271 # there could actually be two labels with
272 # the current day: one is the current day of the current month,
273@@ -93,54 +95,39 @@
274
275 def test_show_next_years(self):
276 """It must be possible to show next years by swiping the view."""
277- self.change_year(1)
278+ self._change_year(1)
279
280 def test_show_previous_years(self):
281 """It must be possible to show previous years by swiping the view."""
282- self.change_year(-1)
283+ self._change_year(-1)
284
285- def change_year(self, direction, how_many=5):
286+ def _change_year(self, direction, how_many=5):
287 current_year = datetime.now().year
288
289 for i in range(1, how_many):
290- #prevent timing issues with swiping
291 self.main_view.swipe_view(direction, self.year_view)
292
293 self.assertThat(
294 lambda: self.year_view.currentYear,
295 Eventually(Equals(current_year + (i * direction))))
296
297- def assert_current_year_is_default_one(self, month_component):
298- self.assertThat(self.main_view.get_year(month_component),
299- Equals(datetime.now().year))
300-
301- def current_month(self):
302+ def _current_month(self):
303 now = datetime.now()
304- current_month_name = now.strftime("%B")
305+ _current_month_name = now.strftime("%B")
306
307 # for months after June, we must scroll down the page to have
308 # the month components loaded in the view.
309 if now.month > 6:
310- self.drag_page_up()
311+ self.page.drag_page_up()
312
313- year_grid = self.get_current_year()
314+ year_grid = self._get_current_year()
315 self.assertThat(year_grid, NotEquals(None))
316 months = year_grid.select_many("MonthComponent")
317
318 for month in months:
319- current_month_label = month.select_single(
320+ _current_month_label = month.select_single(
321 "Label", objectName="monthLabel")
322- if current_month_name == current_month_label.text:
323+ if _current_month_name == _current_month_label.text:
324 return month
325
326 return None
327-
328- def drag_page_up(self):
329- x_line = (self.year_view.globalRect[0] +
330- self.year_view.globalRect[2] / 2)
331-
332- y_stop = self.year_view.globalRect[1]
333- y_start = self.year_view.globalRect[3]
334-
335- self.pointing_device.drag(x_line, y_start, x_line, y_stop)
336- self.pointing_device.drag(x_line, y_start, x_line, y_stop)

Subscribers

People subscribed via source and target branches

to status/vote changes: