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

Proposed by Nicholas Skaggs
Status: Merged
Approved by: Nicholas Skaggs
Approved revision: 286
Merged at revision: 273
Proposed branch: lp:~nskaggs/music-app/fix-shuffle-test
Merge into: lp:music-app/trusty
Diff against target: 293 lines (+156/-71)
1 file modified
tests/autopilot/music_app/tests/test_music.py (+156/-71)
To merge this branch: bzr merge lp:~nskaggs/music-app/fix-shuffle-test
Reviewer Review Type Date Requested Status
Victor Thompson Approve
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Andrew Hayzen Approve
Review via email: mp+198485@code.launchpad.net

Commit message

Simplify shuffle test and make it consistent, fix next and previous as well -- don't force order

Description of the change

Change the shuffle test to be much simpler. Test now checks to see that we cycle at least one song and that song was not the 'next' song in the queue order to prove shuffle is random.

fix next and previous as well -- don't force order or use specific song names

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
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

Shuffle is failing on the dashboard, and this should make it more consistent. I'm not sure if there is indeed more you wish to test, but the current iteration isn't useful

Revision history for this message
Andrew Hayzen (ahayzen) wrote :

Not sure how useful this is, but I ran autopilot with this branch on a Nexus 4 five times and got the following results.

First run:
Ran 8 tests in 200.609s
OK

Second run:
Traceback (most recent call last):
File "/home/phablet/autopilot/music_app/tests/test_music.py", line 146, in test_previous_and_mp3
self.assertThat(title, Eventually(Equals("TestMP3Title")))
File "/usr/lib/python2.7/dist-packages/testtools/testcase.py", line 412, in assertThat
raise MismatchError(matchee, matcher, mismatch, verbose)
MismatchError: After 10.0 seconds test failed: 'TestMP3Title' != u'Swansong'

Ran 8 tests in 214.126s
FAILED (failures=1)

Third run:
Traceback (most recent call last):
File "/home/phablet/autopilot/music_app/tests/test_music.py", line 168, in test_shuffle
self.assertThat(self.main_view.random, Eventually(Equals(True)))
File "/usr/lib/python2.7/dist-packages/testtools/testcase.py", line 412, in assertThat
raise MismatchError(matchee, matcher, mismatch, verbose)
MismatchError: After 10.0 seconds test on MainView.random failed: True != dbus.Boolean(False, variant_level=1)

Ran 8 tests in 199.545s
FAILED (failures=1)

Fourth run:
Ran 8 tests in 199.184s
OK

Fifth run:
Traceback (most recent call last):
File "/home/phablet/autopilot/music_app/tests/test_music.py", line 168, in test_shuffle
self.assertThat(self.main_view.random, Eventually(Equals(True)))
File "/usr/lib/python2.7/dist-packages/testtools/testcase.py", line 412, in assertThat
raise MismatchError(matchee, matcher, mismatch, verbose)
MismatchError: After 10.0 seconds test on MainView.random failed: True != dbus.Boolean(False, variant_level=1)

Ran 8 tests in 201.106s
FAILED (failures=1)

If you need any more information please let me know.

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

Can you explain the shuffle test changes? It looks like you're checking just to see if the song has changed to confirm that your "shuffled" variable is true. This is incorrect--hitting next when shuffle is off also changes the currently playing song. We need to confirm that the next track is NOT what would have otherwise came next. Then it looks like you keep hitting forward until you get to the original track. What does that prove?

maybe a decent strategy would be to hit forward, turn off shuffle, hit back, if the current track is the original track then shuffle hasn't been verified so we try again (currently up to 100 times).

review: Needs Fixing
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 :

Victor, thanks for commenting. I knew you would probably find an issue with the implementation which is why I asked. I must admit shuffle has been a source of confusion for me with the music app :-)

"We need to confirm that the next track is NOT what would have otherwise came next. "

^^ This is a good point. Your view of what I'm doing is correct. It's possible the original implementation would work now with the tweaks made, but I like your idea of hitting next and previous while toggling shuffle. I'll implement that and see what it looks like.

Revision history for this message
Andrew Hayzen (ahayzen) wrote :

As a note, I haven't had the latest code fail, *yet*.

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

Victor, having Andrew review. Made the changes you requested, and it seems to verify fine. I ran it enough times that it sometimes had to cycle a bit to "prove" the randomness. I think all is well.

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: Needs Fixing (continuous-integration)
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
Andrew Hayzen (ahayzen) wrote :

Looks good, ran 5 times on Nexus 4 without fail :)

'Ran 9 tests in 258.209s
OK'

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

