439164 - [FIXr]intermittent mochitest failure in test_bug413310

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

We're investigating intermittent test failures, and find that this test
sometimes passes, sometimes fails, even when there are no code changes. Its
happened with a few builds this week. (It was happening before, as well as
after, the recent NetApp woes). 

Can you review how this test was written and see if the test can be made more
robust? 


suite: mochitest
stdio line#13065
error msg: *** 13678 ERROR FAIL Error thrown during test: doNextStep is not defined /tests/docshell/test/test_bug413310.html	

Note: This test does not *break* builds, they only fail out as orange.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Hmm.  Yeah, I think I see how this could happen.  Diff that should help coming up.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Like s

The comments more or less explain what the change is here.

Comment 3

[Lukas Blakk [:lsblakk] use ?needinfo]

John - is the patch good to go?

Comment 4

[Rob Campbell [:rc] (:robcee)]

Boris: this should probably be reviewed by Waldo (who's afk), sayrer or myself. R+ from me though you should add a space to the comment line (NOTget):

+// modified accordingly!  It assumes that subframes do NOTget bfcached.

I'm not sure this will fix the problem as we're still using explicit timeout values here, but the order might improve reliability.

Also, as a final note, it's well-past May now. Might want to change your "very little time" message. Helpful tip provided at no extra charge!

Comment 5

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Minor nit: can you fix typo in comment "do NOTget " should be "do NOT get "! 

Comment 6

[Boris Zbarsky [:bzbarsky]]

> I'm not sure this will fix the problem as we're still using explicit timeout
> values here

That's OK.  All that's needed there is that the code run async.  When is not important.  I almost changed the timeout value to 0 to make that clearer.

And fixed the name.  If forgot to update it when late May didn't free up...

I won't be able to check this in for probably a week or so.  If someone can get to it before then, great.

Comment 7

[Mike Schroepfer]

Gavin or reed can you land this?

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Landed on mozilla-central:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/6f6c7d842726
and CVS trunk:
mozilla/docshell/test/test_bug413310.html 	1.2 

Comment 9

[Boris Zbarsky [:bzbarsky]]

Should be fixed.