1184516 - [Aries] test_settings_wallpaper.py fails because we wait for 4 wallpapers to be present and only 3 are in spark builds

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

Description

[Johan Lorenzo [:jlorenzo]]

And we end up with this error.

> TEST-UNEXPECTED-ERROR | test_settings_wallpaper.py TestWallpaper.test_change_wallpaper | TimeoutException: TimeoutException: Timed out after 30.0 seconds with message: 4 wallpaper(s) not present after timeout
> 
> 
> Traceback (most recent call last):
>   File "/home/jlorenzo/git/gaia/tests/python/gaia-ui-tests/.env/lib/python2.7/site-packages/marionette_client-0.16-py2.7.egg/marionette/marionette_test.py", line 296, in run
>     testMethod()
>   File "/home/jlorenzo/git/gaia/tests/python/gaia-ui-tests/gaiatest/tests/functional/settings/test_settings_wallpaper.py", line 29, in test_change_wallpaper
>     wallpaper.tap_wallpaper_by_index(3)
>   File "/home/jlorenzo/git/gaia/tests/python/gaia-ui-tests/gaiatest/apps/wallpaper/app.py", line 19, in tap_wallpaper_by_index
>     message='%d wallpaper(s) not present after timeout' % (index + 1))
>   File "/home/jlorenzo/git/gaia/tests/python/gaia-ui-tests/.env/lib/python2.7/site-packages/marionette_driver-0.9-py2.7.egg/marionette_driver/wait.py", line 143, in until
>     cause=last_exc)
> TEST-INFO took 68467ms

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] JohanLorenzo:bug-1184516 > mozilla-b2g:master

Comment 2

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8652834 [details] [review]
[gaia] JohanLorenzo:bug-1184516 > mozilla-b2g:master

I changed the test to check that the wallpaper is actually changed in the system app. We currently use base64 to do the comparison.

No-Jun suggested to use the Python Image Library to perform the check. Here's what to do:

> self.marionette.set_context(self.marionette.CONTEXT_CHROME)
> screenshot = self.marionette.screenshot(format="binary")
> self.marionette.set_context(self.marionette.CONTEXT_CONTENT)

> # get the size of the status bar to crop off
> status_bar = self.marionette.find_element(*System._status_bar_locator)
> # get the size of the status bar, and multiply it by the device pixel ratio to get the exact size on device
> self.crop_height = int(status_bar.size['height']
>                        * self.marionette.execute_script('return
>                           window.wrappedJSObject.devicePixelRatio;'))


> im = Image.open(StringIO(screenshot))
> crop_box = (0, self.crop_height) + im.size
> new_image = im.crop(crop_box)

Comment 3

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8652834 [details] [review]
[gaia] JohanLorenzo:bug-1184516 > mozilla-b2g:master

I took a closer look at how image_compare works[1]. We have to call the ImageMagic subprocess, which has to be installed separately[2]. This sounds doable, but I'd rather do and document it in a follow up task. I'm afraid it'll take a bit longer than I expected.

Are you guys okay with the base64 version at the moment?

[1] https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/gaia_graphics_test.py#L126
[2] https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/tests/graphics/requirements.txt#L2

Comment 4

[No-Jun Park [:njpark]]

If you save the image as hash, (using perceptual hashing as described in  http://blog.iconfinder.com/detecting-duplicate-images-using-python/) then you can compare the images by comparing hashes.  This way you can avoid the complication with the imagemagick.

If you're converting base64 as the image, and apply hash to it, that should be okay too.  

also as davehunt said, if you can take the screenshot of the particular element (say, a background image), you can avoid cropping altogether as well.

Having said that, as long as the test fails when the background image fails to update/refresh, I don't see a problem with it. Even when we change to a same wallpaper image, technically it is changed. we're testing whether a wallpaper is refreshed, not whether the previous wallpaper is same as the new one, or whether the wallpaper is *displayed* properly, which is what imagecompare script was designed for.  

So i think the test is okay, r+ed.

Comment 5

[Martijn Wargers (dead)]

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #2)
> No-Jun suggested to use the Python Image Library to perform the check.
> Here's what to do:
> 
> > self.marionette.set_context(self.marionette.CONTEXT_CHROME)
> > screenshot = self.marionette.screenshot(format="binary")
> > self.marionette.set_context(self.marionette.CONTEXT_CONTENT)

What does that have to do with the Python Image Library?
I thought marionette screenshotting was done here:
http://mxr.mozilla.org/mozilla-central/source/testing/marionette/driver/marionette_driver/marionette.py#1753
http://mxr.mozilla.org/mozilla-central/source/testing/marionette/listener.js#1987
Or am I wrong?

Comment 6

[Martijn Wargers (dead)]

Comment on attachment 8652834 [details] [review]
[gaia] JohanLorenzo:bug-1184516 > mozilla-b2g:master

Johan, I don't really understand this conversion to base64.
Isn't it possible to just compare the screen_element.style.backgroundImage (or just computed style if that's not possible) between old an new?

Comment 7

[Johan Lorenzo [:jlorenzo]]

screen_element.style.backgroundImage is just a BlobUrl (like blob:app://system.gaiamobile.org/a-uuid-number). We can check that the UUID has changed, which has the same effect than the base64 at the moment.

The only thing that changes is if one day we want to compare the images themselves, you can easily get a Python image thanks to base64, but you can't do anything thanks to the BlobUrl.

Do you think we should drop the base64?

Comment 8

[Martijn Wargers (dead)]

Comment on attachment 8652834 [details] [review]
[gaia] JohanLorenzo:bug-1184516 > mozilla-b2g:master

I'm fine with your changes. This part of the "don't use internal things to check the state, because that's part of integration testing" sort of thing.

In the end, I think we'd want to do image compare of screenshots here, because that's really end-user centric.

Comment 9

[Martijn Wargers (dead)]

Btw, I think it would be beneficial to have our own js atom function to do screenshotting. self.marionette-screenshot() can only make screenshots of the whole element and most of the time we want screenshots of the screen minus the status bar.
I think we could also add an option to blank out certain areas that are susceptible to change (like time, battery, wifi settings).

Comment 10

[Martijn Wargers (dead)]

There is something called value_of_css_property, btw, that you perhaps could use to get the background-image:
http://mxr.mozilla.org/mozilla-central/source/testing/marionette/driver/marionette_driver/marionette.py#163

Comment 11

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8652834 [details] [review]
[gaia] JohanLorenzo:bug-1184516 > mozilla-b2g:master

You're absolutely right. I simplified the commit to only check the blob url. As the base 64 changes even if the same image is displayed, there's no point of using it. If one day we need to transform a blob to a base64, I leave the commir [1] for further reference.


[1] https://github.com/JohanLorenzo/gaia/commit/32f5121bdd9ad685a2b81f57a5c348702ea19693

Comment 12

[Johan Lorenzo [:jlorenzo]]

Landed in master at: https://github.com/mozilla-b2g/gaia/commit/c80e8ff25425b007181fd6e3de0500a0358fab37