Closed Bug 940381 Opened 10 years ago Closed 9 years ago

Intermittent TEST-UNEXPECTED-FAIL | /tests/content/svg/content/test/test_viewport.html | Test timed out.

Categories

(Testing :: Mochitest, defect, P5)

Product:
Testing
Component:
Mochitest
Platform:
x86
macOS
Type:
defect

Tracking

(firefox26 unaffected, firefox27 affected, firefox28 affected, firefox-esr24 unaffected)

Status:
RESOLVED WORKSFORME
Tracking Flags:
Tracking Status
firefox26 --- unaffected
firefox27 --- affected
firefox28 --- affected
firefox-esr24 --- unaffected

People

(Reporter: cbook, Unassigned)

References

(
URL
)

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Fix up calls to addEventListener
11.57 KB, patch
dholbert
: review+
Details | Diff | Splinter Review 
Rev4 MacOSX Lion 10.7 mozilla-aurora opt test mochitest-1 on 2013-11-18 16:10:05 PST for push 1c1fcdc50acc

slave: talos-r4-lion-069

https://tbpl.mozilla.org/php/getParsedLog.php?id=30730024&tree=Mozilla-Aurora


198574 ERROR TEST-UNEXPECTED-FAIL | /tests/content/svg/content/test/test_viewport.html | Test timed out.
198577 ERROR TEST-UNEXPECTED-FAIL | /tests/content/svg/content/test/test_viewport.html | uncaught exception - TypeError: inner is null at http://mochi.test:8888/tests/content/svg/content/test/test_viewport.html:37
198578 ERROR TEST-UNEXPECTED-FAIL | /tests/content/svg/content/test/test_viewport.html | [SimpleTest.finish()] this test already called finish!
198579 ERROR TEST-UNEXPECTED-FAIL | /tests/content/svg/content/test/test_viewport.html | called finish() multiple times
inner is null at ... test_viewport.html:37
http://mxr.mozilla.org/mozilla-central/source/content/svg/content/test/test_viewport.html?force=1#37

Maybe the iframe isn't loaded yet when we execute line 37?
Should the "load" listener on line 74 be on the iframe instead?
Flags: needinfo?(longsonr)
Priority: -- → P5
Yes, I imagine it should be on the iframe.
Flags: needinfo?(longsonr)
Assignee: nobody → longsonr

        Attachment #8340279 -
        Flags: review?(dholbert)

    
    

    
  
I think the iframe's load event blocks the outer document's load event, right?

At least, that seems to be the case for:
data:text/html,<body onload="alert('body')"><iframe src="https://people.mozilla.com" onload="alert('iframe')">

Still, I guess this could be worth a shot, and it may get us closer to the problem if it's not already addressing the problem.

    
    

    
  
Comment on attachment 8340279 [details] [diff] [review]
Fix up calls to addEventListener

>diff --git a/content/svg/content/test/test_animLengthReadonly.xhtml b/content/svg/content/test/test_animLengthReadonly.xhtml
>--- a/content/svg/content/test/test_animLengthReadonly.xhtml
>+++ b/content/svg/content/test/test_animLengthReadonly.xhtml
>-var animate = document.getElementById('animate');
>-if (animate && animate.targetElement) {
>-  window.addEventListener("load", main, false);
>-} else {
>-  // SMIL not enabled: skip tests but don't report 'todo' either
>-  ok(true);
>-  SimpleTest.finish();
>-}
>+window.addEventListener("load", main, false);

I'd prefer that this hunk land separately, since it's cleanup that's unrelated to what the rest of this patch is doing.  (Same bug number is fine if you like, with "part 2" or "followup", or even "no bug". r=me on this chunk on its own, regardless.)

In the unlikely event that using the iframe's load event causes other problems and we have to back this patch out, I'd still like to keep that ^^^ (unrelated) cleanup part in.

>+++ b/content/svg/content/test/test_bug872812.html
>@@ -8,18 +8,16 @@ https://bugzilla.mozilla.org/show_bug.cg
>   <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
>   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
> </head>
> <body>
> <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=872812">Mozilla Bug 872812</a>
> <p id="display"></p>
> <div id="content" style="display: none"></div>
> 
>-<iframe id="svg" src="viewport-helper.svg"></iframe>
>-