I don't know if I agree with the next and previous changes. It looks like you attempted to harden then against failure, but at the same time it looks like you've made it almost impossible for the tests to fail. IMO it should not loop through 10 times and hit the forward/back button until it hits it's target. These are not randomized like shuffle so if it doesn't work the first try the test should fail. Or am I missing something?

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

Victor I was just looking at them again. I think I could tweak them a bit further. The trouble is I didn't like enforcing which track was the first -- it was gimicky. Instead, we should do the same as shuffle. Hit next, check song, hit previous and make sure it's the original song (and of course that the "next" song != original). Repeat once and I'd say we've confirmed it eh? The only thing is you wanted to ensure you tested an mp3 playback as well, so that might be trickier. Can you hop on IRC?

280. By Nicholas Skaggs

revamped next, previous tests and tweaked mp3 test

281. By Nicholas Skaggs

pep8 and pyflakes :-)

282. By Nicholas Skaggs

tweak comments

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 :

The test_next_previous looks good. I'm not so sure with the test_mp3 1) maybe ensure shuffle is off outside the loop, 2) it's possible to break out of the loop because of the count--so I think you need to assert that the file actually ends with .mp3 just as you assert that it is playing.

review: Needs Fixing
283. By Nicholas Skaggs

tweaks for mp3 test, endswith check fails

284. By Nicholas Skaggs

mp3 test fixed

285. By Nicholas Skaggs

fix mp3 test and add debug line

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
286. By Nicholas Skaggs

make queue size dynamic (read size)

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 :

