Test failure 'Favicon is loaded: got '' ' in /testSecurity/testDVCertificate.js

RESOLVED FIXED

Status

Mozilla QA
Mozmill Tests
P2
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: whimboo, Assigned: vladmaniac)

Tracking

({regression})

unspecified
x86
Linux
regression

Firefox Tracking Flags

(firefox14 unaffected, firefox15 unaffected, firefox16 unaffected, firefox17 unaffected, firefox-esr10 fixed)

Details

(Whiteboard: [mozmill-test-failure], URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

6 years ago
This failure has been started on Aug 10th and is only visible for ESR10 builds on Linux 32bit. Not sure what has been changed but we should at least check if something has been checked into the branch which broke our tests.

Could someone check the appropriate builds around this date please?
(Assignee)

Comment 1

6 years ago
Since its intermittent failures day for me I'll gladly take a look
Assignee: nobody → vlad.mozbugs
Status: NEW → ASSIGNED
(Assignee)

Comment 3

6 years ago
Created attachment 652753 [details] [diff] [review]
proposed fix v1.0

* replaced assert with waitFor, in case the favicon is loaded lazily under heavy environment.
Attachment #652753 - Flags: review?(hskupin)
(Assignee)

Updated

6 years ago
Attachment #652753 - Flags: review?(dave.hunt)
(Reporter)

Comment 4

6 years ago
Comment on attachment 652753 [details] [diff] [review]
proposed fix v1.0

>   var favicon = new elementslib.ID(controller.window.document, "page-proxy-favicon");
>-  controller.assert(function () {
>+  controller.waitFor(function () {
>     return favicon.getNode().src.indexOf("mozqa.com") !== -1;
>   }, "Favicon is loaded: got '" + favicon.getNode().src + "'");

As said a couple of times in the past, you can't refer to the real node of the element you want to test, because any statement in the message will be executed *before* the waitFor() method gets entered. So this will give an error.
Attachment #652753 - Flags: review?(hskupin)
Attachment #652753 - Flags: review?(dave.hunt)
Attachment #652753 - Flags: review-
(Assignee)

Comment 5

6 years ago
Created attachment 653681 [details] [diff] [review]
patch v1.1 (esr10) [checked-in]

* killed the node reference from the waitFor message, it should not have been there in the first place 

Still, I think a waitFor will fix our issue here
Attachment #652753 - Attachment is obsolete: true
Attachment #653681 - Flags: review?(hskupin)
(Assignee)

Updated

6 years ago
Attachment #653681 - Flags: review?(dave.hunt)
(Reporter)

Updated

6 years ago
Attachment #653681 - Flags: review?(hskupin)
Attachment #653681 - Flags: review?(dave.hunt)
Attachment #653681 - Flags: review+
(Reporter)

Comment 6

6 years ago
Landed on esr10 first:
http://hg.mozilla.org/qa/mozmill-tests/rev/c0a07422ee08

If the failure is gone we also want to have this fix for the more recent branches.
status-firefox-esr10: --- → fixed
status-firefox14: --- → affected
status-firefox15: --- → affected
status-firefox16: --- → affected
status-firefox17: --- → affected
(Assignee)

Comment 7

6 years ago
(In reply to Henrik Skupin (:whimboo) from comment #6)
> Landed on esr10 first:
> http://hg.mozilla.org/qa/mozmill-tests/rev/c0a07422ee08
> 
> If the failure is gone we also want to have this fix for the more recent
> branches.

Seems like its green so far, but better wait some more since this was intermittent in the first place. 

http://mozmill-ci.blargon7.com/#/functional/report/d87d47fd1034f072b9bece6ee684ad9b
(Assignee)

Comment 9

6 years ago
Sorry - wrong bug :) please ignore comment 8
(Reporter)

Comment 10

6 years ago
Vlad, please test if this patch can be applied to the other branches.
(Assignee)

Comment 11

6 years ago
(In reply to Henrik Skupin (:whimboo) from comment #10)
> Vlad, please test if this patch can be applied to the other branches.

Its not applicable - should I come up with fixes for other branches? 
It did not fail there, but still if we want to backport its fine, just give the go :)
(Assignee)

Comment 12

6 years ago
Created attachment 654958 [details] [diff] [review]
patch (other branches)

* this will work and apply cleanly for mozilla-release, mozilla-beta, mozilla-aurora and default
Attachment #654958 - Flags: review?(hskupin)
(Assignee)

Updated

6 years ago
Attachment #654958 - Flags: review?(dave.hunt)
(Reporter)

Updated

6 years ago
Attachment #653681 - Attachment description: proposed fix v1.1 → patch v1.1 (esr10) [checked-in]
(Reporter)

Comment 13

6 years ago
Comment on attachment 654958 [details] [diff] [review]
patch (other branches)

This is not a backport patch. And yes it's different. So as what I can see is that we only would have to change the controller.assert() call into expect.*(). There is no waitFor() call necessary because the attribute is getting set immediately. And the before-mentioned change will happen on bug 782918.

As conclusion we do not have to change anything for other branches.
Attachment #654958 - Attachment description: backport v 1.0 → patch (other branches)
Attachment #654958 - Attachment is obsolete: true
Attachment #654958 - Flags: review?(hskupin)
Attachment #654958 - Flags: review?(dave.hunt)
(Reporter)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
status-firefox14: affected → unaffected
status-firefox15: affected → unaffected
status-firefox16: affected → unaffected
status-firefox17: affected → unaffected
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.