Last Comment Bug 762798 - l10n tests shouldn't make use of window title but windowtype property
: l10n tests shouldn't make use of window title but windowtype property
Status: RESOLVED FIXED
[mozmill-l10n][lib][mozmill-test-fail...
:
Product: Mozilla QA
Classification: Other
Component: Mozmill Tests (show other bugs)
: unspecified
: All All
: -- critical (vote)
: ---
Assigned To: Henrik Skupin (:whimboo)
:
Mentors:
http://mozmill-ci.blargon7.com/#/l10n...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-08 00:31 PDT by Henrik Skupin (:whimboo)
Modified: 2012-08-14 15:06 PDT (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed
fixed
fixed
fixed


Attachments
Patch v1 (14.50 KB, patch)
2012-06-08 02:52 PDT, Henrik Skupin (:whimboo)
anthony.s.hughes: review+
l10n: feedback+
Details | Diff | Splinter Review

Description Henrik Skupin (:whimboo) 2012-06-08 00:31:28 PDT
I'm absolutely not sure what's going on here but some locales are failing by checking a sub window of the preferences dialog. One example is 'is':

http://mozmill-ci.blargon7.com/#/l10n/report/fdec829b93b19c73985be1d38878292c

Not sure yet, which window it is. I will have to find this out next.
Comment 1 Henrik Skupin (:whimboo) 2012-06-08 02:18:30 PDT
So in the 'is' case it's because of a broken property string:

http://hg.mozilla.org/releases/l10n/mozilla-aurora/is/file/195eb83e843e/browser/chrome/browser/preferences/preferences.properties#l19

cookiepermissionstitle=Undanþágur - Smákökur\n

The final backslash shouldn't be there. That means we should always try to get the window by type and not by title. So we wouldn't be affected by those issues.
Comment 2 Henrik Skupin (:whimboo) 2012-06-08 02:51:00 PDT
Here an example with the fix in-place:
http://mozmill-crowd.blargon7.com/#/l10n/report/87961186c2b807b7747aa7e6d4076279
Comment 3 Henrik Skupin (:whimboo) 2012-06-08 02:52:18 PDT
Created attachment 631323 [details] [diff] [review]
Patch v1
Comment 4 Axel Hecht [:Pike] 2012-06-08 04:49:19 PDT
Comment on attachment 631323 [details] [diff] [review]
Patch v1

Review of attachment 631323 [details] [diff] [review]:
-----------------------------------------------------------------

I guess I can do a feedback+ on this patch, but not a real review, in particular as I have no estimate on teh impact of the dom-utils changes.

The general idea to identify windows by type sounds good to me, too.

I wonder if the "surprising" titles would actually make a good test themselves?
Comment 5 Henrik Skupin (:whimboo) 2012-06-08 04:52:50 PDT
Comment on attachment 631323 [details] [diff] [review]
Patch v1

(In reply to Axel Hecht [:Pike] from comment #4)
> I guess I can do a feedback+ on this patch, but not a real review, in
> particular as I have no estimate on teh impact of the dom-utils changes.

Alright. The changes there only affect l10n tests and no other test-run.

> The general idea to identify windows by type sounds good to me, too.
> 
> I wonder if the "surprising" titles would actually make a good test
> themselves?

Yes, all that should be a separate test where we check the translation and functionality of items. But that's a whole different story we will not have time to work on in the foreseeable future.

Anthony, mind giving a review today? This failure kills some of our testruns and I would like to get it checked into each branch. Thanks.
Comment 6 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-06-08 10:14:20 PDT
Comment on attachment 631323 [details] [diff] [review]
Patch v1

Review of attachment 631323 [details] [diff] [review]:
-----------------------------------------------------------------

Patch looks fine to me. Please land.

Note You need to log in before you can comment on or make changes to this bug.