916005 - [sq] Test failure "Timeout exceeded for waitForElement Lookup" in /testSecurity/testSafeBrowsingNotificationBar.js

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect, P2)

Description

[Cosmin Malutan, [:cosmin-malutan]]

This failed on Mac x64 with Aurora, sq locale
http://mozmill-release.blargon7.com/#/functional/report/dc3d4d2f66af31b9e75e291fa0345fa1

Comment 1

[Cosmin Malutan, [:cosmin-malutan]]

This one reproduces , so it should be a P1

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Cosmin Malutan from comment #1)
> This one reproduces , so it should be a P1

Not necessarily. Only if it would be failing constantly for beta and release candidate builds. On Aurora we usually don't run the sq locale.

Comment 3

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch_v1.0

(In reply to Henrik Skupin (:whimboo) from comment #2)
> Not necessarily. Only if it would be failing constantly for beta and release
> candidate builds. On Aurora we usually don't run the sq locale.
It actually failed on Release and I failed wrong, but all branches are affected on this locale, except Nightly for whom I couldn't find a build with this locale.
  
It fails because it can't decode the label value clearly so I had to search for element by accessKey.
{\"label\":\"Ky nuk \u00ebsht\u00eb \"site\" sulmesh\u2026\"}

Reports with the patch :
http://mozmill-crowd.blargon7.com/#/remote/report/dc3d4d2f66af31b9e75e291fa0a4c40d
http://mozmill-crowd.blargon7.com/#/remote/report/dc3d4d2f66af31b9e75e291fa0a4de88
http://mozmill-crowd.blargon7.com/#/remote/report/dc3d4d2f66af31b9e75e291fa0a4d15a

Reports without the patch:
http://mozmill-crowd.blargon7.com/#/remote/report/dc3d4d2f66af31b9e75e291fa0a4f674
http://mozmill-crowd.blargon7.com/#/remote/report/dc3d4d2f66af31b9e75e291fa0a4f7f3
http://mozmill-crowd.blargon7.com/#/remote/report/dc3d4d2f66af31b9e75e291fa0a50618

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Cosmin Malutan from comment #3)
> It fails because it can't decode the label value clearly so I had to search
> for element by accessKey.
> {\"label\":\"Ky nuk \u00ebsht\u00eb \"site\" sulmesh\u2026\"}

Can you please check that the current state with the label works with Mozmill 2.0? I have fixed it there a while back and that would be a good proof. Thanks.

Comment 5

[Cosmin Malutan, [:cosmin-malutan]]

Sorry Henrink but it fails on mozmill 2.0rc5 to with JSON.parse error


ERROR | Test Failure | {
  "exception": {
    "stack": [
      "Lookup/reduceLookup@resource://mozmill/driver/elementslib.js:512", 
      "Lookup@resource://mozmill/driver/elementslib.js:536", 
      "Lookup@resource://mozmill/driver/mozelement.js:79", 
      "tabBrowser_getTabPanelElement@resource://mozmill/stdlib/securable-module.js -> file:///home/cosmin/Projects/Mozilla/clean2/mozmill-tests/lib/tabs.js:365", 
      "checkNoPhishingButton@resource://mozmill/modules/frame.js -> file:///home/cosmin/Projects/Mozilla/clean2/mozmill-tests/tests/remote/testSecurity/testSafeBrowsingNotificationBar.js:115", 
      "testNotificationBar/<@resource://mozmill/modules/frame.js -> file:///home/cosmin/Projects/Mozilla/clean2/mozmill-tests/tests/remote/testSecurity/testSafeBrowsingNotificationBar.js:49", 
      "testNotificationBar@resource://mozmill/modules/frame.js -> file:///home/cosmin/Projects/Mozilla/clean2/mozmill-tests/tests/remote/testSecurity/testSafeBrowsingNotificationBar.js:42", 
      "Runner.prototype.execFunction@resource://mozmill/modules/frame.js:756", 
      "Runner.prototype.runTestModule@resource://mozmill/modules/frame.js:710", 
      "Runner.prototype.runTestFile@resource://mozmill/modules/frame.js:694", 
      "runTestFile@resource://mozmill/modules/frame.js:779", 
      "Bridge.prototype._execFunction@resource://jsbridge/modules/Bridge.jsm:140", 
      "Bridge.prototype.execFunction@resource://jsbridge/modules/Bridge.jsm:147", 
      "@resource://jsbridge/modules/Server.jsm:32", 
      ""
    ], 
    "message": "SyntaxError: JSON.parse. String to be parsed was || {\"label\":\"Ky nuk \u00ebsht\u00eb \"site\" sulmesh\u2026\"} ||", 
    "fileName": "resource://mozmill/driver/elementslib.js", 
    "name": "SyntaxError", 
    "lineNumber": 512
  }
}

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hm, please file a bug with a minimized testcase then please. Thanks.

Comment 7

[Andrei Eftimie]

Comment on attachment 805184 [details] [diff] [review]
patch_v1.0

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

I like not relying on the label anymore.
Looks good, seems to work fine across branches. Works fine in mozmill 2.0 as well.

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/614bac8d09f8 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/fe2c35e2c327 (mozilla-aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/24586655a449 (mozilla-beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/770c19430d8c (mozilla-release)
http://hg.mozilla.org/qa/mozmill-tests/rev/5231b60ab611 (mozilla-esr24)
http://hg.mozilla.org/qa/mozmill-tests/rev/1d40c6dad03c (mozilla-esr17)

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Andrei, please do not land fixes on all branches at once. The risk is to high that something regresses. We should first get the confidence from at least nightly and aurora. Then we should backport to beta and release branches.

Comment 9

[Andrei Eftimie]

(In reply to Henrik Skupin (:whimboo) from comment #8)
> Andrei, please do not land fixes on all branches at once. The risk is to
> high that something regresses. We should first get the confidence from at
> least nightly and aurora. Then we should backport to beta and release
> branches.

Ok, I'll be more conservative.

In this case, the change is relatively small, we know exactly what we changed, and it only affects 1 test. I've manually ran the test on each branch on both an sq localised version as well as on en-US.

With that in mind, the confidence for this patch is high that it doesn't regress. In the event that it does, only 1 test is affected, and can be easily backed out. We don't run [sq] on Nightly at all (we have no builds), and we don't run [sq] on Aurora on a regular basis. I'm not sure we've ever run it. This locale is called out on ondemand tests atm, so we've had to firstly land it on mozilla-beta and/or mozilla-release.

For this particular case I do feel that landing on all branches was the right call.

As I've said, noted, and will usually land changes on default first and see how they react there before backporting them.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Andrei Eftimie from comment #9)
> With that in mind, the confidence for this patch is high that it doesn't
> regress. In the event that it does, only 1 test is affected, and can be
> easily backed out. We don't run [sq] on Nightly at all (we have no builds),
> and we don't run [sq] on Aurora on a regular basis. I'm not sure we've ever
> run it. This locale is called out on ondemand tests atm, so we've had to
> firstly land it on mozilla-beta and/or mozilla-release.

I'm only cautious because of the landing on beta and release given that Anthony had to run ondemand tests for all locales of Fx 25.0b1 yesterday. I'm more confident in landing fixes which have been proven to work. So in case of Nightly and Aurora you are right, but we could always trigger them manually via the CI. Well, as long as those builds exist on the FTP server. That way we would know that all will be fine in our production environment. Unexpected failures like the one we discovered with bug 879989 could always happen.