Did a few runs on the device. LGTM.

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/test_music.py'
--- tests/autopilot/music_app/tests/test_music.py 2013-12-06 17:04:38 +0000
+++ tests/autopilot/music_app/tests/test_music.py 2013-12-12 19:26:51 +0000
@@ -9,6 +9,8 @@
99
10from __future__ import absolute_import10from __future__ import absolute_import
1111
12import time
13import logging
12from autopilot.matchers import Eventually14from autopilot.matchers import Eventually
13from testtools.matchers import Equals, Is, Not, LessThan, NotEquals15from testtools.matchers import Equals, Is, Not, LessThan, NotEquals
14from testtools.matchers import GreaterThan16from testtools.matchers import GreaterThan
@@ -16,6 +18,8 @@
1618
17from music_app.tests import MusicTestCase19from music_app.tests import MusicTestCase
1820
21logger = logging.getLogger(__name__)
22
1923
20class TestMainWindow(MusicTestCase):24class TestMainWindow(MusicTestCase):
2125
@@ -86,63 +90,117 @@
86 self.pointing_device.click_object(playbutton)90 self.pointing_device.click_object(playbutton)
87 self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))91 self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
8892
89 def test_next(self):93 def test_next_previous(self):
90 """ Test going to next track (Music Library must exist) """94 """ Test going to next track (Music Library must exist) """
9195
92 # populate queue96 # populate queue
93 first_genre_item = self.main_view.get_first_genre_item()97 first_genre_item = self.main_view.get_first_genre_item()
94 self.pointing_device.click_object(first_genre_item)98 self.pointing_device.click_object(first_genre_item)
9599
96 forwardbutton = self.main_view.get_forward_button()100 playbutton = self.main_view.get_now_playing_play_button()
101 shufflebutton = self.main_view.get_shuffle_button()
97102
98 title = lambda: self.main_view.currentTracktitle103 title = lambda: self.main_view.currentTracktitle
99 artist = lambda: self.main_view.currentArtist104 artist = lambda: self.main_view.currentArtist
100 self.assertThat(title,105
101 Eventually(Equals("Foss Yeaaaah! (Radio Edit)")))106 orgTitle = self.main_view.currentTracktitle
102 self.assertThat(artist, Eventually(Equals("Benjamin Kerensa")))107 orgArtist = self.main_view.currentArtist
103108
104 """ Track is playing"""109 #check original track
105 self.assertThat(self.main_view.isPlaying, Equals(True))110 self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
111 logger.debug("Original Song %s, %s" % (orgTitle, orgArtist))
112
113 """ Pause track """
114 self.pointing_device.click_object(playbutton)
115 self.assertThat(self.main_view.isPlaying, Eventually(Equals(False)))
116
117 #ensure shuffle is off
118 if self.main_view.random:
119 logger.debug("Turning off shuffle")
120 self.pointing_device.click_object(shufflebutton)
121 else:
122 logger.debug("Shuffle already off")
123 self.assertThat(self.main_view.random, Eventually(Equals(False)))
124
125 """ Select next """
126 #goal is to go back and forth and ensure 2 different songs
127 forwardbutton = self.main_view.get_forward_button()
106 self.pointing_device.click_object(forwardbutton)128 self.pointing_device.click_object(forwardbutton)
107
108 """ Track is playing"""
109 self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))129 self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
110 self.assertThat(title, Eventually(Equals("Swansong")))130
111 self.assertThat(artist, Eventually(Equals("Josh Woodward")))131 #ensure different song
112132 self.assertThat(title, Eventually(NotEquals(orgTitle)))
113 def test_previous_and_mp3(self):133 self.assertThat(artist, Eventually(NotEquals(orgArtist)))
114 """ Test going to previous track, last item must be an MP3134 nextTitle = self.main_view.currentTracktitle
115 (Music Library must exist) """135 nextArtist = self.main_view.currentArtist
116136 logger.debug("Next Song %s, %s" % (nextTitle, nextArtist))
117 # populate queue
118 first_genre_item = self.main_view.get_first_genre_item()
119 self.pointing_device.click_object(first_genre_item)
120
121 playbutton = self.main_view.get_now_playing_play_button()
122137
123 """ Pause track """138 """ Pause track """
124 self.pointing_device.click_object(playbutton)139 self.pointing_device.click_object(playbutton)
125 self.assertThat(self.main_view.isPlaying, Eventually(Equals(False)))140 self.assertThat(self.main_view.isPlaying, Eventually(Equals(False)))
126141
127 """ Repeat is off """
128 repeatbutton = self.main_view.get_repeat_button()
129 self.pointing_device.click_object(repeatbutton)
130
131 previousbutton = self.main_view.get_previous_button()
132
133 title = lambda: self.main_view.currentTracktitle
134 artist = lambda: self.main_view.currentArtist
135 self.assertThat(title,
136 Eventually(Equals("Foss Yeaaaah! (Radio Edit)")))
137 self.assertThat(artist, Eventually(Equals("Benjamin Kerensa")))
138
139 """ Select previous """142 """ Select previous """
143 previousbutton = self.main_view.get_previous_button()
140 self.pointing_device.click_object(previousbutton)144 self.pointing_device.click_object(previousbutton)
145 self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
146
147 #ensure we're back to original song
148 self.assertThat(title, Eventually(Equals(orgTitle)))
149 self.assertThat(artist, Eventually(Equals(orgArtist)))
150
151 def test_mp3(self):
152 """ Test that mp3 "plays" or at least doesn't crash on load """
153
154 # populate queue
155 first_genre_item = self.main_view.get_first_genre_item()
156 self.pointing_device.click_object(first_genre_item)
157
158 playbutton = self.main_view.get_now_playing_play_button()
159 shufflebutton = self.main_view.get_shuffle_button()
160
161 title = self.main_view.currentTracktitle
162 artist = self.main_view.currentArtist
163
164 #ensure track is playing
165 self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
166
167 #ensure shuffle is off
168 if self.main_view.random:
169 logger.debug("Turning off shuffle")
170 self.pointing_device.click_object(shufflebutton)
171 else:
172 logger.debug("Shuffle already off")
173 self.assertThat(self.main_view.random, Eventually(Equals(False)))
141174
142 """ Track is playing """175 """ Track is playing """
176 count = 1
177 #ensure track appears before looping through queue more than once
178 #needs to contain test mp3 metadata and end in *.mp3
179 queue = self.main_view.get_queue_track_count()
180 while title != "TestMP3Title" and artist != "TestMP3Artist":
181 self.assertThat(count, LessThan(queue))
182
183 """ Pause track """
184 self.pointing_device.click_object(playbutton)
185 self.assertThat(self.main_view.isPlaying,
186 Eventually(Equals(False)))
187
188 """ Select next """
189 forwardbutton = self.main_view.get_forward_button()
190 self.pointing_device.click_object(forwardbutton)
191 self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
192
193 title = self.main_view.currentTracktitle
194 artist = self.main_view.currentArtist
195 logger.debug("Current Song %s, %s" % (title, artist))
196 logger.debug("File found %s" % self.main_view.currentFile)
197
198 count = count + 1
199
200 #make sure mp3 plays
201 self.assertThat(self.main_view.currentFile.endswith("mp3"),
202 Equals(True))
143 self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))203 self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
144 self.assertThat(title, Eventually(Equals("TestMP3Title")))
145 self.assertThat(artist, Eventually(Equals("TestMP3Artist")))
146204
147 def test_shuffle(self):205 def test_shuffle(self):
148 """ Test shuffle (Music Library must exist) """206 """ Test shuffle (Music Library must exist) """
@@ -151,55 +209,82 @@
151 first_genre_item = self.main_view.get_first_genre_item()209 first_genre_item = self.main_view.get_first_genre_item()
152 self.pointing_device.click_object(first_genre_item)210 self.pointing_device.click_object(first_genre_item)
153211
154 shufflebutton = self.main_view.get_shuffle_button()
155
156 forwardbutton = self.main_view.get_forward_button()
157
158 previousbutton = self.main_view.get_previous_button()
159
160 playbutton = self.main_view.get_now_playing_play_button()
161
162 title = lambda: self.main_view.currentTracktitle
163 artist = lambda: self.main_view.currentArtist
164 self.assertThat(title,
165 Eventually(Equals("Foss Yeaaaah! (Radio Edit)")))
166 self.assertThat(artist, Eventually(Equals("Benjamin Kerensa")))
167
168 """ Track is playing, shuffle is turned on"""212 """ Track is playing, shuffle is turned on"""
169 self.assertThat(self.main_view.isPlaying, Equals(True))213 shufflebutton = self.main_view.get_shuffle_button()
170 self.pointing_device.click_object(shufflebutton)214 forwardbutton = self.main_view.get_forward_button()
171 self.assertThat(self.main_view.random, Eventually(Equals(True)))215 playbutton = self.main_view.get_now_playing_play_button()
172216 previousbutton = self.main_view.get_previous_button()
173 forward = True217
218 #play for a second, then pause
219 if not self.main_view.isPlaying:
220 logger.debug("Play not selected")
221 self.pointing_device.click_object(playbutton)
222 else:
223 logger.debug("Already playing")
224
225 self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
226 time.sleep(1)
227 self.pointing_device.click_object(playbutton)
228 self.assertThat(self.main_view.isPlaying, Eventually(Equals(False)))
229
174 count = 0230 count = 0
175 while True:231 while True:
176 self.assertThat(count, LessThan(100))232 self.assertThat(count, LessThan(100))
177233
234 #goal is to hit next under shuffle mode
235 #then verify original track is not the previous track
236 #this means a true shuffle happened
237 #if it doesn't try again, up to count times
238
239 orgTitle = self.main_view.currentTracktitle
240 orgArtist = self.main_view.currentArtist
241 logger.debug("Original Song %s, %s" % (orgTitle, orgArtist))
242
178 if (not self.main_view.toolbarShown):243 if (not self.main_view.toolbarShown):
179 self.main_view.show_toolbar()244 self.main_view.show_toolbar()
180245
181 if forward:246 #ensure shuffle is on
182 self.pointing_device.click_object(forwardbutton)247 if not self.main_view.random:
248 logger.debug("Turning on shuffle")
249 self.pointing_device.click_object(shufflebutton)
183 else:250 else:
184 self.pointing_device.click_object(previousbutton)251 logger.debug("Shuffle already on")
252 self.assertThat(self.main_view.random, Eventually(Equals(True)))
185253
186 """ Track is playing"""254 self.pointing_device.click_object(forwardbutton)
187 self.assertThat(self.main_view.isPlaying,255 self.assertThat(self.main_view.isPlaying,
188 Eventually(Equals(True)))256 Eventually(Equals(True)))
189 if (self.main_view.currentTracktitle == "TestMP3Title" and257 title = self.main_view.currentTracktitle
190 self.main_view.currentArtist == "TestMP3Artist"):258 artist = self.main_view.currentArtist
259 logger.debug("Current Song %s, %s" % (title, artist))
260
261 #go back to previous and check against original
262 #play song, then pause before switching
263 time.sleep(1)
264 self.pointing_device.click_object(playbutton)
265 self.assertThat(self.main_view.isPlaying,
266 Eventually(Equals(False)))
267
268 #ensure shuffle is off
269 if self.main_view.random:
270 logger.debug("Turning off shuffle")
271 self.pointing_device.click_object(shufflebutton)
272 else:
273 logger.debug("Shuffle already off")
274 self.assertThat(self.main_view.random, Eventually(Equals(False)))
275
276 self.pointing_device.click_object(previousbutton)
277
278 title = self.main_view.currentTracktitle
279 artist = self.main_view.currentArtist
280
281 if title != orgTitle and artist != orgArtist:
282 #we shuffled properly
283 logger.debug("Yay, shuffled %s, %s" % (title, artist))
191 break284 break
192 else:285 else:
193 """ Show toolbar if hidden """286 logger.debug("Same track, no shuffle %s, %s" % (title, artist))
194 if (not self.main_view.toolbarShown):287 count += 1
195 self.main_view.show_toolbar()
196
197 """ Pause track """
198 self.pointing_device.click_object(playbutton)
199 self.assertThat(self.main_view.isPlaying,
200 Eventually(Equals(False)))
201 forward = not forward
202 count += 1
203288
204 def test_show_albums_sheet(self):289 def test_show_albums_sheet(self):
205 """tests navigating to the Albums tab and displaying the album sheet"""290 """tests navigating to the Albums tab and displaying the album sheet"""

Subscribers

People subscribed via source and target branches

to status/vote changes: