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)

defect

Tracking

(firefox18 fixed, firefox19 fixed, firefox20 fixed, firefox-esr10 fixed, firefox-esr17 fixed)

RESOLVED FIXED
Tracking Status
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)

This is failing since yesterday on Aurora and ESR10, fr and en-US locales, with Linux 12.04 and all Windows.
Why hasn't this test been disabled today across branches? That's a real show stopper for this test!
Priority: -- → P1
Whiteboard: [mozmill-test-failure] → [mozmill-test-failure] s=121119 u=failure c=preferences p=1
Assignee: nobody → andreea.matei
Status: NEW → ASSIGNED
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.
And what's the proposed fix you would suggest here?
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".
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.
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.
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.
Depends on: 813470
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.
Attached patch patch v1 (obsolete) (deleted) — Splinter Review
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 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-
Attached patch patch v2 (obsolete) (deleted) — Splinter Review
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 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-
Blocks: 826251
Attached patch Patch v3 (deleted) — Splinter Review
(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 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+
(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.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
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
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: