Closed
Bug 812435
Opened 12 years ago
Closed 12 years ago
Test failure "could not find element Link: Documenti" in testPreferences/testPreferredLanguage.js
Categories
(Mozilla QA Graveyard :: Mozmill Tests, defect, P1)
Mozilla QA Graveyard
Mozmill Tests
Tracking
(firefox18 fixed, firefox19 fixed, firefox20 fixed, firefox-esr10 fixed, firefox-esr17 fixed)
People
(Reporter: AndreeaMatei, Assigned: AndreeaMatei)
References
()
Details
(Whiteboard: [mozmill-test-failure] s=121119 u=failure c=preferences p=1)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
whimboo
:
review+
whimboo
:
checkin+
|
Details | Diff | Splinter Review |
This is failing since yesterday on Aurora and ESR10, fr and en-US locales, with Linux 12.04 and all Windows.
Assignee | ||
Updated•12 years ago
|
status-firefox-esr10:
--- → affected
status-firefox18:
--- → affected
Whiteboard: [mozmill-test-failure]
Comment 1•12 years ago
|
||
Why hasn't this test been disabled today across branches? That's a real show stopper for this test!
status-firefox16:
--- → affected
status-firefox17:
--- → affected
status-firefox19:
--- → affected
status-firefox-esr17:
--- → affected
Priority: -- → P1
Whiteboard: [mozmill-test-failure] → [mozmill-test-failure] s=121119 u=failure c=preferences p=1
Comment 2•12 years ago
|
||
http://hg.mozilla.org/qa/mozmill-tests/rev/87b1fdf16911 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/5a01f9b4739b (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/af9b42b45d09 (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/e118ad72f4e3 (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/9adf38996ade (esr17)
http://hg.mozilla.org/qa/mozmill-tests/rev/3235c72219a3 (esr10)
Whiteboard: [mozmill-test-failure] s=121119 u=failure c=preferences p=1 → [mozmill-test-failure][mozmill-test-skipped] s=121119 u=failure c=preferences p=1
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → andreea.matei
Assignee | ||
Updated•12 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•12 years ago
|
||
Looked over the google page and the Documenti link indeed no longer exists, that is why the test fails. I believe it was changed into Drive, in the English version we had Docs before, now it's called Drive no matter the language.
Comment 4•12 years ago
|
||
And what's the proposed fix you would suggest here?
Assignee | ||
Comment 5•12 years ago
|
||
I would say we use the word "Immagini" (for Images link) in order to be an italian word, instead of "Drive". The equivalent for Polish is "Grafika".
Comment 6•12 years ago
|
||
This will work until they are making another change. We have to update this test already a couple of times, so I would like to see a better solution here.
Assignee | ||
Comment 7•12 years ago
|
||
We can check the language under the Google logo in the center. For all the words (in the upper bar or the buttons) they can do changes and rename them.
Another option would be to use a new local page that will check the build locale and be loaded accordingly to it.
Comment 8•12 years ago
|
||
I would be happy to get a new test page. It would have to read the HTTP header for the accepted language. I don't think we should implement this via httpd.js but upload it to mozqa.com. It will be a very simple PHP script.
Comment 9•12 years ago
|
||
Sorry, but the new test page is not necessary. See bug 813470 comment 6. Sorry for wasting the time on it.
So just to repeat from the other bug:
That means testing the HTTP header is already done and we do not have to do it again! Instead our test simply has to check if we correctly set the appropriate preference via the preferences UI. There is no real need to have an even external testcase hosted.
Assignee | ||
Comment 10•12 years ago
|
||
We are now checking that the primary language is indeed the one set before. Also fixed some indentation issues.
Attachment #690793 -
Flags: review?(hskupin)
Attachment #690793 -
Flags: review?(dave.hunt)
Comment 11•12 years ago
|
||
Comment on attachment 690793 [details] [diff] [review]
patch v1
Review of attachment 690793 [details] [diff] [review]:
-----------------------------------------------------------------
::: tests/functional/testPreferences/testPreferredLanguage.js
@@ +7,5 @@
> var modalDialog = require("../../../lib/modal-dialog");
> var prefs = require("../../../lib/prefs");
> var utils = require("../../../lib/utils");
>
> +const ACCEPT_LANG_PREF = "intl.accept_languages";
The name should start with PREF_
@@ +27,5 @@
> prefs.openPreferencesDialog(controller, prefDialogCallback);
>
> + // If we test an Italian build, check that the primary language is Polish
> + if (browserLocale === "it") {
> + assert.ok(prefs.preferences.getPref(ACCEPT_LANG_PREF, '').indexOf("pl") === 0,
I would read in the value of the preference before the if/else clause for better readability.
@@ +33,5 @@
> + }
> + else {
> + // Verify the primary language is Italian
> + assert.ok(prefs.preferences.getPref(ACCEPT_LANG_PREF, '').indexOf("it") === 0,
> + "The primary language set is Italian");
Personally I'm not that happy with those hard-coded languages. I would prefer something more dynamically. Would that be easy to do?
@@ +94,5 @@
> // Move the Language to the Top of the List and Accept the new settings
> var upButton = new elementslib.ID(controller.window.document, "up");
>
> while (upButton.getNode().getAttribute("disabled") != "true") {
> controller.click(upButton);
You should leave in the short sleep call so that we do not block the ui.
@@ +107,5 @@
>
> /**
> * Map test functions to litmus tests
> */
> // testSetLanguages.meta = {litmusids : [8322]};
Kill this. Litmus doesn't exist anymore.
@@ +109,5 @@
> * Map test functions to litmus tests
> */
> // testSetLanguages.meta = {litmusids : [8322]};
>
> setupModule.__force_skip__ = "Bug 812435 - Test failure 'could not find element Link: Documenti'";
You want to remove the skip line here and in the manifest.
Attachment #690793 -
Flags: review?(hskupin)
Attachment #690793 -
Flags: review?(dave.hunt)
Attachment #690793 -
Flags: review-
Comment 12•12 years ago
|
||
Addressed Henrik's comments.
> Personally I'm not that happy with those hard-coded languages. I would prefer
> something more dynamically. Would that be easy to do?
I am not sure at this moment. Either me or Andreea will come back regarding a more generalised solution.
Attachment #690793 -
Attachment is obsolete: true
Attachment #696318 -
Flags: review?(hskupin)
Attachment #696318 -
Flags: review?(dave.hunt)
Comment 13•12 years ago
|
||
Comment on attachment 696318 [details] [diff] [review]
patch v2
Review of attachment 696318 [details] [diff] [review]:
-----------------------------------------------------------------
So we should read-out the preferences in setupModule to figure out the currently set languages for content. When we add a new language we could select a random one from the list which is different from the list retrieved in setupModule(). I would say we should file a new bug for that and keep this one for replacing the external content.
::: tests/functional/testPreferences/testPreferredLanguage.js
@@ +7,5 @@
> var modalDialog = require("../../../lib/modal-dialog");
> var prefs = require("../../../lib/prefs");
> var utils = require("../../../lib/utils");
>
> const gDelay = 0;
Please remove this variable too because it doesn't seem necessary to keep. 0ms is a no-op. Please test it.
@@ +31,4 @@
>
> + // If we test an Italian build, check that the primary language is Polish
> + if (browserLocale === "it") {
> + assert.ok(acceptedLanguage.indexOf("pl") === 0,
Don't repeat pl or it all over the place. Create constants for both the current and expected locale. It's hard to follow that way. Also this should be an expect and not an assert.
Attachment #696318 -
Flags: review?(hskupin)
Attachment #696318 -
Flags: review?(dave.hunt)
Attachment #696318 -
Flags: review-
Comment 14•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #13)
> So we should read-out the preferences in setupModule to figure out the
> currently set languages for content. When we add a new language we could
> select a random one from the list which is different from the list retrieved
> in setupModule(). I would say we should file a new bug for that and keep
> this one for replacing the external content.
I like this idea.
I've created bug 826251 for the removal of hardcoded languages.
> ::: tests/functional/testPreferences/testPreferredLanguage.js
> @@ +7,5 @@
> > var modalDialog = require("../../../lib/modal-dialog");
> > var prefs = require("../../../lib/prefs");
> > var utils = require("../../../lib/utils");
> >
> > const gDelay = 0;
>
> Please remove this variable too because it doesn't seem necessary to keep.
> 0ms is a no-op. Please test it.
I've removed the gDelay variable.
Not sure what to test: the test passes with and without the
> controller.sleep(0);
You said earlier to keep the sleep to not block the UI.
I am not sure how and why the UI should be blocked and why a sleep(0) should help unblock it.
> @@ +31,4 @@
> >
> > + // If we test an Italian build, check that the primary language is Polish
> > + if (browserLocale === "it") {
> > + assert.ok(acceptedLanguage.indexOf("pl") === 0,
>
> Don't repeat pl or it all over the place. Create constants for both the
> current and expected locale. It's hard to follow that way. Also this should
> be an expect and not an assert.
Changed to expect.
I'm skipping creating constants since we'll refactor to use a random language in bug 826251 as you suggested.
Attachment #696318 -
Attachment is obsolete: true
Attachment #697426 -
Flags: review?(hskupin)
Attachment #697426 -
Flags: review?(dave.hunt)
Comment 15•12 years ago
|
||
Comment on attachment 697426 [details] [diff] [review]
Patch v3
Review of attachment 697426 [details] [diff] [review]:
-----------------------------------------------------------------
http://hg.mozilla.org/qa/mozmill-tests/rev/5ba754c00190 (default)
If it sticks please check if backports need an update. Also please be aware that I have changed the commit message.
Attachment #697426 -
Flags: review?(hskupin)
Attachment #697426 -
Flags: review?(dave.hunt)
Attachment #697426 -
Flags: review+
Attachment #697426 -
Flags: checkin+
Updated•12 years ago
|
status-firefox20:
--- → fixed
Comment 16•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #15)
> Comment on attachment 697426 [details] [diff] [review]
> Patch v3
>
> Review of attachment 697426 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> http://hg.mozilla.org/qa/mozmill-tests/rev/5ba754c00190 (default)
>
> If it sticks please check if backports need an update. Also please be aware
> that I have changed the commit message.
No further errors.
Applies cleanly to all branches.
Comment 17•12 years ago
|
||
http://hg.mozilla.org/qa/mozmill-tests/rev/8c17ca70a514 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/886b749c7384 (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/4b1a829e0e1d (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/f4557a493df8 (esr17)
http://hg.mozilla.org/qa/mozmill-tests/rev/9c6d7052e67a (esr10)
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
status-firefox16:
disabled → ---
status-firefox17:
disabled → ---
Resolution: --- → FIXED
Whiteboard: [mozmill-test-failure][mozmill-test-skipped] s=121119 u=failure c=preferences p=1 → [mozmill-test-failure] s=121119 u=failure c=preferences p=1
Updated•5 years ago
|
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•