ubuntu-touch-coreapps-reviewers team mailing list archive

[Nicholas Skaggs <nskaggs@xxxxxxxxxx>]

Review: Needs Fixing

One additional comment on how you are removing files.

Diff comments:

> 
> === 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-18 02:13:52 +0000
> @@ -141,29 +132,41 @@
>              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)
> -
> -        logger.debug("Music copied, files " + str(os.listdir(musicpath)))
> +        if os.path.isdir(songs_dir) and db_dir == 'mediascanner-2.0':
> +            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)))
> +
> +        if os.path.isdir(songs_dir) and db_dir == 'blank-mediascanner-2.0':

I would simply look for a missing songs dir here. If the content folder has a songs dir, copy music. If it does not have a songs dir, delete music. Don't include any checks to db_dir.

That should work fine.

> +            try:
> +                os.remove(os.path.join(songs_dir, '1.ogg'), musicpath)
> +                os.remove(os.path.join(songs_dir, '2.ogg'), musicpath)
> +                os.remove(os.path.join(songs_dir, '3.mp3'), musicpath)
> +            except OSError:
> +                pass
> +
> +            logger.debug("Music deleted, files " + str(os.listdir(musicpath)))
>  
>          if self.test_type is not 'click':
>              self._patch_mediascanner_home(content_dir, mediascannerpath)


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