Similar to above; this is unrelated to the rest of this patch, and I'm unclear on what effect (if any) this might have on the test's behavior. (Note that it's a test for a security bug about assertions, so while the iframe may seem unused, it might be key for triggering the formerly-buggy codepath).

In any case, if you want this change, it deserves a separate bug and probably should have input from the test author, to be sure you're not neutering the test somehow.

>diff --git a/content/svg/content/test/test_text_selection.html b/content/svg/content/test/test_text_selection.html
>--- a/content/svg/content/test/test_text_selection.html
>-<iframe src="text-helper-selection.svg" width="400" height="300"></iframe>
>+<iframe id="svg" src="text-helper-selection.svg" width="400" height="300"></iframe>
> 
[...]
>@@ -132,14 +132,14 @@ function runTest()
>-  window.addEventListener("load", runTest, false);
>+  $("svg").addEventListener("load", runTest, false);

This test currently has
  62 function testSelection()
  63 {
  64   svg = document.getElementsByTagName("iframe")[0];
http://mxr.mozilla.org/mozilla-central/source/content/svg/content/test/test_text_selection.html?force=1#62

Now that you're adding an ID to the iframe and referencing it by that ID, please also fix up line 64's reference to it, for consistency.

r=me with the above addressed.

        Attachment #8340279 -
        Flags: review?(dholbert) → review+
Flags: in-testsuite+

    
    

    
  
(In reply to Daniel Holbert [:dholbert] from comment #4)
> I think the iframe's load event blocks the outer document's load event,
> right?

I think you're right, my mistake.  So maybe the iframe load simply hangs for some reason?
Looking at the log again, I see that the "inner is null" error occurs after the 5 minute
timeout.  So when the test framework is aborting this test (I don't know what that
entails) it's causing the load event to occur?  there's also this:
   [SimpleTest.finish()] this test already called finish!
that must be our call in runTest(), right? but the framework already called finish() as
part of aborting the test?

    
    

    
  
> it's causing the load event to occur?

Aborting a load will in fact fire onload on the aborted document.

> but the framework already called finish() as part of aborting the test?

Entirely possible.

The new test after the comment 6 changes seems _less_ robust to me, because the iframe can finish loading before the script ever runs, so the load listener it adds would never get triggered.  As in, the new code is _definitely_ buggy, while the old code at least has a chance at being correct.

    
    

    
  
(In reply to Boris Zbarsky [:bz] from comment #8)
> > but the framework already called finish() as part of aborting the test?
> 
> Entirely possible.

Yeah, I recall seeing this in other mochitest test timeouts (in cases where there's clearly only one code-path in the test that could reach Finish()). So I wouldn't pay too much attention to that 

> The new test after the comment 6 changes seems _less_ robust to me, because
> the iframe can finish loading before the script [that sets up the load listener] ever runs

Oh, I didn't think of that -- good point.  With that realization, I agree -- this is now less-good, and we should probably back out. :-/

On the bright side, the test-timeout has only ever happened once, AFAICT, so whatever's causing it is pretty anomalous.

    
    

    
  
Backed out part 1: https://hg.mozilla.org/integration/mozilla-inbound/rev/27c14a1b1d4c

(part 2 is unrelated test-simplification and should stay in)

    
    

    
  
My apologies for causing unnecessary work here. :-(
Flags: in-testsuite+

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/c1e2e3d9b4eb
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla28

    
    

    
  
Canceling FIXED setting, as this was backed out after it landed (on inbound).
Status: RESOLVED → REOPENED
Flags: in-testsuite+
Resolution: FIXED → ---
Target Milestone: mozilla28 → ---

    
    

    
  
(and the merged-to-central cset for part 2 in comment 12 is tangential and didn't fix this)

    
    

    
  
The missing iframe load event doesn't seem likely to be an SVG bug.  It seems more likely
the problem is on the server side, in the mochitest framework I presume.
Component: SVG → Mochitest
Product: Core → Testing

    
    

    
  
I doubt this is a Mochitest framework bug. It's likely either a platform bug or a bug in the test.

    
    

    
      
  
Assignee: longsonr → nobody

    
    

    
  
Closing bugs where TBPLbot has previously commented, but have now not been modified for >3 months & do not contain the whiteboard strings for disabled/annotated tests or use the keyword leave-open. Filter on: mass-intermittent-bug-closure-2014-07
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → WORKSFORME

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