ubuntu-touch-coreapps-reviewers team mailing list archive

[Nicholas Skaggs <nskaggs@xxxxxxxxxx>]

In addition to the comments below, does this pass pep8?

Diff comments:

> 
> === modified file 'tests/autopilot/music_app/__init__.py'
> --- tests/autopilot/music_app/__init__.py	2015-11-03 03:55:07 +0000
> +++ tests/autopilot/music_app/__init__.py	2015-12-17 03:22:43 +0000
> @@ -135,6 +135,15 @@
>          # wait for now playing page to be visible
>          self.get_now_playing_page().visible.wait_for(True)
>  
> +    def get_LibraryEmptyState(self):
> +        libraryEmpty = self.app.wait_select_single(LibraryEmptyState, objectName="emptyLibrary")

Small nitpick, but you can simply 
return self.app.wait_select_single(LibraryEmptyState, objectName="emptyLibrary")

No need for a variable.

> +        return libraryEmpty
> +        
> +class LibraryEmptyState(UbuntuUIToolkitCustomProxyObjectBase):
> +    """Autopilot helper for LibraryEmptyState"""
> +    def __init__(self, *args):
> +        super(LibraryEmptyState, self).__init__(*args)
> +        
>  
>  class Page(UbuntuUIToolkitCustomProxyObjectBase):
>      """Autopilot helper for Pages."""
> 
> === added file 'tests/autopilot/music_app/content/mediascanner-2.0/.help'

Weird.. Not needed file. Remove it.

> === added file 'tests/autopilot/music_app/content/mediascanner-2.0/blank_mediastore.sql.THIS'

Weird.. Not needed file. Remove it.

> 
> === added file 'tests/autopilot/music_app/content/mediascanner-2.0/blank_mediastore.sql~'

This file isn't necessary. Remove it.

> 
> === added directory 'tests/autopilot/music_app/content/mediascanner-2.0/songs'
> === renamed file 'tests/autopilot/music_app/content/1.ogg' => 'tests/autopilot/music_app/content/mediascanner-2.0/songs/1.ogg'
> === renamed file 'tests/autopilot/music_app/content/2.ogg' => 'tests/autopilot/music_app/content/mediascanner-2.0/songs/2.ogg'
> === renamed file 'tests/autopilot/music_app/content/3.mp3' => 'tests/autopilot/music_app/content/mediascanner-2.0/songs/3.mp3'
> === modified file 'tests/autopilot/music_app/tests/__init__.py'
> --- tests/autopilot/music_app/tests/__init__.py	2015-02-03 14:42:34 +0000
> +++ tests/autopilot/music_app/tests/__init__.py	2015-12-17 03:22:43 +0000
> @@ -141,29 +134,31 @@
>              logger.debug("Patched home to fake home directory %s" % temp_dir)
>          return temp_dir
>  
> -    def _create_music_library(self):
> +    def _create_music_library(self, db_dir):
>          logger.debug("Creating music library for %s test" % self.test_type)
>          logger.debug("Home set to %s" % self.home_dir)
>          musicpath = os.path.join(self.home_dir, 'Music')
>          logger.debug("Music path set to %s" % musicpath)
>          mediascannerpath = os.path.join(self.home_dir,
>                                          '.cache/mediascanner-2.0')
> +        logger.debug("Mediascanner path set to %s" % mediascannerpath)
> +
>          if not os.path.exists(musicpath):
>              os.makedirs(musicpath)
> -        logger.debug("Mediascanner path set to %s" % mediascannerpath)
>  
>          # set content path
>          content_dir = os.path.join(os.path.dirname(music_app.__file__),
> -                                   'content')
> -
> +                                                 'content', db_dir)
> +        songs_dir = os.path.join(content_dir, 'songs')
>          logger.debug("Content dir set to %s" % content_dir)
>  
>          # copy content
> -        shutil.copy(os.path.join(content_dir, '1.ogg'), musicpath)
> -        shutil.copy(os.path.join(content_dir, '2.ogg'), musicpath)
> -        shutil.copy(os.path.join(content_dir, '3.mp3'), musicpath)
> +        if os.path.isdir(songs_dir):

I'm concerned here that we might need to remove these three specific files (for the empty library test) or the tests will fail on the phone. Removing specific files should be safe enough to not remove Andrew's Music library :p

> +            shutil.copy(os.path.join(songs_dir, '1.ogg'), musicpath)
> +            shutil.copy(os.path.join(songs_dir, '2.ogg'), musicpath)
> +            shutil.copy(os.path.join(songs_dir, '3.mp3'), musicpath)
>  
> -        logger.debug("Music copied, files " + str(os.listdir(musicpath)))
> +            logger.debug("Music copied, files " + str(os.listdir(musicpath)))
>  
>          if self.test_type is not 'click':
>              self._patch_mediascanner_home(content_dir, mediascannerpath)
> @@ -220,3 +242,18 @@
>      def setUp(self):
>          super(MusicAppTestCase, self).setUp()
>          self.app = MusicApp(self.launcher())
> +
> +
> +class MusicAppTestCaseEmptyLibrary(EmptyLibraryWithPatchedHome):
> +
> +    """Test case that launches the music-app with no music: an empty library."""
> +
> +    def setUp(self):
> +        super(MusicAppTestCaseEmptyLibrary, self).setUp()
> +        self.app = MusicApp(self.launcher())
> +

Why the extra spaces?

> +
> +
> +
> +
> +


-- 
https://code.launchpad.net/~vamrocks602/music-app/test-empty-library/+merge/280794
Your team Music App Developers is subscribed to branch lp:music-app.