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

Proposed by Nicholas Skaggs
Status: Merged
Approved by: Nicholas Skaggs
Approved revision: 386
Merged at revision: 441
Proposed branch: lp:~nskaggs/music-app/fix-1292044
Merge into: lp:music-app/trusty
Diff against target: 191 lines (+25/-81)
1 file modified
tests/autopilot/music_app/tests/__init__.py (+25/-81)
To merge this branch: bzr merge lp:~nskaggs/music-app/fix-1292044
Reviewer Review Type Date Requested Status
Victor Thompson Approve
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Review via email: mp+211637@code.launchpad.net

Commit message

Fix phablet device test runs not using mocked home

Description of the change

Fix phablet device test runs not using mocked home

To post a comment you must log in.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Victor Thompson (vthompson) wrote :

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
Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

Yes, sorry, this is still WIP :-)

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

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.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Victor Thompson (vthompson) wrote :

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

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

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?

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

Setting to WIP until then.

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

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

lp:~nskaggs/music-app/fix-1292044 updated
383. By Nicholas Skaggs

change str replace func

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
lp:~nskaggs/music-app/fix-1292044 updated
384. By Nicholas Skaggs

more tweaks with str replace ;-)

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

Python3, 1; me, 0.

I <3 UnicodeDecodeError.

lp:~nskaggs/music-app/fix-1292044 updated
385. By Nicholas Skaggs

files are binary, d'oh

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

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.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Victor Thompson (vthompson) wrote :

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
Revision history for this message
Victor Thompson (vthompson) wrote :

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.

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

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

Revision history for this message
Victor Thompson (vthompson) wrote :

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

Revision history for this message
Victor Thompson (vthompson) wrote :

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.

