Merge lp:~nskaggs/music-app/fix-1292044 into lp:music-app/trusty

[Nicholas Skaggs]

Fix phablet device test runs not using mocked home

Fix phablet device test runs not using mocked home

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Continuous integration, rev:378
http://91.189.93.70:8080/job/music-app-ci/649/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-trusty/1794/console
    FAILURE: http://91.189.93.70:8080/job/music-app-raring-amd64-ci/649/console
    FAILURE: http://91.189.93.70:8080/job/music-app-saucy-amd64-ci/651/console
    FAILURE: http://91.189.93.70:8080/job/music-app-trusty-amd64-ci/370/console

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

review: Needs Fixing (continuous-integration)

[Victor Thompson (vthompson)]

I ran a click test against this change. I think there's instance(s) of calls to 'temp_move_sqlite_db()'--which you are removing.

======================================================================
ERROR: music_app.tests.test_music.TestMainWindow.test_create_playlist_from_songs_tab(with touch)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/phablet/autopilot/music_app/tests/test_music.py", line 26, in setUp
super(TestMainWindow, self).setUp()
File "/home/phablet/autopilot/music_app/tests/__init__.py", line 68, in setUp
self.temp_move_sqlite_db()
AttributeError: 'TestMainWindow' object has no attribute 'temp_move_sqlite_db'

review: Needs Fixing

[Nicholas Skaggs (nskaggs)]

Yes, sorry, this is still WIP :-)

[Nicholas Skaggs (nskaggs)]

So this simply removes not using the mocking process on devices. It seems like mediascanner doesn't take kindly to the changes on the device and although everything is lined up, music doesn't see any music when run.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Continuous integration, rev:379
http://91.189.93.70:8080/job/music-app-ci/652/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-trusty/1802/console
    FAILURE: http://91.189.93.70:8080/job/music-app-raring-amd64-ci/652/console
    FAILURE: http://91.189.93.70:8080/job/music-app-saucy-amd64-ci/654/console
    FAILURE: http://91.189.93.70:8080/job/music-app-trusty-amd64-ci/373/console

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

review: Needs Fixing (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:380
http://91.189.93.70:8080/job/music-app-ci/653/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/1804
    SUCCESS: http://91.189.93.70:8080/job/music-app-raring-amd64-ci/653
    SUCCESS: http://91.189.93.70:8080/job/music-app-saucy-amd64-ci/655
    SUCCESS: http://91.189.93.70:8080/job/music-app-trusty-amd64-ci/374

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

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

Here's a comment from Sergio about getting the click home directory mocking to work [1]:

"You can't mock the env with click as it's launched with upstart; in any case you could inject stuff in upstart with initctl set-env"

[1] https://code.launchpad.net/~vthompson/music-app/fix-1293488/+merge/211435/comments/499800

[Nicholas Skaggs (nskaggs)]

Sounds like Leo has a proper solution for us here:

https://code.launchpad.net/~elopio/ubuntu-ui-toolkit/initctl_env_var/+merge/208612

I guess we can deal with the pain till it lands and use it eh?

[Nicholas Skaggs (nskaggs)]

Setting to WIP until then.

[Nicholas Skaggs (nskaggs)]

initctl_env has been added to the sdk autopilot helper, so going to update the MP using this.

[lp:~nskaggs/music-app/fix-1292044]

383. By Nicholas Skaggs on 2014-04-22

change str replace func

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:382
http://91.189.93.70:8080/job/music-app-ci/726/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2184
    SUCCESS: http://91.189.93.70:8080/job/music-app-raring-amd64-ci/726
    SUCCESS: http://91.189.93.70:8080/job/music-app-saucy-amd64-ci/728
    SUCCESS: http://91.189.93.70:8080/job/music-app-trusty-amd64-ci/447

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

review: Approve (continuous-integration)

[lp:~nskaggs/music-app/fix-1292044]

384. By Nicholas Skaggs on 2014-04-22

more tweaks with str replace ;-)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:384
http://91.189.93.70:8080/job/music-app-ci/727/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2185
    SUCCESS: http://91.189.93.70:8080/job/music-app-raring-amd64-ci/727
    SUCCESS: http://91.189.93.70:8080/job/music-app-saucy-amd64-ci/729
    SUCCESS: http://91.189.93.70:8080/job/music-app-trusty-amd64-ci/448

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

review: Approve (continuous-integration)

[Nicholas Skaggs (nskaggs)]

Python3, 1; me, 0.

I <3 UnicodeDecodeError.

[lp:~nskaggs/music-app/fix-1292044]

385. By Nicholas Skaggs on 2014-04-22

files are binary, d'oh

[Nicholas Skaggs (nskaggs)]

This properly runs on my device now :-) Due note jenkins is still running using fake env, so the review really needs to happen on a real device so you can confirm the bug is fixed.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:385
http://91.189.93.70:8080/job/music-app-ci/728/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2186
    SUCCESS: http://91.189.93.70:8080/job/music-app-raring-amd64-ci/728
    SUCCESS: http://91.189.93.70:8080/job/music-app-saucy-amd64-ci/730
    SUCCESS: http://91.189.93.70:8080/job/music-app-trusty-amd64-ci/449

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

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

Do we need to add another dependency to the app? Or do I need a certain version of the toolkit? When I run I get the following error from phablet-test-run. Note this is on Trusty image r303:

======================================================================
ERROR: unittest.loader.ModuleImportFailure.music_app.tests.test_music
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/lib/python3/dist-packages/autopilot/run.py", line 420, in _filter_tests
test.debug()
File "/usr/lib/python3.4/unittest/case.py", line 627, in debug
getattr(self, self._testMethodName)()
File "/usr/lib/python3.4/unittest/loader.py", line 32, in testFailure
raise exception
ImportError: Failed to import test module: music_app.tests.test_music
Traceback (most recent call last):
File "/usr/lib/python3.4/unittest/loader.py", line 312, in _find_tests
module = self._get_module_from_name(name)
File "/usr/lib/python3.4/unittest/loader.py", line 290, in _get_module_from_name
__import__(name)
File "/home/phablet/autopilot/music_app/tests/__init__.py", line 28, in <module>
from ubuntuuitoolkit import (
ImportError: cannot import name 'environment'

Installed ubuntu-ui-toolkit-plugin version:

$apt-cache policy qtdeclarative5-ubuntu-ui-toolkit-plugin
qtdeclarative5-ubuntu-ui-toolkit-plugin:
  Installed: 0.1.46+14.04.20140408.1-0ubuntu1
  Candidate: 0.1.46+14.04.20140408.1-0ubuntu1
  Version table:
 *** 0.1.46+14.04.20140408.1-0ubuntu1 0
        500 http://ports.ubuntu.com/ubuntu-ports/ trusty/main armhf Packages
        100 /var/lib/dpkg/status

review: Needs Information

[Victor Thompson (vthompson)]

Tests pass on the device. However, after running the tests the next launch of the app says that no music can be found. I'll dig a bit more, but the ~/Music directory is still intact and ~/.cache/mediascanner also seems to still be in tact.

[Nicholas Skaggs (nskaggs)]

I'll have a look.. I'm assuming you are saying the tests aren't cleaning up well after running.

[Victor Thompson (vthompson)]

Correct, and IIRC everything went back to normal after a reboot.

[Victor Thompson (vthompson)]

I verified again and a reboot fixed it. The contents of the mediascanner db was identical before and after the reboot... So I don't know what the issue is. Perhaps the environment faking is becoming permanent until we reboot? If we truly can't figure out what's going on, I'm perfectly fine approving this so we only need to reboot to return to normalcy. I'll start reviewing the code changes.

[Victor Thompson (vthompson)]

I reviewed the code changes and I don't see any issues. To fix the cleanup issue, this seems to be all that is needed:

=== modified file 'tests/autopilot/music_app/tests/__init__.py'
--- tests/autopilot/music_app/tests/__init__.py 2014-04-22 19:54:51 +0000
+++ tests/autopilot/music_app/tests/__init__.py 2014-04-30 12:01:14 +0000
@@ -118,6 +118,7 @@
         #click can use initctl env (upstart), but desktop still requires mock
         if self.test_type == 'click':
             environment.set_initctl_env_var('HOME', temp_dir)
+ self.addCleanup(environment.unset_initctl_env_var, 'HOME')
         else:
             patcher = mock.patch.dict('os.environ', {'HOME': temp_dir})
             patcher.start()

Once that is in place, I think we can merge this branch.

[Nicholas Skaggs (nskaggs)]

vthompson, sorry didn't get to this, but yes that finishes it. Need to tell the env to cleanup as well, hah!

[lp:~nskaggs/music-app/fix-1292044]

386. By Nicholas Skaggs on 2014-04-30

ty Victor; adding cleanup step so env cleans up after too!

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:386
http://91.189.93.70:8080/job/music-app-ci/746/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2318
    SUCCESS: http://91.189.93.70:8080/job/music-app-raring-amd64-ci/746
    SUCCESS: http://91.189.93.70:8080/job/music-app-saucy-amd64-ci/748
    SUCCESS: http://91.189.93.70:8080/job/music-app-trusty-amd64-ci/467

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

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

Looks good! Thanks for doing this--I will no longer shudder at the thought of testing the app!

review: Approve

=== modified file 'tests/autopilot/music_app/tests/__init__.py'
--- tests/autopilot/music_app/tests/__init__.py	2014-03-07 20:10:34 +0000
+++ tests/autopilot/music_app/tests/__init__.py	2014-04-30 15:25:17 +0000
@@ -8,7 +8,6 @@
 """Music app autopilot tests."""
 
 import tempfile
-
 try:
     from unittest import mock
 except ImportError:
@@ -28,7 +27,8 @@
 
 from ubuntuuitoolkit import (
     base,
-    emulators as toolkit_emulators
+    emulators as toolkit_emulators,
+    environment
 )
 
 
@@ -50,9 +50,6 @@
     local_location_dir = os.path.dirname(os.path.dirname(working_dir))
     local_location = local_location_dir + "/music-app.qml"
     installed_location = "/usr/share/music-app/music-app.qml"
-    sqlite_dir = os.path.expanduser(
-        "~/.local/share/com.ubuntu.music/Databases")
-    backup_dir = sqlite_dir + ".backup"
 
     def setup_environment(self):
         if os.path.exists(self.local_location):
@@ -67,15 +64,8 @@
         return launch, test_type
 
     def setUp(self):
-        #backup and wipe db's before testing
-        self.temp_move_sqlite_db()
-        self.addCleanup(self.restore_sqlite_db)
-
         launch, self.test_type = self.setup_environment()
-        if self.test_type != 'click':
-            self.home_dir = self._patch_home()
-        else:
-            self.home_dir = self._save_home()
+        self.home_dir = self._patch_home()
         self._create_music_library()
         self.pointing_device = Pointer(self.input_device_class.create())
         super(MusicTestCase, self).setUp()
@@ -103,25 +93,10 @@
             "com.ubuntu.music",
             emulator_base=toolkit_emulators.UbuntuUIToolkitEmulatorBase)
 
