1214286 - Imagecompare: improve capture and compare methods

Status: RESOLVED FIXED

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

Description

[No-Jun Park [:njpark]]

recent changes with take_screenshot() method in gaia_graphics_test.py is making this test to fail.

Comment 1

[No-Jun Park [:njpark]]

Should also disable tests that use a11y methods as well.

Comment 2

[No-Jun Park [:njpark]]

Changes:
- Better error message (checks to see whether the imagecompare generated messages other than pixel mismatch)
- Check to see whether the diff file is present before moving to mismatch folder
- Adds option to stay in the top level frame (for cases where the switch_to_displayed_app() gives the app frame, when it currently opened the top frame context. (this fixes test_homescreen_change_wallpaper.py)
- Disable dialer tests that incorrectly used a11y methods

Comment 3

[GitHub Autolander Bot]

Attachment: [gaia] npark-mozilla:1214286 > mozilla-b2g:master

Comment 4

[Martijn Wargers (dead)]

Comment on attachment 8673297 [details] [review]
[gaia] npark-mozilla:1214286 > mozilla-b2g:master

I can give you an r+ on this, if you want, but I find it strange that self.apps.displayed_app is now returning something different here than it used to be.
So it did return the system app here and now it's returning something else? What does it return now, the homescreen app?
getDisplayedApp function is here:
http://mxr.mozilla.org/gaia/source/tests/atoms/gaia_apps.js#361

Comment 5

[Martijn Wargers (dead)]

Ok, window.Service.query('getTopMostWindow'); does return the Homescreen, it seems, while the tap_change_wallpaper context menu is open, even though the Homescreen is not visible at all.
I wonder when this behavior changed.

Comment 6

[Martijn Wargers (dead)]

Oh wait, that's what you say in comment 0:
"recent changes with take_screenshot() method in gaia_graphics_test.py is making this test to fail."
What changes in take_screenshot() method caused this to fail?

Comment 7

[Martijn Wargers (dead)]

Ah, ok, it's this change: https://github.com/mozilla-b2g/gaia/commit/4cfa8ab866448687f8dab3b91cd3bb9c92b3b2e8#diff-471acbd5a0e0ee7c3bf38f3ebcbe7dc6
Which is from bug 1213301.

Why was that change necessary? Perhaps that part can be reverted without harm?

Comment 8

[Martijn Wargers (dead)]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #7)
> Ah, ok, it's this change:
> https://github.com/mozilla-b2g/gaia/commit/
> 4cfa8ab866448687f8dab3b91cd3bb9c92b3b2e8#diff-
> 471acbd5a0e0ee7c3bf38f3ebcbe7dc6
> Which is from bug 1213301.
> 
> Why was that change necessary? Perhaps that part can be reverted without
> harm?

I guess that might be not possible because the NGA music app uses iframes itself? So you have nested iframes here, I guess then we have the situation as described in bug 1115905, comment 4:
"
Only if the active frame is not a nested one, otherwise you won't be able to switch directly to it from the system app frame - you'd first need to switch to it's ancestor available from the system app and then through each of it's ancestors until you can reach it.
"

So we would need an api that would store all the iframes where we're switched in, then when we go to the system app, we could afterwards go back into it.

Comment 9

[Martijn Wargers (dead)]

Comment on attachment 8673297 [details] [review]
[gaia] npark-mozilla:1214286 > mozilla-b2g:master

(In reply to Martijn Wargers [:mwargers] (QA) from comment #8)
> I guess that might be not possible because the NGA music app uses iframes
> itself? 

No-Jun confirmed on irc that's the case.
I guess than this is the way to go then, although I don't really like it.

Comment 10

[Martijn Wargers (dead)]

Oh, indeed, Gaia UI tests and imagecompare tests should not be using a11y methods at all.
From what I understood from Johan yesterday, he want's to see it removed from the the repository and use a separate repository, which would then hook in ours.

Comment 11

[No-Jun Park [:njpark]]

I think the way they handle frames while changing wallpaper is a bit suspect, as you mentioned in Comment 5 as this is the only place I've seen where it does this.  (There are other similar cases, but switching to displayed app always picks the correct frame).  I'm considering it as a workaround at the moment.


Merged:
https://github.com/mozilla-b2g/gaia/commit/19bd6bf375248e3b3ab1f17da5210dcbfb853f15