Merge lp:~ahayzen/music-app/add-url-dispatcher-tests into lp:music-app/trusty
- add-url-dispatcher-tests
- Merge into trusty
Status: | Work in progress |
---|---|
Proposed branch: | lp:~ahayzen/music-app/add-url-dispatcher-tests |
Merge into: | lp:music-app/trusty |
Diff against target: |
280 lines (+124/-31) 7 files modified
click/manifest.json.in (+7/-1) debian/control (+2/-0) music-app.qml (+24/-26) tests/autopilot/music_app/__init__.py (+6/-1) tests/autopilot/music_app/tests/__init__.py (+3/-3) tests/autopilot/music_app/tests/test_music.py (+60/-0) tests/autopilot/music_app/url_dispatcher.py (+22/-0) |
To merge this branch: | bzr merge lp:~ahayzen/music-app/add-url-dispatcher-tests |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Ubuntu Phone Apps Jenkins Bot | continuous-integration | Approve | |
Leo Arias (community) | code review of the autopilot code | Approve | |
Andrew Hayzen | Abstain | ||
Review via email: mp+233840@code.launchpad.net |
Commit message
* Add tests for url-dispatcher
* Fix for depends and allow autopkg support
* Fix for recent item added even with a bad url
Description of the change
* Add tests for url-dispatcher
* Fix for depends and allow autopkg support
* Fix for recent item added even with a bad url
TESTING:
This should be tested on device in the following way...
Install the new click package:
$ bzr branch lp:~andrew-hayzen/music-app/add-url-dispatcher-tests /tmp/add-
$ cd /tmp/add-
$ click-buddy --dir . --provision
To just run the changed tests:
$ ADT_AUTOPILOT_
$ ADT_AUTOPILOT_
To run all tests:
$ adt-run /tmp/add-
Note you need a newer adt if you are on trusty see instructions for install of autopkgtest here [0]
0 - http://
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
- 620. By Andrew Hayzen
-
* Move dispatcher to helper
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
FAILED: Continuous integration, rev:620
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 621. By Andrew Hayzen
-
* Merge of trunk
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
FAILED: Continuous integration, rev:621
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
FAILED: Continuous integration, rev:621
http://
Executed test runs:
UNSTABLE: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 622. By Andrew Hayzen
-
* Fix for use of old artistName
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
FAILED: Continuous integration, rev:622
http://
Executed test runs:
UNSTABLE: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 623. By Andrew Hayzen
-
* Fix for unicode errors
* Jenkins debug
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
FAILED: Continuous integration, rev:623
http://
Executed test runs:
UNSTABLE: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
FAILED: Continuous integration, rev:623
http://
Executed test runs:
UNSTABLE: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 624. By Andrew Hayzen
-
* Ensure url-dispatcher has started before calling it
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
FAILED: Continuous integration, rev:624
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 625. By Andrew Hayzen
-
* Pep8 fix
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
FAILED: Continuous integration, rev:625
http://
Executed test runs:
UNSTABLE: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 626. By Andrew Hayzen
-
* Only run tests when in click environment
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
PASSED: Continuous integration, rev:626
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Andrew Hayzen (ahayzen) wrote : | # |
#blocked as you cannot test due to bug 1370800
I've started resolving the comments, but we really need to fix ^^ bug first, otherwise you can't test if these works.
- 627. By Andrew Hayzen
-
* Various fixes for url-dispatcher
- 628. By Andrew Hayzen
-
* Merge of trunk
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
FAILED: Continuous integration, rev:628
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 629. By Andrew Hayzen
-
* Fixes for pep8
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
PASSED: Continuous integration, rev:629
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 630. By Andrew Hayzen
-
* Remove code comments
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
PASSED: Continuous integration, rev:630
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Leo Arias (elopio) wrote : | # |
Some pita comments inline. This is pretty good, thanks Andrew.
- 631. By Andrew Hayzen
-
* Various fixes
Andrew Hayzen (ahayzen) wrote : | # |
I would like to land this after this [0] has landed so blocking myself for a bit.
0 - https:/
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
PASSED: Continuous integration, rev:631
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 632. By Andrew Hayzen
-
* Merge of trunk
Andrew Hayzen (ahayzen) wrote : | # |
Unblocking myself as the other mp has landed.
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
PASSED: Continuous integration, rev:632
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Leo Arias (elopio) wrote : | # |
ahayzen_: as you added the decorator to the url dispatcher call, you don't need
235 + logger.
elopio, ah yes good spot ;)
ahayzen_: and last pita comment, pep8 recommends to capitalize all the letter of an abbreviation.
so URLDispatcher instead of UrlDispatcher.
elopio, ah yes ok i'll do that
ahayzen_: I'm really happy with your python code. I don't know much about the QML code, so it would be safer if you get somebody to review that part.
- 633. By Andrew Hayzen
-
* Removal of extra debugging
* Rename UrlDispatcher to URLDispatcher
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote : | # |
PASSED: Continuous integration, rev:633
http://
Executed test runs:
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Victor Thompson (vthompson) wrote : | # |
In my opinion we could do either of the following:
1. Finish review and merge this into trunk, then merge the change into the remix branch. Benefit being that the 2 added tests get some usage early on, negative being that this is extra work if something isn't quite right.
2. Create a new MP for this once remix is trunk. Benefit being we can still focus on our redesign without possible distraction.
#2 might be the safest, but #1 might be preferable if we want these tests in as early as possible.
Nicholas Skaggs (nskaggs) wrote : | # |
I'd vote to go with the second; heads down on remix. Thanks.
Victor Thompson (vthompson) wrote : | # |
I'll put this as a WIP as it will probably be resubmitted.
Unmerged revisions
- 633. By Andrew Hayzen
-
* Removal of extra debugging
* Rename UrlDispatcher to URLDispatcher - 632. By Andrew Hayzen
-
* Merge of trunk
- 631. By Andrew Hayzen
-
* Various fixes
- 630. By Andrew Hayzen
-
* Remove code comments
- 629. By Andrew Hayzen
-
* Fixes for pep8
- 628. By Andrew Hayzen
-
* Merge of trunk
- 627. By Andrew Hayzen
-
* Various fixes for url-dispatcher
- 626. By Andrew Hayzen
-
* Only run tests when in click environment
- 625. By Andrew Hayzen
-
* Pep8 fix
- 624. By Andrew Hayzen
-
* Ensure url-dispatcher has started before calling it
Preview Diff
1 | === modified file 'click/manifest.json.in' |
2 | --- click/manifest.json.in 2014-09-08 08:08:13 +0000 |
3 | +++ click/manifest.json.in 2014-09-25 16:56:36 +0000 |
4 | @@ -20,6 +20,12 @@ |
5 | "vcs-bzr-revno": "@BZR_REVNO@" |
6 | }, |
7 | "x-test": { |
8 | - "autopilot": "@AUTOPILOT_DIR@" |
9 | + "autopilot": { |
10 | + "autopilot_module": "@AUTOPILOT_DIR@", |
11 | + "depends": [ |
12 | + "python3-fixtures", |
13 | + "url-dispatcher-tools" |
14 | + ] |
15 | + } |
16 | } |
17 | } |
18 | |
19 | === modified file 'debian/control' |
20 | --- debian/control 2014-09-08 13:24:22 +0000 |
21 | +++ debian/control 2014-09-25 16:56:36 +0000 |
22 | @@ -39,7 +39,9 @@ |
23 | Depends: libautopilot-qt (>= 1.4), |
24 | libqt5test5, |
25 | music-app (= ${source:Version}), |
26 | + python3-fixtures, |
27 | ubuntu-ui-toolkit-autopilot, |
28 | + url-dispatcher-tools, |
29 | ${misc:Depends}, |
30 | Description: Autopilot tests for Music App |
31 | This package contains the autopilot tests for the Music App |
32 | |
33 | === modified file 'music-app.qml' |
34 | --- music-app.qml 2014-09-23 15:59:28 +0000 |
35 | +++ music-app.qml 2014-09-25 16:56:36 +0000 |
36 | @@ -204,22 +204,17 @@ |
37 | target: UriHandler |
38 | |
39 | function processAlbum(uri) { |
40 | - selectedAlbum = true; |
41 | var split = uri.split("/"); |
42 | |
43 | if (split.length < 2) { |
44 | console.debug("Unknown artist-album " + uri + ", skipping") |
45 | - return; |
46 | + } else { |
47 | + selectedAlbum = true; |
48 | + |
49 | + // Filter by artist and album |
50 | + songsAlbumArtistModel.artist = decodeURIComponent(split[0]); |
51 | + songsAlbumArtistModel.album = decodeURIComponent(split[1]); |
52 | } |
53 | - |
54 | - // Filter by artist and album |
55 | - songsAlbumArtistModel.artist = decodeURIComponent(split[0]); |
56 | - songsAlbumArtistModel.album = decodeURIComponent(split[1]); |
57 | - |
58 | - // Add album to recent list |
59 | - Library.addRecent(songsAlbumArtistModel.album, songsAlbumArtistModel.artist, songsAlbumArtistModel.art, songsAlbumArtistModel.album, "album") |
60 | - mainView.hasRecent = true |
61 | - recentModel.filterRecent() |
62 | } |
63 | |
64 | function processFile(uri, play) { |
65 | @@ -230,20 +225,19 @@ |
66 | |
67 | if (!track) { |
68 | console.debug("Unknown file " + uri + ", skipping") |
69 | - return; |
70 | - } |
71 | - |
72 | - if (play) { |
73 | - // clear play queue |
74 | - trackQueue.model.clear() |
75 | - } |
76 | - |
77 | - // enqueue |
78 | - trackQueue.append(makeDict(track)); |
79 | - |
80 | - // play first URI |
81 | - if (play) { |
82 | - trackQueueClick(trackQueue.model.count - 1); |
83 | + } else { |
84 | + if (play) { |
85 | + // clear play queue |
86 | + trackQueue.model.clear() |
87 | + } |
88 | + |
89 | + // enqueue |
90 | + trackQueue.append(makeDict(track)); |
91 | + |
92 | + // play first URI |
93 | + if (play) { |
94 | + trackQueueClick(trackQueue.model.count - 1); |
95 | + } |
96 | } |
97 | } |
98 | |
99 | @@ -257,7 +251,6 @@ |
100 | else if (uri.indexOf("music://") === 0) { |
101 | uriHandler.processFile(uri.substring(8), play); |
102 | } |
103 | - |
104 | else { |
105 | console.debug("Unsupported URI " + uri + ", skipping") |
106 | } |
107 | @@ -735,6 +728,11 @@ |
108 | // Play album it tracks exist |
109 | if (rowCount > 0 && selectedAlbum) { |
110 | trackClicked(songsAlbumArtistModel, 0, true, true); |
111 | + |
112 | + // Add album to recent list |
113 | + Library.addRecent(songsAlbumArtistModel.album, songsAlbumArtistModel.artist, null, songsAlbumArtistModel.album, "album") |
114 | + mainView.hasRecent = true |
115 | + recentModel.filterRecent() |
116 | } else if (selectedAlbum) { |
117 | console.debug("Unknown artist-album " + artist + "/" + album + ", skipping") |
118 | } |
119 | |
120 | === modified file 'tests/autopilot/music_app/__init__.py' |
121 | --- tests/autopilot/music_app/__init__.py 2014-09-05 22:20:29 +0000 |
122 | +++ tests/autopilot/music_app/__init__.py 2014-09-25 16:56:36 +0000 |
123 | @@ -6,6 +6,7 @@ |
124 | # by the Free Software Foundation. |
125 | |
126 | """music-app tests and emulators - top level package.""" |
127 | +from music_app.url_dispatcher import URLDispatcher |
128 | from ubuntuuitoolkit import MainView, UbuntuUIToolkitCustomProxyObjectBase |
129 | |
130 | |
131 | @@ -32,7 +33,7 @@ |
132 | return func_wrapper |
133 | |
134 | |
135 | -class MusicApp(object): |
136 | +class MusicApp(): |
137 | """Autopilot helper object for the Music application.""" |
138 | |
139 | def __init__(self, app_proxy): |
140 | @@ -40,6 +41,10 @@ |
141 | self.main_view = self.app.wait_select_single(MainView) |
142 | self.player = self.app.select_single(Player, objectName='player') |
143 | |
144 | + def call_url_dispatcher(self, protocol, path): |
145 | + dispatcher = URLDispatcher() |
146 | + dispatcher.call_dispatcher(protocol, path) |
147 | + |
148 | def get_add_to_playlist_page(self): |
149 | return self.app.wait_select_single(Page11, |
150 | objectName="addToPlaylistPage") |
151 | |
152 | === modified file 'tests/autopilot/music_app/tests/__init__.py' |
153 | --- tests/autopilot/music_app/tests/__init__.py 2014-09-25 00:39:47 +0000 |
154 | +++ tests/autopilot/music_app/tests/__init__.py 2014-09-25 16:56:36 +0000 |
155 | @@ -62,10 +62,10 @@ |
156 | |
157 | def setUp(self): |
158 | super(BaseTestCaseWithPatchedHome, self).setUp() |
159 | - _, test_type = self.get_launcher_method_and_type() |
160 | - self.home_dir = self._patch_home(test_type) |
161 | + launcher_method, self.test_type = self.get_launcher_method_and_type() |
162 | + self.home_dir = self._patch_home(self.test_type) |
163 | |
164 | - self._create_music_library(test_type) |
165 | + self._create_music_library(self.test_type) |
166 | |
167 | @autopilot_logging.log_action(logger.info) |
168 | def launch_test_local(self): |
169 | |
170 | === modified file 'tests/autopilot/music_app/tests/test_music.py' |
171 | --- tests/autopilot/music_app/tests/test_music.py 2014-09-23 20:45:41 +0000 |
172 | +++ tests/autopilot/music_app/tests/test_music.py 2014-09-25 16:56:36 +0000 |
173 | @@ -11,6 +11,7 @@ |
174 | |
175 | import logging |
176 | from autopilot.matchers import Eventually |
177 | +import os |
178 | from testtools.matchers import Equals, GreaterThan, LessThan, NotEquals |
179 | |
180 | |
181 | @@ -47,6 +48,9 @@ |
182 | } |
183 | ] |
184 | |
185 | + def is_click(self): |
186 | + return self.test_type == "click" |
187 | + |
188 | @property |
189 | def player(self): |
190 | return self.app.player |
191 | @@ -627,6 +631,62 @@ |
192 | Eventually(Equals(self.tracks[-1]["title"]))) |
193 | self.assertThat(self.player.isPlaying, Eventually(Equals(True))) |
194 | |
195 | + def test_launch_album_from_url_dispatcher_must_play_it(self): |
196 | + """Test that the album:// url plays the correct album/artist""" |
197 | + |
198 | + # Url dispatcher only works in a click-based env for now |
199 | + if not self.is_click(): |
200 | + self.skipTest("URL dispatcher works only for installed click " |
201 | + "packages.") |
202 | + |
203 | + # create path for album:// |
204 | + album_artist = os.path.join(os.path.sep, self.tracks[1]["artist"], |
205 | + self.tracks[1]["album"]) |
206 | + |
207 | + # Run url-dispatcher call |
208 | + self.app.call_url_dispatcher("album://", album_artist) |
209 | + |
210 | + self.player.isPlaying.wait_for(True) # ensure track is playing |
211 | + |
212 | + # Check that the album and artist are correct |
213 | + self.assertThat(self.player.currentMetaArtist, |
214 | + Eventually(Equals(self.tracks[1]["artist"]))) |
215 | + self.assertThat(self.player.currentMetaAlbum, |
216 | + Eventually(Equals(self.tracks[1]["album"]))) |
217 | + |
218 | + # Check now playing page is shown |
219 | + now_playing_page = self.app.get_now_playing_page() |
220 | + now_playing_page.visible.wait_for(True) |
221 | + |
222 | + # Check that the play queue count is correct |
223 | + self.assertThat(now_playing_page.get_count(), Eventually(Equals(1))) |
224 | + |
225 | + def test_launch_music_from_url_dispatcher_must_play_it(self): |
226 | + """Test that the music:// url plays the correct file""" |
227 | + |
228 | + # Url dispatcher only works in a click-based env for now |
229 | + if not self.is_click(): |
230 | + self.skipTest("URL dispatcher works only for installed click " |
231 | + "packages.") |
232 | + |
233 | + path = os.path.join(self.home_dir, "Music", self.tracks[0]["source"]) |
234 | + |
235 | + # Run url-dispatcher call |
236 | + self.app.call_url_dispatcher("music://", path) |
237 | + |
238 | + self.player.isPlaying.wait_for(True) # ensure track is playing |
239 | + |
240 | + # Check that the source of the file is correct |
241 | + self.assertThat(self.player.source, |
242 | + Eventually(Equals("file://" + path))) |
243 | + |
244 | + # Check now playing page is shown |
245 | + now_playing_page = self.app.get_now_playing_page() |
246 | + now_playing_page.visible.wait_for(True) |
247 | + |
248 | + # Check that the play queue count is correct |
249 | + self.assertThat(now_playing_page.get_count(), Eventually(Equals(1))) |
250 | + |
251 | def test_pressing_prev_after_5_seconds(self): |
252 | """Pressing previous after 5s jumps to the start of current song""" |
253 | |
254 | |
255 | === added file 'tests/autopilot/music_app/url_dispatcher.py' |
256 | --- tests/autopilot/music_app/url_dispatcher.py 1970-01-01 00:00:00 +0000 |
257 | +++ tests/autopilot/music_app/url_dispatcher.py 2014-09-25 16:56:36 +0000 |
258 | @@ -0,0 +1,22 @@ |
259 | +# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*- |
260 | +# Copyright 2014 Canonical |
261 | +# |
262 | +# This program is free software: you can redistribute it and/or modify it |
263 | +# under the terms of the GNU General Public License version 3, as published |
264 | +# by the Free Software Foundation. |
265 | + |
266 | +import autopilot.logging as autopilot_logging |
267 | +import logging |
268 | +import subprocess |
269 | + |
270 | +logger = logging.getLogger(__name__) |
271 | + |
272 | + |
273 | +class URLDispatcher(object): |
274 | + |
275 | + """Helper for the URLDispatcher""" |
276 | + |
277 | + @autopilot_logging.log_action(logger.info) |
278 | + def call_dispatcher(self, protocol, path): |
279 | + # Run call to url-dispatcher |
280 | + return subprocess.check_call(["url-dispatcher", protocol + path]) |
FAILED: Continuous integration, rev:619 91.189. 93.70:8080/ job/music- app-ci/ 1111/ 91.189. 93.70:8080/ job/generic- mediumtests- utopic- python3/ 257/console 91.189. 93.70:8080/ job/music- app-utopic- amd64-ci/ 335/console
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild: 91.189. 93.70:8080/ job/music- app-ci/ 1111/rebuild
http://