-    def _save_home(self):
-        logger.debug('Saving HOME')
-        home_dir = os.environ['HOME']
-        backup_list = ('Music', )  # '.cache/mediascanner')
-        backup_path = [os.path.join(home_dir, i) for i in backup_list]
-        backups = [(i, '%s.bak' % i) for i in backup_path if os.path.exists(i)]
-        for b in backups:
-            logger.debug('backing up %s to %s' % b)
-            try:
-                shutil.rmtree(b[1])
-            except:
-                pass
-            shutil.move(b[0], b[1])
-            #self.addCleanup(shutil.move(b[1], b[0]))
-        return home_dir
-
     def _patch_home(self):
         #make a temp dir
         temp_dir = tempfile.mkdtemp()
+
         #delete it, and recreate it to the length
         #required so our patching the db works
         #require a length of 25
@@ -130,6 +105,7 @@
         os.mkdir(temp_dir)
         logger.debug("Created fake home directory " + temp_dir)
         self.addCleanup(shutil.rmtree, temp_dir)
+
         #if the Xauthority file is in home directory
         #make sure we copy it to temp home, otherwise do nothing
         xauth = os.path.expanduser(os.path.join('~', '.Xauthority'))
@@ -138,10 +114,17 @@
             shutil.copyfile(
                 os.path.expanduser(os.path.join('~', '.Xauthority')),
                 os.path.join(temp_dir, '.Xauthority'))
-        patcher = mock.patch.dict('os.environ', {'HOME': temp_dir})
-        patcher.start()
+
+        #click can use initctl env (upstart), but desktop still requires mock
+        if self.test_type == 'click':
+            environment.set_initctl_env_var('HOME', temp_dir)
+            self.addCleanup(environment.unset_initctl_env_var, 'HOME')
+        else:
+            patcher = mock.patch.dict('os.environ', {'HOME': temp_dir})
+            patcher.start()
+            self.addCleanup(patcher.stop)
+
         logger.debug("Patched home to fake home directory " + temp_dir)
-        self.addCleanup(patcher.stop)
         return temp_dir
 
     def _create_music_library(self):
@@ -159,29 +142,19 @@
 
         logger.debug("Content dir set to %s" % content_dir)
 
-        #stop media scanner
-        #if self.test_type == 'click':
-        #    subprocess.check_call(['stop', 'mediascanner'])
-
         #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 self.test_type != 'click':
-            shutil.copytree(os.path.join(content_dir, 'mediascanner'),
-                            mediascannerpath)
+        shutil.copytree(
+            os.path.join(content_dir, 'mediascanner'), mediascannerpath)
 
         logger.debug("Music copied, files " + str(os.listdir(musicpath)))
 
-        if self.test_type != 'click':
-            self._patch_mediascanner_home(mediascannerpath)
-            logger.debug(
-                "Mediascanner database copied, files " +
-                str(os.listdir(mediascannerpath)))
-
-        #start media scanner
-        #if self.test_type == 'click':
-        #    subprocess.check_call(['start', 'mediascanner'])
+        self._patch_mediascanner_home(mediascannerpath)
+        logger.debug(
+            "Mediascanner database copied, files " +
+            str(os.listdir(mediascannerpath)))
 
     def _patch_mediascanner_home(self, mediascannerpath):
         #do some inline db patching
@@ -204,10 +177,10 @@
         #replace all occurences of string find with string replace
         #in the given file
         out_filename = in_filename + ".tmp"
-        infile = open(in_filename, 'r')
-        outfile = open(out_filename, 'w')
-        for s in infile.xreadlines():
-            outfile.write(s.replace(find, replace))
+        infile = open(in_filename, 'rb')
+        outfile = open(out_filename, 'wb')
+        for line in infile:
+            outfile.write(line.replace(str.encode(find), str.encode(replace)))
         infile.close()
         outfile.close()
 
@@ -215,35 +188,6 @@
         os.remove(in_filename)
         os.rename(out_filename, in_filename)
 
-    def temp_move_sqlite_db(self):
-        try:
-            shutil.rmtree(self.backup_dir)
-        except:
-            pass
-        else:
-            logger.warning("Prexisting backup database found and removed")
-
-        try:
-            shutil.move(self.sqlite_dir, self.backup_dir)
-        except:
-            logger.warning("No current database found")
-        else:
-            logger.debug("Backed up database")
-
-    def restore_sqlite_db(self):
-        if os.path.exists(self.backup_dir):
-            if os.path.exists(self.sqlite_dir):
-                try:
-                    shutil.rmtree(self.sqlite_dir)
-                except:
-                    logger.error("Failed to remove test database and restore" /
-                                 "database")
-                    return
-            try:
-                shutil.move(self.backup_dir, self.sqlite_dir)
-            except:
-                logger.error("Failed to restore database")
-
     @property
     def player(self):
         return self.main_view.get_player()