Imagecompare: improve capture and compare methods

RESOLVED FIXED

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: njpark, Assigned: njpark)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

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

Updated

3 years ago
Assignee: nobody → npark
(Assignee)

Updated

3 years ago
Summary: Imagecompare: test_homescreen_change_wallpaper.py fails due to wrong frame → Imagecompare: improve capture and compare methods
(Assignee)

Comment 1

3 years ago
Should also disable tests that use a11y methods as well.
(Assignee)

Comment 2

3 years ago
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
Created attachment 8673297 [details] [review]
[gaia] npark-mozilla:1214286 > mozilla-b2g:master
(Assignee)

Updated

3 years ago
Attachment #8673297 - Flags: review?(martijn.martijn)
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
Flags: needinfo?(npark)
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.
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?
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?
(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 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.
Attachment #8673297 - Flags: review?(martijn.martijn) → review+
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.
(Assignee)

Comment 11

3 years ago
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
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(npark)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.