Revision history for this message
Victor Thompson (vthompson) wrote :

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.

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

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 updated
386. By Nicholas Skaggs

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

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Victor Thompson (vthompson) wrote :

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

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
8"""Music app autopilot tests."""8"""Music app autopilot tests."""
99
10import tempfile10import tempfile
11
12try:11try:
13 from unittest import mock12 from unittest import mock
14except ImportError:13except ImportError:
@@ -28,7 +27,8 @@
2827
29from ubuntuuitoolkit import (28from ubuntuuitoolkit import (
30 base,29 base,
31 emulators as toolkit_emulators30 emulators as toolkit_emulators,
31 environment
32)32)
3333
3434
@@ -50,9 +50,6 @@
50 local_location_dir = os.path.dirname(os.path.dirname(working_dir))50 local_location_dir = os.path.dirname(os.path.dirname(working_dir))
51 local_location = local_location_dir + "/music-app.qml"51 local_location = local_location_dir + "/music-app.qml"
52 installed_location = "/usr/share/music-app/music-app.qml"52 installed_location = "/usr/share/music-app/music-app.qml"
53 sqlite_dir = os.path.expanduser(
54 "~/.local/share/com.ubuntu.music/Databases")
55 backup_dir = sqlite_dir + ".backup"
5653
57 def setup_environment(self):54 def setup_environment(self):
58 if os.path.exists(self.local_location):55 if os.path.exists(self.local_location):
@@ -67,15 +64,8 @@
67 return launch, test_type64 return launch, test_type
6865
69 def setUp(self):66 def setUp(self):
70 #backup and wipe db's before testing
71 self.temp_move_sqlite_db()
72 self.addCleanup(self.restore_sqlite_db)
73
74 launch, self.test_type = self.setup_environment()67 launch, self.test_type = self.setup_environment()
75 if self.test_type != 'click':68 self.home_dir = self._patch_home()
76 self.home_dir = self._patch_home()
77 else:
78 self.home_dir = self._save_home()
79 self._create_music_library()69 self._create_music_library()
80 self.pointing_device = Pointer(self.input_device_class.create())70 self.pointing_device = Pointer(self.input_device_class.create())
81 super(MusicTestCase, self).setUp()71 super(MusicTestCase, self).setUp()
@@ -103,25 +93,10 @@
103 "com.ubuntu.music",93 "com.ubuntu.music",
104 emulator_base=toolkit_emulators.UbuntuUIToolkitEmulatorBase)94 emulator_base=toolkit_emulators.UbuntuUIToolkitEmulatorBase)
10595
106 def _save_home(self):
107 logger.debug('Saving HOME')
108 home_dir = os.environ['HOME']
109 backup_list = ('Music', ) # '.cache/mediascanner')
110 backup_path = [os.path.join(home_dir, i) for i in backup_list]
111 backups = [(i, '%s.bak' % i) for i in backup_path if os.path.exists(i)]
112 for b in backups:
113 logger.debug('backing up %s to %s' % b)
114 try:
115 shutil.rmtree(b[1])
116 except:
117 pass
118 shutil.move(b[0], b[1])
119 #self.addCleanup(shutil.move(b[1], b[0]))
120 return home_dir
121
122 def _patch_home(self):96 def _patch_home(self):
123 #make a temp dir97 #make a temp dir
124 temp_dir = tempfile.mkdtemp()98 temp_dir = tempfile.mkdtemp()
99
125 #delete it, and recreate it to the length100 #delete it, and recreate it to the length
126 #required so our patching the db works101 #required so our patching the db works
127 #require a length of 25102 #require a length of 25
@@ -130,6 +105,7 @@
130 os.mkdir(temp_dir)105 os.mkdir(temp_dir)
131 logger.debug("Created fake home directory " + temp_dir)106 logger.debug("Created fake home directory " + temp_dir)
132 self.addCleanup(shutil.rmtree, temp_dir)107 self.addCleanup(shutil.rmtree, temp_dir)
108
133 #if the Xauthority file is in home directory109 #if the Xauthority file is in home directory
134 #make sure we copy it to temp home, otherwise do nothing110 #make sure we copy it to temp home, otherwise do nothing
135 xauth = os.path.expanduser(os.path.join('~', '.Xauthority'))111 xauth = os.path.expanduser(os.path.join('~', '.Xauthority'))
@@ -138,10 +114,17 @@
138 shutil.copyfile(114 shutil.copyfile(
139 os.path.expanduser(os.path.join('~', '.Xauthority')),115 os.path.expanduser(os.path.join('~', '.Xauthority')),
140 os.path.join(temp_dir, '.Xauthority'))116 os.path.join(temp_dir, '.Xauthority'))
141 patcher = mock.patch.dict('os.environ', {'HOME': temp_dir})117
142 patcher.start()118 #click can use initctl env (upstart), but desktop still requires mock
119 if self.test_type == 'click':
120 environment.set_initctl_env_var('HOME', temp_dir)
121 self.addCleanup(environment.unset_initctl_env_var, 'HOME')
122 else:
123 patcher = mock.patch.dict('os.environ', {'HOME': temp_dir})
124 patcher.start()
125 self.addCleanup(patcher.stop)
126
143 logger.debug("Patched home to fake home directory " + temp_dir)127 logger.debug("Patched home to fake home directory " + temp_dir)
144 self.addCleanup(patcher.stop)
145 return temp_dir128 return temp_dir
146129
147 def _create_music_library(self):130 def _create_music_library(self):
@@ -159,29 +142,19 @@
159142
160 logger.debug("Content dir set to %s" % content_dir)143 logger.debug("Content dir set to %s" % content_dir)
161144
162 #stop media scanner
163 #if self.test_type == 'click':
164 # subprocess.check_call(['stop', 'mediascanner'])
165
166 #copy content145 #copy content
167 shutil.copy(os.path.join(content_dir, '1.ogg'), musicpath)146 shutil.copy(os.path.join(content_dir, '1.ogg'), musicpath)
168 shutil.copy(os.path.join(content_dir, '2.ogg'), musicpath)147 shutil.copy(os.path.join(content_dir, '2.ogg'), musicpath)
169 shutil.copy(os.path.join(content_dir, '3.mp3'), musicpath)148 shutil.copy(os.path.join(content_dir, '3.mp3'), musicpath)
170 if self.test_type != 'click':149 shutil.copytree(
171 shutil.copytree(os.path.join(content_dir, 'mediascanner'),150 os.path.join(content_dir, 'mediascanner'), mediascannerpath)
172 mediascannerpath)
173151
174 logger.debug("Music copied, files " + str(os.listdir(musicpath)))152 logger.debug("Music copied, files " + str(os.listdir(musicpath)))
175153
176 if self.test_type != 'click':154 self._patch_mediascanner_home(mediascannerpath)
177 self._patch_mediascanner_home(mediascannerpath)155 logger.debug(
178 logger.debug(156 "Mediascanner database copied, files " +
179 "Mediascanner database copied, files " +157 str(os.listdir(mediascannerpath)))
180 str(os.listdir(mediascannerpath)))
181
182 #start media scanner
183 #if self.test_type == 'click':
184 # subprocess.check_call(['start', 'mediascanner'])
185158
186 def _patch_mediascanner_home(self, mediascannerpath):159 def _patch_mediascanner_home(self, mediascannerpath):
187 #do some inline db patching160 #do some inline db patching
@@ -204,10 +177,10 @@
204 #replace all occurences of string find with string replace177 #replace all occurences of string find with string replace
205 #in the given file178 #in the given file
206 out_filename = in_filename + ".tmp"179 out_filename = in_filename + ".tmp"
207 infile = open(in_filename, 'r')180 infile = open(in_filename, 'rb')
208 outfile = open(out_filename, 'w')181 outfile = open(out_filename, 'wb')
209 for s in infile.xreadlines():182 for line in infile:
210 outfile.write(s.replace(find, replace))183 outfile.write(line.replace(str.encode(find), str.encode(replace)))
211 infile.close()184 infile.close()
212 outfile.close()185 outfile.close()
213186
@@ -215,35 +188,6 @@
215 os.remove(in_filename)188 os.remove(in_filename)
216 os.rename(out_filename, in_filename)189 os.rename(out_filename, in_filename)
217190
218 def temp_move_sqlite_db(self):
219 try:
220 shutil.rmtree(self.backup_dir)
221 except:
222 pass
223 else:
224 logger.warning("Prexisting backup database found and removed")
225
226 try:
227 shutil.move(self.sqlite_dir, self.backup_dir)
228 except:
229 logger.warning("No current database found")
230 else:
231 logger.debug("Backed up database")
232
233 def restore_sqlite_db(self):
234 if os.path.exists(self.backup_dir):
235 if os.path.exists(self.sqlite_dir):
236 try:
237 shutil.rmtree(self.sqlite_dir)
238 except:
239 logger.error("Failed to remove test database and restore" /
240 "database")
241 return
242 try:
243 shutil.move(self.backup_dir, self.sqlite_dir)
244 except:
245 logger.error("Failed to restore database")
246
247 @property191 @property
248 def player(self):192 def player(self):
249 return self.main_view.get_player()193 return self.main_view.get_player()

Subscribers

People subscribed via source and target branches

to status/vote changes: