Closed Bug 442125 Opened 12 years ago Closed 12 years ago

ensure focus test failing, but no other tests failing (on unittest VMs)

Categories

(Testing :: Mochitest, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ted, Assigned: ajschult784)

References

()

Details

(Keywords: fixed1.9.0.2, intermittent-failure)

Attachments

(3 files)

For some reason the "ensure focus" test from bug 431464 has been failing intermittently on the Linux moz2 unittest box, but none of the other focus-related tests are failing.
It seems a bit strange not only that there are no real failures, but also that focus gets fixed after the next test... I haven't seen this locally.
This only seems to happen on the VMs. We probably need to figure out what weirdness is happening here and make it more reliable in that environment. Andrew: any thoughts on extra debug output we could put into that test to figure out what's happening here?
Blocks: 438871
Summary: ensure focus test failing, but no other tests failing → ensure focus test failing, but no other tests failing (on unittest VMs)
It'd be nice to know what does have focus.  Is it failing because !document.hasFocus() or because document.activeElement is not the iframe (and if not, what is document.activeElement)?

Might it help to move the window.scrollTo inside the if block (before window.focus())?
I don't know, I'm asking you. :) We can add any output that you think would be useful, or modify the test to make it more reliable. The Linux unittest VMs hit this fairly often, we need to track down the cause and fix it.
Duplicate of this bug: 439165
Attached patch patchSplinter Review
this moves the window.scrollTo and adds what has focus to the failure message.
Attachment #330570 - Attachment is patch: true
Attachment #330570 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 330570 [details] [diff] [review]
patch

Please land this so we can start to track down this failure!
Attachment #330570 - Flags: review+
Comment on attachment 330570 [details] [diff] [review]
patch

I landed this.
*** 477 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_bug331215.xul | Unable to restore focus ([none]), expect failures and timeouts.

from http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1216757778.1216761362.9039.gz
OK... so some other window (dialog?) has focus.  Perhaps a new window from the previous test took a long time to close (the ensure focus bits already wait 3 seconds, which seems like plenty).  Focus might get restored when a test opens a window.  In this case the previous test (test_bug304188.xul) did open a new window).  And it looks like the ensure-focus part failed for two tests in a row after that (so that's 6 seconds).  The first test (test_bug331215.xul) didn't open a window while the second (test_bug360437.xul) did.

I'm not sure what else can be added to provide more info.  We need to figure out what has focus, which probably involves someone seeing it fail.  That, or add something like my hack to automation.py.in that takes a screenshot when it sees something in the log.

http://rheneas.eng.buffalo.edu/~andrew/screenshot_hack
We saw in another bug where setTimeout(N, ...) would fire the associated function, but Date.now() would show that no time had actually elapsed. I wonder if we're hitting something similar here, where our timeout fires, but to the VM, no time has really elapsed. (I don't know how that happens, but apparently it does!) Maybe you could add something here to store the current time, and display how much time Date.now() thinks has elapsed between setTimeout calls? Might help us narrow it down.
Attached patch check Date.now()Splinter Review
Attachment #330889 - Flags: review?(ted.mielczarek)
Comment on attachment 330889 [details] [diff] [review]
check Date.now()

Worth a shot, for sure.
Attachment #330889 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 330889 [details] [diff] [review]
check Date.now()

OK, landed this too.
looks like it is waiting 3 seconds:

*** 477 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_bug331215.xul | Unable to restore focus after 2.974 seconds ([none]), expect failures and timeouts.
So I was thinking, how do you feel about changing this to a purely informational message, instead of using ok()? While it's a useful diagnostic, it's not worth having random orange over. We could change it to just log "Error: unable to restore focus...", so that it'd show up in the brief tinderbox log, but not be counted as a testfail.
Assignee: nobody → ajschult
Status: NEW → ASSIGNED
Attachment #331307 - Flags: review?(ted.mielczarek)
Comment on attachment 331307 [details] [diff] [review]
reverts previous patches and turns failure into a warning

+            TestRunner.logger.log("Unable to restore focus, expect failures and timeouts.");

Please make this start with "Error: " so that the tinderbox error parser will still show it in the brief log.

Thanks!
Attachment #331307 - Flags: review?(ted.mielczarek) → review+
I checked this in for you with my suggested change:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/bfd3534f3473
http://hg.mozilla.org/mozilla-central/index.cgi/rev/654ad593ecd2

(in two changesets because I forgot to make my suggested change the first time)
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
(In reply to comment #18)
> (From update of attachment 331307 [details] [diff] [review])
> +            TestRunner.logger.log("Unable to restore focus, expect failures
> and timeouts.");

While the elapsed time proved to be working as expected, so not useful anymore;
and same for the focus owner which seemed to always be "none";
wasn't the name of the test interesting to (still) have on that line too ?

Could it be re-added ?
(even if it used(?) to be provided by a function we're not using anymore.)
I checked in the effective diff of all these patches into CVS (produced by doing  hg diff -r 3a6edfb3fa9f -r 654ad593ecd2 testing/mochitest/tests/SimpleTest/TestRunner.js):

Checking in testing/mochitest/tests/SimpleTest/TestRunner.js;
/cvsroot/mozilla/testing/mochitest/tests/SimpleTest/TestRunner.js,v  <--  TestRu
nner.js
new revision: 1.17; previous revision: 1.16
done
Keywords: fixed1.9.0.2
Component: Testing → Mochitest
Product: Core → Testing
QA Contact: testing → mochitest
Version: Trunk → unspecified
Whiteboard: [orange]
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.