← Back to team overview

ubuntu-touch-coreapps-reviewers team mailing list archive

Re: [Merge] lp:~fazerlicourice/music-app/test-empty-library into lp:music-app

 

Review: Needs Fixing

OK this is erroring still due to 
[Errno 2] No such file or directory: '/tmp/adt-run.XH20gO/tree/tests/autopilot/music_app/content/blank-mediascanner-2.0/songs/1.ogg'
[Errno 2] No such file or directory: '/tmp/adt-run.XH20gO/tree/tests/autopilot/music_app/content/blank-mediascanner-2.0/songs/2.ogg'
[Errno 2] No such file or directory: '/tmp/adt-run.XH20gO/tree/tests/autopilot/music_app/content/blank-mediascanner-2.0/songs/3.mp3'

However it was silently failing, so we didn't know it was unable to remove the files, full log with my extra debugging can be found here [0]


There are two inline comments (3 instances of each) to fix, which should resolve this
1) This should be 
os.remove(os.path.join(musicpath, '1.ogg'))
2) As these were failing silently, can we change these to 
except OSError as e:
    logger.debug("Error removing" + str(e))


Once you have fixed those issues, the test passes for me :-) My diff looks like this [1]

0 - http://pastebin.ubuntu.com/14404744/
1 - http://pastebin.ubuntu.com/14404783/

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	2016-01-04 21:04:17 +0000
> @@ -141,29 +131,47 @@
>              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)))
> +        # delete content if previously copied
> +        elif not os.path.isdir(songs_dir):
> +            try:
> +                os.remove(os.path.join(songs_dir, '1.ogg'))

This should be 
os.remove(os.path.join(musicpath, '1.ogg'))

> +            except OSError:

As these were failing silently, can we change these to 
except OSError as e:
    logger.debug("Error removing" + str(e))

> +                pass
> +            try:
> +                os.remove(os.path.join(songs_dir, '2.ogg'))

This should be 
os.remove(os.path.join(musicpath, '2.ogg'))

> +            except OSError:

As these were failing silently, can we change these to 
except OSError as e:
    logger.debug("Error removing" + str(e))

> +                pass
> +            try:
> +                os.remove(os.path.join(songs_dir, '3.mp3'))

This should be 
os.remove(os.path.join(musicpath, '3.mp3'))

> +            except OSError:

As these were failing silently, can we change these to 
except OSError as e:
    logger.debug("Error removing" + str(e))

> +                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/~fazerlicourice/music-app/test-empty-library/+merge/280794
Your team Music App Developers is subscribed to branch lp:music-app.


Follow ups

References