Merge lp:~ahayzen/music-app/add-url-dispatcher-tests into lp:music-app/trusty

Proposed by Andrew Hayzen
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
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-dispatcher-tests
$ cd /tmp/add-dispatcher-tests
$ click-buddy --dir . --provision

To just run the changed tests:
$ ADT_AUTOPILOT_MODULE="-v music_app.tests.test_music.TestMainWindow.test_url_dispatcher_album_play" adt-run /tmp/add-dispatcher-tests --click com.ubuntu.music --- ssh -s adb
$ ADT_AUTOPILOT_MODULE="-v music_app.tests.test_music.TestMainWindow.test_url_dispatcher_music_play" adt-run /tmp/add-dispatcher-tests --click com.ubuntu.music --- ssh -s adb

To run all tests:
$ adt-run /tmp/add-dispatcher-tests --click com.ubuntu.music --- ssh -s adb

Note you need a newer adt if you are on trusty see instructions for install of autopkgtest here [0]

0 - http://www.theorangenotebook.com/2014/09/autopilot-test-runners.html

To post a comment you must log in.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
620. By Andrew Hayzen

* Move dispatcher to helper

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
621. By Andrew Hayzen

* Merge of trunk

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
622. By Andrew Hayzen

* Fix for use of old artistName

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
623. By Andrew Hayzen

* Fix for unicode errors
* Jenkins debug

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
624. By Andrew Hayzen

* Ensure url-dispatcher has started before calling it

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
625. By Andrew Hayzen

* Pep8 fix

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
626. By Andrew Hayzen

* Only run tests when in click environment

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Leo Arias (elopio) wrote :

Some inline comments.

Revision history for this message
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

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
629. By Andrew Hayzen

* Fixes for pep8

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
630. By Andrew Hayzen

* Remove code comments

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Leo Arias (elopio) wrote :

Some pita comments inline. This is pretty good, thanks Andrew.

631. By Andrew Hayzen

* Various fixes

Revision history for this message
Andrew Hayzen (ahayzen) wrote :

I would like to land this after this [0] has landed so blocking myself for a bit.

0 - https://code.launchpad.net/~andrew-hayzen/music-app/ap-mocking-fixes/+merge/235821

review: Needs Fixing
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
632. By Andrew Hayzen

* Merge of trunk

Revision history for this message
Andrew Hayzen (ahayzen) wrote :

Unblocking myself as the other mp has landed.

review: Abstain
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Leo Arias (elopio) wrote :

ahayzen_: as you added the decorator to the url dispatcher call, you don't need
235 + logger.debug("Calling URL Dispatcher - " + path)
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.

review: Approve (code review of the autopilot code)
633. By Andrew Hayzen

* Removal of extra debugging
* Rename UrlDispatcher to URLDispatcher

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

What's up with this mp?

Revision history for this message
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.

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

I'd vote to go with the second; heads down on remix. Thanks.

Revision history for this message
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

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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])

Subscribers

People subscribed via source and target branches

to status/vote changes: