ubuntu-touch-coreapps-reviewers team mailing list archive

[Andrew Hayzen <ahayzen@xxxxxxxxx>]

Review: Needs Fixing

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/

Diff comments:

> === modified file 'MusicPlaylists.qml'
> --- MusicPlaylists.qml	2015-01-21 00:10:33 +0000
> +++ MusicPlaylists.qml	2015-03-20 13:25:44 +0000
> @@ -93,6 +93,7 @@
>          delegate: Card {
>              id: playlistCard
>              coverSources: Playlists.getPlaylistCovers(model.name)
> +	    objectName: "playlistCardItem" + index
>              primaryText: model.name
>              secondaryText: i18n.tr("%1 song", "%1 songs", model.count).arg(model.count)
>  
> 
> === modified file 'common/SongsPage.qml'
> --- common/SongsPage.qml	2015-01-30 01:23:39 +0000
> +++ common/SongsPage.qml	2015-03-20 13:25:44 +0000
> @@ -593,6 +593,7 @@
>          id: removePlaylistDialog
>          Dialog {
>              id: dialogRemovePlaylist
> +	    objectName: "dialogRemovePlaylist"
>              // TRANSLATORS: this is a title of a dialog with a prompt to delete a playlist
>              title: i18n.tr("Permanently delete playlist?")
>              text: "("+i18n.tr("This cannot be undone")+")"
> @@ -602,6 +603,7 @@
>              Button {
>                  text: i18n.tr("Remove")
>                  color: styleMusic.dialog.confirmRemoveButtonColor
> +		objectName: "removePlaylistDialogRemoveButton"
>                  onClicked: {
>                      // removing playlist
>                      Playlists.removePlaylist(dialogRemovePlaylist.oldPlaylistName)
> 
> === modified file 'tests/autopilot/music_app/__init__.py'
> --- tests/autopilot/music_app/__init__.py	2015-01-19 17:13:12 +0000
> +++ tests/autopilot/music_app/__init__.py	2015-03-20 13:25:44 +0000
> @@ -75,6 +75,10 @@
>          return self.main_view.wait_select_single(
>              Dialog, objectName="dialogNewPlaylist")
>  
> +    def get_delete_playlist_dialog(self):
> +        return self.main_view.wait_select_single(
> +            Dialog, objectName="dialogRemovePlaylist")
> +
>      def get_now_playing_page(self):
>          return self.app.wait_select_single(MusicNowPlaying,
>                                             objectName="nowPlayingPage")
> @@ -208,6 +212,20 @@
>          return self.wait_select_single(
>              "CardView", objectName="playlistsCardView").count
>  
> +    def click_new_playlist_action(self):
> +            self.main_view.get_header().click_action_button("newPlaylistButton")
> +
> +    @click_object
> +    def click_playlist(self, i):
> +        return self.get_playlist(i)
> +
> +    def get_playlist(self, i):
> +        return (self.wait_select_single("Card",
> +                objectName="playlistCardItem" + str(i)))
> +
> +    def click_delete_playlist_action(self):
> +            self.main_view.get_header().click_action_button("deletePlaylist")
> +
>  
>  class MusicaddtoPlaylist(MusicPage):
>      """ Autopilot helper for add to playlist page """
> @@ -411,6 +429,11 @@
>          self.wait_select_single(
>              "TextField", objectName="playlistNameTextField").write(text)
>  
> +    @click_object
> +    def click_remove_playlist_dialog_remove_button(self):
> +        return self.wait_select_single(
> +            "Button", objectName="removePlaylistDialogRemoveButton")
> +
>  
>  class MainView(MainView):
>      """Autopilot custom proxy object for the MainView."""
> 
> === modified file 'tests/autopilot/music_app/tests/test_music.py'
> --- tests/autopilot/music_app/tests/test_music.py	2015-01-19 17:13:12 +0000
> +++ tests/autopilot/music_app/tests/test_music.py	2015-03-20 13:25:44 +0000
> @@ -81,7 +81,8 @@
>          track = self.app.get_songs_page().get_track(0)
>          track.swipe_reveal_actions()
>  
> -        track.click_add_to_queue_action()  # add track to the queue
> +        # add track to the queue
> +        track.click_add_to_queue_action()  
>  
>          # wait for track to be queued
>          self.app.get_queue_count().wait_for(initial_tracks_count + 1)
> @@ -449,7 +450,7 @@
>  
>          track.click_add_to_playlist_action()  # add track to queue
>  
> -        add_to_playlist_page = self.app.get_add_to_playlist_page()
> +        add_to_playlist_page = self.app.get_add_to_playlists_page()

This should be calling get_add_to_playlist_page() not playlists?

>  
>          # get initial list view playlist count
>          playlist_count = add_to_playlist_page.get_count()
> @@ -484,8 +485,54 @@
>  
>          # verify song has been added to playlist
>          self.assertThat(playlists_page.get_count(), Equals(1))
> -
> -    def test_artists_tab_album(self):
> +        
> +    def test_select_and_delete_playlist(self):
> +        """tests deleting a playlist by creating a playlist, 
> +            selecting it, and then deleting it. """
> +
> +        # switch to playlist page
> +        playlists_page = self.app.get_playlists_page()
> +
> +        # get initial list view playlist count
> +        playlist_count = playlists_page.get_count()
> +
> +        # click on New playlist button in header
> +        playlists_page.click_new_playlist_action()
> +
> +        # get dialog
> +        new_dialog = self.app.get_new_playlist_dialog()
> +
> +        # input playlist name
> +        new_dialog.type_new_playlist_dialog_name("testDeletePlaylist")
> +
> +        # click on the create Button
> +        new_dialog.click_new_playlist_dialog_create_button()
> +
> +        # verify playlist has been sucessfully created
> +        self.assertThat(playlists_page.get_count(),
> +                        Eventually(Equals(playlist_count + 1)))
> +
> +        self.assertThat(playlists_page.get_playlist(0).primaryText,
> +                        Equals("testDeletePlaylist"))
> +
> +        # select the playlist that was just created
> +        playlists_page.click_playlist(0)
> +
> +        # click the delete icon
> +        playlists_page.click_delete_playlist_action()
> +
> +        # get dialog
> +        delete_dialog = self.app.get_delete_playlist_dialog()
> +
> +        # click on the remove Button
> +        delete_dialog.click_remove_playlist_dialog_remove_button()
> +
> +        playlists_page = self.app.get_playlists_page()
> +
> +        # verify that the playlist has been removed
> +        self.assertThat(playlists_page.get_count(), Equals(playlist_count))
> +
> + def test_artists_tab_album(self):
>          """tests navigating to the Artists tab and playing an album"""
>  
>          # get number of tracks in queue before queuing a track
> 


-- 
https://code.launchpad.net/~darran-kelinske/music-app/lp_bug_1428454/+merge/253566
Your team Music App Developers is subscribed to branch lp:music-app.