Last Comment Bug 705182 - Timeout exceeded for waitForElement ID: getMeOutButton in testSafeBrowsingWarningPages.js
: Timeout exceeded for waitForElement ID: getMeOutButton in testSafeBrowsingWar...
Status: RESOLVED FIXED
[mozmill-test-failure] s=q3 u=failure...
: regression
Product: Mozilla QA
Classification: Other
Component: Mozmill Tests (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Maniac Vlad Florin (:vladmaniac)
:
Mentors:
Depends on:
Blocks: 468313
  Show dependency treegraph
 
Reported: 2011-11-24 12:28 PST by Aaron Train [:aaronmt]
Modified: 2012-09-13 00:42 PDT (History)
5 users (show)
hskupin: in‑litmus? (vlad.mozbugs)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed
fixed
fixed
fixed


Attachments
default (mozilla.com to mozilla.org) (1.21 KB, patch)
2011-11-24 16:00 PST, Aaron Train [:aaronmt]
no flags Details | Diff | Splinter Review
disable test [landed:default,release,esr] (1.09 KB, patch)
2011-11-28 17:53 PST, Aaron Train [:aaronmt]
anthony.s.hughes: review+
Details | Diff | Splinter Review
proposed fix v1.0 (2.53 KB, patch)
2012-09-11 02:24 PDT, Maniac Vlad Florin (:vladmaniac)
hskupin: review-
Details | Diff | Splinter Review
fix patch v1.1 (3.75 KB, patch)
2012-09-11 03:48 PDT, Maniac Vlad Florin (:vladmaniac)
hskupin: review-
Details | Diff | Splinter Review
patch v1.2 (3.75 KB, patch)
2012-09-11 04:17 PDT, Maniac Vlad Florin (:vladmaniac)
hskupin: review+
Details | Diff | Splinter Review
esr patch v1.0 (3.91 KB, patch)
2012-09-13 00:27 PDT, Maniac Vlad Florin (:vladmaniac)
hskupin: review+
Details | Diff | Splinter Review

Description Aaron Train [:aaronmt] 2011-11-24 12:28:27 PST
Test: testWarningPages
Module: testSafeBrowsingWarningPages.js
Error: Timeout exceeded for waitForElement ID: getMeOutButton
Branch: 11.0a1 (Nightly)
Platform: Windows NT 5.0.2195, Windows NT 5.1.2600, Windows NT 6.0.6002, Windows NT 6.1.7601,  Mac OS X 10.6.8, Linux Ubuntu 10.10

http://mozmill-release.brasstacks.mozilla.com/#/functional/failure?test=%2FtestSecurity%2FtestSafeBrowsingWarningPages.js&func=testSafeBrowsingWarningPages.js%3A%3AtestWarningPages

Failing since November 11th, 2011.
Comment 1 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-11-24 13:06:11 PST
This could be a regression in Firefox 11. Aaron, can you check to see if there were any recent changes?

FYI, we have had this issue before: bug 655885
Comment 2 Aaron Train [:aaronmt] 2011-11-24 13:32:13 PST
activities in bug 693389 looks to be the culprit from the associated push log
Comment 3 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-11-24 15:09:40 PST
Looks to be. We have been dealing with this in one-off tests. I suggest filing a bug to do a broad sweep to update all tests referencing mozilla.com safe-browsing pages.

Aaron, would you be willing to take care of that?
Comment 4 Aaron Train [:aaronmt] 2011-11-24 16:00:54 PST
Created attachment 576836 [details] [diff] [review]
default (mozilla.com to mozilla.org)

All other tests under the default branch appear to be good with those recent changes. Note I have not tested this.
Comment 5 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-11-28 17:07:48 PST
Test still fails for me, even with this change. Aaron, could you please continue to investigate? In the meantime, please submit a patch which marks this test skipped.

Thanks
Comment 6 Aaron Train [:aaronmt] 2011-11-28 17:53:57 PST
Created attachment 577448 [details] [diff] [review]
disable test [landed:default,release,esr]
Comment 7 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-02 17:10:23 PST
Comment on attachment 577448 [details] [diff] [review]
disable test [landed:default,release,esr]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/1194b3ad8f19 (default)
Comment 8 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-02 17:11:19 PST
Test has now been skipped. Aaron, do you have time to investigate this failure? If not I will have to re-assign it.

Thanks
Comment 9 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-02-08 14:31:43 PST
This test remains enabled on release branch and I noticed it's failing there; porting disable patch to release branch.
Comment 10 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-02-08 14:34:44 PST
Comment on attachment 577448 [details] [diff] [review]
disable test [landed:default,release,esr]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/7b81f660a18f (mozilla-release)
Comment 11 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-03 07:34:35 PST
Comment on attachment 577448 [details] [diff] [review]
disable test [landed:default,release,esr]

Noticed a failure today on ESR branch so I've ported the disable patch.

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/e8cfa158b5f0 (mozilla-esr10)
Comment 12 Dave Hunt (:davehunt) 2012-04-27 08:07:23 PDT
I noticed this is the only bug with [mozmill-test-skipped] that s not currently assigned. Can someone pick this up?
Comment 13 Maniac Vlad Florin (:vladmaniac) 2012-09-10 06:27:53 PDT
This is failing because bug 468313 has landed. 
So this makes this test invalid at least for the 'http://www.mozilla.com/firefox/its-an-attack.html'
Do you suggest we split 'testSafeBrowsingWarningPages.js' into two tests? One to handle 'its a trap' page and one to handle the 'its an attack' because for the last one we need different checks. 

In any case, this is not about enabling this test anymore, we need to consult Anthony to get some feedback from him before we proceed because a manual test case is not present in MozTrap at the moment afaik.
Comment 14 Henrik Skupin (:whimboo) 2012-09-10 12:11:00 PDT
Please see bug 468313 comment 35 for the solution.
Comment 15 Henrik Skupin (:whimboo) 2012-09-10 12:12:13 PDT
(In reply to Maniac Vlad Florin (:vladmaniac) from comment #13)
> Do you suggest we split 'testSafeBrowsingWarningPages.js' into two tests?
> One to handle 'its a trap' page and one to handle the 'its an attack'
> because for the last one we need different checks. 

You can file a follow-up bug for that but it doesn't affect our work to get this failure fixed.
Comment 16 Maniac Vlad Florin (:vladmaniac) 2012-09-11 02:24:03 PDT
Created attachment 660014 [details] [diff] [review]
proposed fix v1.0

Then its easy. We are setting the permission target wrongly because mozilla.com redirects to mozilla.org. A simple domain change in the test fixes the problem
Comment 17 Henrik Skupin (:whimboo) 2012-09-11 03:13:53 PDT
Comment on attachment 660014 [details] [diff] [review]
proposed fix v1.0

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

You missed to enable the test in the manifest. Things like that should really be caught by your internal review.

::: tests/functional/testSecurity/testSafeBrowsingWarningPages.js
@@ +18,5 @@
>  }
>  
>  function teardownModule(module) {
>    // Clear the Safe Browsing permission
> +  utils.removePermission("www.mozilla.org", "safe-browsing");

Please move the domain out to a global constant so that we do not have to fix that on multiple places when something gets changed.
Comment 18 Maniac Vlad Florin (:vladmaniac) 2012-09-11 03:48:10 PDT
Created attachment 660027 [details] [diff] [review]
fix patch v1.1

* fixed
Comment 19 Henrik Skupin (:whimboo) 2012-09-11 04:07:01 PDT
Comment on attachment 660027 [details] [diff] [review]
fix patch v1.1

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

::: tests/functional/testSecurity/testSafeBrowsingWarningPages.js
@@ +25,5 @@
>  }
>  
>  var testWarningPages = function() {
> +  var urls = ['http://www.mozilla.org/firefox/its-a-trap.html',
> +              'http://www.mozilla.org/firefox/its-an-attack.html'];

Please do internal reviews before uploading updated patches, Vlad! I don't think that this wouldn't have caused a discussion. The domain name is still present here.
Comment 20 Maniac Vlad Florin (:vladmaniac) 2012-09-11 04:17:00 PDT
Created attachment 660029 [details] [diff] [review]
patch v1.2

this passed internal review
Comment 21 Henrik Skupin (:whimboo) 2012-09-11 07:14:43 PDT
Comment on attachment 660029 [details] [diff] [review]
patch v1.2

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

::: tests/functional/testSecurity/testSafeBrowsingWarningPages.js
@@ +25,5 @@
>  }
>  
>  var testWarningPages = function() {
> +  var urls = ['http://' + DOMAIN_NAME + '/firefox/its-a-trap.html',
> +              'http://' + DOMAIN_NAME + '/firefox/its-an-attack.html'];

A global constant for those test pages would be nice. Mind to file a mentored follow-up bug?
Comment 22 Henrik Skupin (:whimboo) 2012-09-11 07:16:55 PDT
Landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/eac1daad96b4

Please check the results tomorrow so that we can land this patch on the other branches.
Comment 23 Aaron Train [:aaronmt] 2012-09-11 07:40:51 PDT
Glad to see a fix here :-)
Comment 24 Maniac Vlad Florin (:vladmaniac) 2012-09-12 05:58:35 PDT
(In reply to Henrik Skupin (:whimboo) from comment #22)
> Landed on default:
> http://hg.mozilla.org/qa/mozmill-tests/rev/eac1daad96b4
> 
> Please check the results tomorrow so that we can land this patch on the
> other branches.

I see no failures today so I think we can land this on other branches
Comment 25 Maniac Vlad Florin (:vladmaniac) 2012-09-12 06:04:00 PDT
Added bug 790597 for the follow up to move the url variable in the global scope as a constant. 
Henrik, mind adding the necessary whiteboard entries? I would add them but not sure I can get it right
Comment 26 Henrik Skupin (:whimboo) 2012-09-12 06:53:14 PDT
http://hg.mozilla.org/qa/mozmill-tests/rev/600510f8a7f2 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/9daf3a935e1c (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/ef42f41a046a (release)

Patch cannot be applied on esr10 due to differences in manifest.ini. Please come up with a working backport patch. In the future please check that before you give the go for backporting.
Comment 27 Maniac Vlad Florin (:vladmaniac) 2012-09-13 00:27:42 PDT
Created attachment 660729 [details] [diff] [review]
esr patch v1.0

this is the backport patch for esr, should apply and work as expected
Comment 28 Henrik Skupin (:whimboo) 2012-09-13 00:42:14 PDT
Landed on esr10 as:
http://hg.mozilla.org/qa/mozmill-tests/rev/867264df02f7

Vlad, please update the Litmus test on the Firefox 10 branch.

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