Merge lp:~darran-kelinske/music-app/lp_bug_1428454 into lp:music-app/remix

Proposed by dazza5000 on 2015-03-19
Status: Merged
Approved by: Andrew Hayzen on 2015-03-24
Approved revision: 853
Merged at revision: 849
Proposed branch: lp:~darran-kelinske/music-app/lp_bug_1428454
Merge into: lp:music-app/remix
Diff against target: 146 lines (+75/-1)
4 files modified
MusicPlaylists.qml (+1/-0)
common/SongsPage.qml (+2/-0)
tests/autopilot/music_app/__init__.py (+24/-0)
tests/autopilot/music_app/tests/test_music.py (+48/-1)
To merge this branch: bzr merge lp:~darran-kelinske/music-app/lp_bug_1428454
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve on 2015-03-24
Andrew Hayzen 2015-03-19 Approve on 2015-03-24
Review via email: mp+253566@code.launchpad.net

Commit Message

* Added a test case for deleting a playlist

Description of the Change

Added a test case for deleting a playlist

To post a comment you must log in.
Andrew Hayzen (ahayzen) wrote :

Looks good so far :) Just a few inline comments...

L65, L75, L188 and possibly L140 - Could you remove this extra blank lines? (See PEP8 styling on this [0] - 2 lines for a class def and 1 for a method)
L102 - Is there any reason for renaming this var and method call? Could we keep it as add_to_playlist singular?

As I don't think jenkins runs for non-team members, I've manually run PEP8/pyflakes for you so you can see the errors [1]

0 - https://www.python.org/dev/peps/pep-0008/#blank-lines
1 - http://pastebin.ubuntu.com/10629706/

review: Needs Fixing
850. By Darran Kelinske <email address hidden> on 2015-03-19

fixing formatting and naming issues

851. By Darran Kelinske <email address hidden> on 2015-03-19

restore tracks variable that was removed

dazza5000 (darran-kelinske) wrote :

Thank yuo for the quick feedback! I think I got the changes. Please let me know. :)

852. By Darran Kelinske <email address hidden> on 2015-03-20

fixing more spacing issues

Andrew Hayzen (ahayzen) wrote :

This is looking closer :) Unfortunately there are still some failures...

Some of them are just because there is whitespace at the end of the line.

Also there is one inline comment.

$ pep8 tests/
tests/autopilot/music_app/__init__.py:216:80: E501 line too long (80 > 79 characters)
tests/autopilot/music_app/tests/test_music.py:85:42: W291 trailing whitespace
tests/autopilot/music_app/tests/test_music.py:488:1: W293 blank line contains whitespace
tests/autopilot/music_app/tests/test_music.py:490:53: W291 trailing whitespace
tests/autopilot/music_app/tests/test_music.py:535:2: E901 IndentationError: unindent does not match any outer indentation level
$ pyflakes tests/
tests/autopilot/music_app/tests/test_music.py:535:35: unindent does not match any outer indentation level
 def test_artists_tab_album(self):
                                  ^

I have made a diff [0] which contains fixes for the issues stated above. Then once it is passing pep8/pyflakes we can try triggering jenkins :)

0 - http://pastebin.ubuntu.com/10641729/

review: Needs Fixing
853. By Darran Kelinske <email address hidden> on 2015-03-21

additional formatting/syntax fixes :)

FAILED: Continuous integration, rev:853
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~darran-kelinske/music-app/lp_bug_1428454/+merge/253566/+edit-commit-message

http://91.189.93.70:8080/job/music-app-ci/1282/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2397
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2397/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-utopic-amd64-ci/376
    SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/134

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/music-app-ci/1282/rebuild

review: Needs Fixing (continuous-integration)
Andrew Hayzen (ahayzen) wrote :

Awesome, thanks for submitting this :) Looks good to me!

review: Approve
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'MusicPlaylists.qml'
2--- MusicPlaylists.qml 2015-01-21 00:10:33 +0000
3+++ MusicPlaylists.qml 2015-03-21 14:00:53 +0000
4@@ -93,6 +93,7 @@
5 delegate: Card {
6 id: playlistCard
7 coverSources: Playlists.getPlaylistCovers(model.name)
8+ objectName: "playlistCardItem" + index
9 primaryText: model.name
10 secondaryText: i18n.tr("%1 song", "%1 songs", model.count).arg(model.count)
11
12
13=== modified file 'common/SongsPage.qml'
14--- common/SongsPage.qml 2015-01-30 01:23:39 +0000
15+++ common/SongsPage.qml 2015-03-21 14:00:53 +0000
16@@ -593,6 +593,7 @@
17 id: removePlaylistDialog
18 Dialog {
19 id: dialogRemovePlaylist
20+ objectName: "dialogRemovePlaylist"
21 // TRANSLATORS: this is a title of a dialog with a prompt to delete a playlist
22 title: i18n.tr("Permanently delete playlist?")
23 text: "("+i18n.tr("This cannot be undone")+")"
24@@ -602,6 +603,7 @@
25 Button {
26 text: i18n.tr("Remove")
27 color: styleMusic.dialog.confirmRemoveButtonColor
28+ objectName: "removePlaylistDialogRemoveButton"
29 onClicked: {
30 // removing playlist
31 Playlists.removePlaylist(dialogRemovePlaylist.oldPlaylistName)
32
33=== modified file 'tests/autopilot/music_app/__init__.py'
34--- tests/autopilot/music_app/__init__.py 2015-01-19 17:13:12 +0000
35+++ tests/autopilot/music_app/__init__.py 2015-03-21 14:00:53 +0000
36@@ -75,6 +75,10 @@
37 return self.main_view.wait_select_single(
38 Dialog, objectName="dialogNewPlaylist")
39
40+ def get_delete_playlist_dialog(self):
41+ return self.main_view.wait_select_single(
42+ Dialog, objectName="dialogRemovePlaylist")
43+
44 def get_now_playing_page(self):
45 return self.app.wait_select_single(MusicNowPlaying,
46 objectName="nowPlayingPage")
47@@ -208,6 +212,21 @@
48 return self.wait_select_single(
49 "CardView", objectName="playlistsCardView").count
50
51+ def click_new_playlist_action(self):
52+ self.main_view.get_header(
53+ ).click_action_button("newPlaylistButton")
54+
55+ @click_object
56+ def click_playlist(self, i):
57+ return self.get_playlist(i)
58+
59+ def get_playlist(self, i):
60+ return (self.wait_select_single("Card",
61+ objectName="playlistCardItem" + str(i)))
62+
63+ def click_delete_playlist_action(self):
64+ self.main_view.get_header().click_action_button("deletePlaylist")
65+
66
67 class MusicaddtoPlaylist(MusicPage):
68 """ Autopilot helper for add to playlist page """
69@@ -411,6 +430,11 @@
70 self.wait_select_single(
71 "TextField", objectName="playlistNameTextField").write(text)
72
73+ @click_object
74+ def click_remove_playlist_dialog_remove_button(self):
75+ return self.wait_select_single(
76+ "Button", objectName="removePlaylistDialogRemoveButton")
77+
78
79 class MainView(MainView):
80 """Autopilot custom proxy object for the MainView."""
81
82=== modified file 'tests/autopilot/music_app/tests/test_music.py'
83--- tests/autopilot/music_app/tests/test_music.py 2015-01-19 17:13:12 +0000
84+++ tests/autopilot/music_app/tests/test_music.py 2015-03-21 14:00:53 +0000
85@@ -81,7 +81,8 @@
86 track = self.app.get_songs_page().get_track(0)
87 track.swipe_reveal_actions()
88
89- track.click_add_to_queue_action() # add track to the queue
90+ # add track to the queue
91+ track.click_add_to_queue_action()
92
93 # wait for track to be queued
94 self.app.get_queue_count().wait_for(initial_tracks_count + 1)
95@@ -485,6 +486,52 @@
96 # verify song has been added to playlist
97 self.assertThat(playlists_page.get_count(), Equals(1))
98
99+ def test_select_and_delete_playlist(self):
100+ """tests deleting a playlist by creating a playlist,
101+ selecting it, and then deleting it. """
102+
103+ # switch to playlist page
104+ playlists_page = self.app.get_playlists_page()
105+
106+ # get initial list view playlist count
107+ playlist_count = playlists_page.get_count()
108+
109+ # click on New playlist button in header
110+ playlists_page.click_new_playlist_action()
111+
112+ # get dialog
113+ new_dialog = self.app.get_new_playlist_dialog()
114+
115+ # input playlist name
116+ new_dialog.type_new_playlist_dialog_name("testDeletePlaylist")
117+
118+ # click on the create Button
119+ new_dialog.click_new_playlist_dialog_create_button()
120+
121+ # verify playlist has been sucessfully created
122+ self.assertThat(playlists_page.get_count(),
123+ Eventually(Equals(playlist_count + 1)))
124+
125+ self.assertThat(playlists_page.get_playlist(0).primaryText,
126+ Equals("testDeletePlaylist"))
127+
128+ # select the playlist that was just created
129+ playlists_page.click_playlist(0)
130+
131+ # click the delete icon
132+ playlists_page.click_delete_playlist_action()
133+
134+ # get dialog
135+ delete_dialog = self.app.get_delete_playlist_dialog()
136+
137+ # click on the remove Button
138+ delete_dialog.click_remove_playlist_dialog_remove_button()
139+
140+ playlists_page = self.app.get_playlists_page()
141+
142+ # verify that the playlist has been removed
143+ self.assertThat(playlists_page.get_count(), Equals(playlist_count))
144+
145 def test_artists_tab_album(self):
146 """tests navigating to the Artists tab and playing an album"""
147

Subscribers

People subscribed via source and target branches