Closed
Bug 1180819
Opened 9 years ago
Closed 9 years ago
Add features in imagecompare to help automating RTL acceptance tests
Categories
(Firefox OS Graveyard :: Gaia::UI Tests, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: njpark, Assigned: njpark)
References
Details
Attachments
(1 file, 1 obsolete file)
RTL QA can benefit from the imagecompare tool if certain features are added, such as locale selection.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → npark
Comment 1•9 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
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)
Attachment #8630114 -
Flags: review?(martijn.martijn)
Attachment #8630114 -
Flags: review?(jlorenzo)
Attachment #8630114 -
Flags: feedback?(gmealer)
Assignee | ||
Updated•9 years ago
|
Attachment #8630072 -
Attachment description: [gaia] npark-mozilla:RTLImageCompare > mozilla-b2g:master → Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30847
Attachment #8630072 -
Attachment is obsolete: true
Comment on attachment 8630114 [details]
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.
Attachment #8630114 -
Flags: feedback?(gmealer) → feedback+
Comment 4•9 years ago
|
||
Comment on attachment 8630114 [details]
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.
Attachment #8630114 -
Flags: review?(martijn.martijn) → review+
Assignee | ||
Comment 5•9 years ago
|
||
refactoring of menu opening methods is now pushed into the pull request
Comment on attachment 8630114 [details]
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!
Attachment #8630114 -
Flags: review?(jlorenzo)
Comment 7•9 years ago
|
||
Comment on attachment 8630114 [details]
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.
Attachment #8630114 -
Flags: review+ → review?(martijn.martijn)
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8630114 [details]
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.
Attachment #8630114 -
Flags: review?(jlorenzo)
Comment 9•9 years ago
|
||
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•9 years ago
|
||
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•9 years ago
|
||
Supposedly, I could also use --locale=ar-sa in the run, but that didn't seem to work, either.
Comment 12•9 years ago
|
||
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•9 years ago
|
||
(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•9 years ago
|
||
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•9 years ago
|
||
(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•9 years ago
|
||
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•9 years ago
|
||
Comment on attachment 8630114 [details]
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?
Attachment #8630114 -
Flags: review?(martijn.martijn)
Assignee | ||
Comment 18•9 years ago
|
||
(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.
Assignee | ||
Comment 19•9 years ago
|
||
(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.
Assignee | ||
Comment 20•9 years ago
|
||
Comment on attachment 8630114 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30847
incorporated feeback.
Attachment #8630114 -
Flags: review?(martijn.martijn)
Comment 21•9 years ago
|
||
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.
Updated•9 years ago
|
Attachment #8630114 -
Flags: review?(martijn.martijn)
Assignee | ||
Comment 22•9 years ago
|
||
(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)
Assignee | ||
Updated•9 years ago
|
Attachment #8630114 -
Flags: review?(martijn.martijn)
Comment 23•9 years ago
|
||
Comment on attachment 8630114 [details]
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
Attachment #8630114 -
Flags: review?(martijn.martijn) → review+
Assignee | ||
Comment 24•9 years ago
|
||
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 on attachment 8630114 [details]
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.
Attachment #8630114 -
Flags: review?(jlorenzo) → review+
Assignee | ||
Comment 26•9 years ago
|
||
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.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•