1180819 - Add features in imagecompare to help automating RTL acceptance tests

Status: RESOLVED FIXED

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

Description

[No-Jun Park [:njpark]]

RTL QA can benefit from the imagecompare tool if certain features are added, such as locale selection.

Comment 1

[GitHub Autolander Bot]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30847

Comment 2

[No-Jun Park [:njpark]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30847

A couple of small features are added in this pull request, as well as the proof-of-concept script for RTL testing

features:
- screenshot names can contain user-defined text
- locale is selectable as gaiatest argument
- mismatches are saved in a separate folder

settings/app.py is changed so:
- the menu locators are ordered the same way the settings app displays them
- additional locators are added 
- method created to simply open the menu without returning an object (found bug 1180363 during the process)

Comment 3

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 8630114 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30847

Looks good to me! I have no doubt you'll hit some bumps and bruises putting it into practice, but this is an awesome starting point.

Comment 4

[Martijn Wargers (dead)]

Comment on attachment 8630114 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30847

It looks good (I haven't tested it locally, though). But indeed, I think it would be good to have this locale pref also as a command-line option in regular gaia UI test.

Comment 5

[No-Jun Park [:njpark]]

refactoring of menu opening methods is now pushed into the pull request

Comment 6

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8630114 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30847

This looks great! We can finish up with some loop refactors, and we'd be good to go!

Comment 7

[Martijn Wargers (dead)]

Comment on attachment 8630114 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30847

I guess the pull request has changed so much by now that I should take a look at it again once you've updated it with the latest changes.

Comment 8

[No-Jun Park [:njpark]]

Comment on attachment 8630114 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30847

So I added an example of multi-level screenshot (including system prompt) with 'more information' page, which also shows that using loop to go through all submenus might be cumberson.

re-requesting review.

Comment 9

[Martijn Wargers (dead)]

These were the failures I was seeing when doing an imagcompare run: http://people.mozilla.org/~mwargers/imgcomparescreenshots/mismatches/
Failures in application storage, Date&Time, Developer (Devtools wifi Device Name), Language (because of time), Wifi.
Perhaps it would be good if certain areas of imagecompare could be ignored?

Comment 10

[Martijn Wargers (dead)]

I've put language.current in my testvars.json file, but I couldn't get the language changed at all there.
No-Jun, what kind of setting do you use to turn the language into Arabic (rtl) for example?

Comment 11

[Martijn Wargers (dead)]

Supposedly, I could also use --locale=ar-sa in the run, but that didn't seem to work, either.

Comment 12

[Martijn Wargers (dead)]

I guess this --locale thing also needs to be documented at https://developer.mozilla.org/en-US/docs/Mozilla/QA/Marionette/Run_ImageMagick_Tool_with_Python_Marionette

Comment 13

[Ahmed Nefzaoui [:Nefzaoui]]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #11)
> Supposedly, I could also use --locale=ar-sa in the run, but that didn't seem
> to work, either.

Logically, we still don't have multi-arabic locales/variations, there's only ar for now (hope that helps) :)

Comment 14

[Martijn Wargers (dead)]

I also don't understand why this is an rtl test:  tests/python/gaia-ui-tests/gaiatest/tests/graphics/RTL/test_settings_RTL_POC.py
Isn't this just a test that goes through all the settings? What makes it specifically an rtl test?

Comment 15

[Martijn Wargers (dead)]

(In reply to Ahmed Nefzaoui [:Nefzaoui] from comment #13)
> (In reply to Martijn Wargers [:mwargers] (QA) from comment #11)
> > Supposedly, I could also use --locale=ar-sa in the run, but that didn't seem
> > to work, either.
> 
> Logically, we still don't have multi-arabic locales/variations, there's only
> ar for now (hope that helps) :)

Thanks, that did the trick.

Comment 16

[Martijn Wargers (dead)]

I was thinking, with mochitest/reftest, you have the --setpref option to add preferences you like added, I think we could use a similar thing regarding --setsetting. But I guess that is more something for a new bug.

Comment 17

[Martijn Wargers (dead)]

Comment on attachment 8630114 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30847

Ok, once you updated the pull request, I'll take another look.
But the failures in comment 9 that I was seeing are an issue, no?

Comment 18

[No-Jun Park [:njpark]]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #14)
> I also don't understand why this is an rtl test: 
> tests/python/gaia-ui-tests/gaiatest/tests/graphics/RTL/test_settings_RTL_POC.
> py
> Isn't this just a test that goes through all the settings? What makes it
> specifically an rtl test?

So the purpose of this is to check whether the surrounding layout displays properly when RTL locale is selected. (Apparently when RTL is selected, some of the css layouts display unexpected outcome) This can also be used for any non-RTL locale though.

Comment 19

[No-Jun Park [:njpark]]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #9)
> These were the failures I was seeing when doing an imagcompare run:
> http://people.mozilla.org/~mwargers/imgcomparescreenshots/mismatches/
> Failures in application storage, Date&Time, Developer (Devtools wifi Device
> Name), Language (because of time), Wifi.
> Perhaps it would be good if certain areas of imagecompare could be ignored?

This is correct, so I also need to come up with the procedure in regards to how to use this tool to speed up RTL testing, because even though the comparision failed due to the date/time/wifi differences, the tester can take a look at the diff file and easily see whether they also contain RTL/l10n related valid bug. That I plan to write one up shortly after this is merged to gaia, and hopefully I can get the feedback from Delphine and other RTL testers.

Comment 20

[No-Jun Park [:njpark]]

Comment on attachment 8630114 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30847

incorporated feeback.

Comment 21

[Martijn Wargers (dead)]

Thanks for all the explanations, No-Jun, appreciated. I'll take one last time on it tomorrow, then I'm done.

These are the mismatches I got running a second time: http://people.mozilla.org/~mwargers/imgcomparescreenshots/mismatches2/
Just in case they are interesting.

One thing that is going wrong, I think, is when I've set language.current=nl in my testvars.json file, then that language is not picked up and instead we go back to the default (which is en-US). Please fix that. For the rest, it looks good to me.

Comment 22

[No-Jun Park [:njpark]]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #21)
> Thanks for all the explanations, No-Jun, appreciated. I'll take one last
> time on it tomorrow, then I'm done.
> 
> These are the mismatches I got running a second time:
> http://people.mozilla.org/~mwargers/imgcomparescreenshots/mismatches2/
> Just in case they are interesting.
> 
> One thing that is going wrong, I think, is when I've set language.current=nl
> in my testvars.json file, then that language is not picked up and instead we
> go back to the default (which is en-US). Please fix that. For the rest, it
> looks good to me.
I caused that bug today, but after following your suggestion, it seems to work for all 3 cases. (empty testvar, testvar containing language.locale value, and command prompt)

Comment 23

[Martijn Wargers (dead)]

Comment on attachment 8630114 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30847

Thanks, I tested it one more time and the test works fine (apart from the failures I mentioned earlier already and which you are aware of).

It makes me think, btw, that we need our regular Gaia UI tests to be able to run and not depend on locale specific things like we have with self.apps.displayed_app.name:
http://mxr.mozilla.org/gaia/search?string=self.apps.displayed_app.name

Comment 24

[No-Jun Park [:njpark]]

I discovered this recently too, as most of our scripts will fail if the locale is changed to arabic or other non-english locale.  Perhaps in the future we should make use of data-l10n-name element rather than ID or CLASSNAME?

Comment 25

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8630114 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30847

LGTM. Passes fine on my device. I agree, let's deal with the app_display_name issue in another bug.

Comment 26

[No-Jun Park [:njpark]]

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

Will monitor the results of imagecompare runs on next few days to makes sure there is no other fallout.