705182 - Timeout exceeded for waitForElement ID: getMeOutButton in testSafeBrowsingWarningPages.js

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Aaron Train [:aaronmt]]

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

[u279076]

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]]

activities in bug 693389 looks to be the culprit from the associated push log

Comment 3

[u279076]

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]]

Attachment: 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

[u279076]

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]]

Attachment: disable test [landed:default,release,esr]

Comment 7

[u279076]

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

[u279076]

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

[u279076]

This test remains enabled on release branch and I noticed it's failing there; porting disable patch to release branch.

Comment 10

[u279076]

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

[u279076]

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] [he/him] ⌚BST]

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)]

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][⌚️UTC+1]]

Please see bug 468313 comment 35 for the solution.

Comment 15

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

(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)]

Attachment: 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][⌚️UTC+1]]

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)]

Attachment: fix patch v1.1

* fixed

Comment 19

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

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)]

Attachment: patch v1.2

this passed internal review

Comment 21

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

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][⌚️UTC+1]]

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]]

Glad to see a fix here :-)

Comment 24

[Maniac Vlad Florin (:vladmaniac)]

(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)]

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][⌚️UTC+1]]

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)]

Attachment: esr patch v1.0

this is the backport patch for esr, should apply and work as expected

Comment 28

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

Landed on esr10 as:
http://hg.mozilla.org/qa/mozmill-tests/rev/867264df02f7

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