Closed Bug 957213 Opened 10 years ago Closed 10 years ago

Intermittent test_bug944397.html | Failed to generate keyboard input or Test timed out or application timed out after 330.0 seconds with no output or [SimpleTest.finish()] this test already called finish!

Categories

(Core :: Networking: Cache, defect)

Other Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla33
Tracking Status
firefox28 --- wontfix
firefox29 --- wontfix
firefox30 --- wontfix
firefox31 --- fixed
firefox32 --- fixed
firefox33 --- fixed
firefox-esr24 --- unaffected
firefox-esr31 --- unaffected
b2g-v1.2 --- wontfix
b2g-v1.3 --- wontfix
b2g-v1.3T --- wontfix
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: mayhemer, Assigned: janjongboom)

Details

(Keywords: intermittent-failure)

Attachments

(4 files, 1 obsolete file)

This test only fails when the new HTTP cache is turned on.
Yuan, do you have any theory what this could be?  If you can help investigate, it would be of great help for us.  The new cache should be enabled by then end of February (some 8 weeks from now).

Thanks.
Flags: needinfo?(xyuan)
I've no idea why this could be. I'll take a look soon. The way to reproduce this bug is to apply the patch of bug 913806 and then run the mochitest?
Assignee: nobody → xyuan
Status: NEW → ASSIGNED
Flags: needinfo?(xyuan)
(In reply to Yuan Xulei [:yxl] from comment #9)
> The way to reproduce
> this bug is to apply the patch of bug 913806 and then run the mochitest?

Thanks.  Yes, just please do a full build (not clobbered, but of the whole tree).  Other way is to just early return true from CacheObserver::UseNewCache (http://mxr.mozilla.org/mozilla-central/source/netwerk/cache2/CacheObserver.cpp#79), then just build binaries will do the job.
Blocks: cache2tests
No longer blocks: cache2enable
Summary: HTTP cache v2: Intermittent test_bug944397.html | Failed to generate keyboard input → HTTP cache v2: Intermittent (osx10.6 opt m3) test_bug944397.html | Failed to generate keyboard input
Yuan, how does it look?  

We want to turn cache2 experimentally on m-c soon (tomorrow or after tomorrow) and this test will have to be disabled to keep the tree green.  If you can fix it before that it would be great.
Yuan, what is the progress?  Please see comment 23.
Flags: needinfo?(xyuan)
It is quite hard for me to reproduce the intermittent failure.
The failure seems not be caused by the new cache. From the TBPL robot log,
it fails without the new cache as well.

So maybe we can remove the dependency of the bug from the new cache.
Flags: needinfo?(xyuan)
I enabled the new cache on the latest mc and tested on ubuntu 12.04 of my own computer for 10 times. I didn't reproduce it.
Attached patch v1 patch (obsolete) — — Splinter Review
It seems that sometimes the input field of file_test_app.html doesn't get focused correctly and that results in the intermittent test failure. So the 
patch refocuses the input field to avoid such failure.

https://tbpl.mozilla.org/?tree=Try&rev=fca6deb00dbf
Attachment #8364926 - Flags: review?(fabrice)
(In reply to Yuan Xulei [:yxl] (PTO from Jan 25 to Feb 9) from comment #26)
> It is quite hard for me to reproduce the intermittent failure.
> The failure seems not be caused by the new cache. From the TBPL robot log,
> it fails without the new cache as well.
> 
> So maybe we can remove the dependency of the bug from the new cache.

Thanks, I somehow have missed the tbpl reports other then mine.
No longer blocks: cache2tests
Summary: HTTP cache v2: Intermittent (osx10.6 opt m3) test_bug944397.html | Failed to generate keyboard input → Intermittent (osx10.6 opt m3) test_bug944397.html | Failed to generate keyboard input
Comment on attachment 8364926 [details] [diff] [review]
v1 patch

Review of attachment 8364926 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/inputmethod/mochitest/test_bug944397.html
@@ +25,5 @@
>    input.oninput = function() {
>      sendAsyncMessage('test:InputMethod:oninput', {});
>    };
> +
> +  /* 

nit: trailing whitespace.
Attachment #8364926 - Flags: review?(fabrice) → review+
Attached patch Fix nits — — Splinter Review
Attachment #8364926 - Attachment is obsolete: true
Attachment #8369870 - Flags: review+
Summary: Intermittent (osx10.6 opt m3) test_bug944397.html | Failed to generate keyboard input → Intermittent test_bug944397.html | Failed to generate keyboard input
https://hg.mozilla.org/mozilla-central/rev/3876aee7eb39
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I guess that didn't work :(
Flags: needinfo?(xyuan)
Target Milestone: mozilla30 → ---
Yes, it didn't work :-(
Need further investigation about the cause.
Flags: needinfo?(xyuan)
I guess we're using this for the frequent timeouts as well.
Flags: needinfo?(xyuan)
Summary: Intermittent test_bug944397.html | Failed to generate keyboard input → Intermittent test_bug944397.html | Failed to generate keyboard input or Test timed out or application timed out after 330.0 seconds with no output
Yes, but not the timeout problem. The timeout is large enough. Maybe the patch of Bug 971651 will fix this bug. See https://bugzilla.mozilla.org/show_bug.cgi?id=971651#c14 for details.

Let's wait until Bug 971651 fixed :-)
Flags: needinfo?(xyuan)
It seems this bug has been fixed. Let's wait one more day for confirmation.
This bug is fixed by Bug 971651 of the commit:
https://hg.mozilla.org/mozilla-central/rev/4665cf1d88d9#l2.12

More specifically, the following line:

     2.1 --- a/dom/inputmethod/MozKeyboard.js
     2.2 +++ b/dom/inputmethod/MozKeyboard.js
     2.3 @@ -324,16 +324,17 @@ MozInputMethod.prototype = {
     2.4      Services.obs.addObserver(this, "inner-window-destroyed", false);
     2.5      cpmm.addMessageListener('Keyboard:FocusChange', this);
     2.6      cpmm.addMessageListener('Keyboard:SelectionChange', this);
     2.7      cpmm.addMessageListener('Keyboard:GetContext:Result:OK', this);
     2.8      cpmm.addMessageListener('Keyboard:LayoutsChange', this);
     2.9    },
    2.10  
    2.11    uninit: function mozInputMethodUninit() {
    2.12 +    this.setActive(false);
    2.13      Services.obs.removeObserver(this, "inner-window-destroyed");
    2.14      cpmm.removeMessageListener('Keyboard:FocusChange', this);
    2.15      cpmm.removeMessageListener('Keyboard:SelectionChange', this);
    2.16      cpmm.removeMessageListener('Keyboard:GetContext:Result:OK', this);
    2.17      cpmm.removeMessageListener('Keyboard:LayoutsChange', this);
    2.18  
    2.19      this._window = null;
    2.20      this._mgmt = null;
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Any updates here, yxl? :)
Flags: needinfo?(xyuan)
I'm still not sure what is the underlying cause, but I'll make a tentative patch to see if we can solve the issue.
Flags: needinfo?(xyuan)
Attached patch part2 fix SetTimeout — — Splinter Review
As I cannot reproduce this bug locally, so I made the test to output more logs to help identify the issue. Also I fixed a possible issue that we may clear a timeout before it is created.
Attachment #8390281 - Flags: review?(fabrice)
Comment on attachment 8390281 [details] [diff] [review]
part2 fix SetTimeout

Review of attachment 8390281 [details] [diff] [review]:
-----------------------------------------------------------------

let's see if that helps...
Attachment #8390281 - Flags: review?(fabrice) → review+
Yes, setTimeout usage is always discouraged in tests for exactly the reasons you're running into :)

https://hg.mozilla.org/integration/b2g-inbound/rev/ca4277d6971f
Keywords: checkin-needed
I'm having a hard time understanding the logic of this test.  We seem to try to focus an input element in appFrameScript every 500ms, we set keyboard.src after pushing the input permission without waiting for the load event to fire, and we seem to be setting a 20s timeout to detect a test failure.

None of these should be needed if you wait for the actual load and focus events.  Also, you should use waitForFocus to ensure that the window you're relying on has the focus, otherwise things such as input.focus(); may not end up working correctly.

If you need help with the above comments please ping me Yuan, I'd be happy to explain further.
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #185)
> I'm having a hard time understanding the logic of this test.  We seem to try
> to focus an input element in appFrameScript every 500ms, we set keyboard.src
> after pushing the input permission without waiting for the load event to
> fire, and we seem to be setting a 20s timeout to detect a test failure.
> 
> None of these should be needed if you wait for the actual load and focus
> events.  Also, you should use waitForFocus to ensure that the window you're
> relying on has the focus, otherwise things such as input.focus(); may not
> end up working correctly.
> 
> If you need help with the above comments please ping me Yuan, I'd be happy
> to explain further.

Ehsan, thanks. I think I can make some improvement based on your comments.
Summary: Intermittent test_bug944397.html | Failed to generate keyboard input or Test timed out or application timed out after 330.0 seconds with no output → Intermittent test_bug944397.html | Failed to generate keyboard input or Test timed out or application timed out after 330.0 seconds with no output or [SimpleTest.finish()] this test already called finish!
Any luck here, Yuan?
Flags: needinfo?(xyuan)
The patch of Bug 987549 may fix this. I guess the cause of this bug is the same as:
https://bugzilla.mozilla.org/show_bug.cgi?id=987549#c12
Flags: needinfo?(xyuan)
I have a patch in bug 983490 that removes all setTimeout and setInterval in the test. I hope that can fix this bug too!
Depends on: 983490
(In reply to Yuan Xulei [:yxl] from comment #234)
> The patch of Bug 987549 may fix this. I guess the cause of this bug is the
> same as:
> https://bugzilla.mozilla.org/show_bug.cgi?id=987549#c12

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #358)
> I have a patch in bug 983490 that removes all setTimeout and setInterval in
> the test. I hope that can fix this bug too!

The above fixes sadly haven't helped - could you take another look at this test? (It's quite high up on OrangeFactor). Thanks!
Flags: needinfo?(xyuan)
(In reply to Ed Morley [:edmorley UTC+0] from comment #404)
> (In reply to Yuan Xulei [:yxl] from comment #234)
> > The patch of Bug 987549 may fix this. I guess the cause of this bug is the
> > same as:
> > https://bugzilla.mozilla.org/show_bug.cgi?id=987549#c12
> 
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from
> comment #358)
> > I have a patch in bug 983490 that removes all setTimeout and setInterval in
> > the test. I hope that can fix this bug too!
> 
> The above fixes sadly haven't helped - could you take another look at this
> test? (It's quite high up on OrangeFactor). Thanks!

Yes, I'll remove these setTimeout and give a new patch.
Flags: needinfo?(xyuan)
To summarize - these tests have been skipped on B2G emulator builds for over 3 months now. They've been failing intermittently on basically every other platform since. Due to the ongoing issues and the clear lack of priority in fixing these, I think it's time to consider disabling these tests across the board.

Per the Test Disabling Policy (https://wiki.mozilla.org/Sheriffing/Test_Disabling_Policy), I'm needinfo'ing yxl to start. In 2 business days, I will needinfo a DOM module owner if there is no progress towards fixing these tests. 2 business days after that, I will disable the tests.
Flags: needinfo?(xyuan)
Assignee: xyuan → janjongboom
So I took the liberty to rewrite these tests in a way they won't cause intermittents anymore. I split the tests for bug 944397 and bug 953044, and made sure that everything happens in order in the tests for 944397. I think this is pretty sane code.

Try is running @ https://tbpl.mozilla.org/?tree=Try&rev=e6b595760b74
Attachment #8445860 - Flags: review?(xyuan)
Flags: needinfo?(xyuan)
Oh yeah, an edge case that was not covered and I think a big part of the problems is that when mozInputMethod.inputcontext is not null (which is pretty often the case I think) when the inputmethod file is loaded it would stall forever. So that's fixed.
New try against latest mc tip and with emulator enabled: https://tbpl.mozilla.org/?tree=Try&rev=e3aa21c52e25
Attached patch Experiment — — Splinter Review
Little experiment to see if we can make intermittents go away... Try @ https://tbpl.mozilla.org/?tree=Try&rev=210dbb29e240
Comment on attachment 8445860 [details] [diff] [review]
Patch v3 - Re-work this mochitest

Review of attachment 8445860 [details] [diff] [review]:
-----------------------------------------------------------------

Jan, thank:)
Attachment #8445860 - Flags: review?(xyuan) → review+
We cannot re-enable on emulator yet, but should fix bug 944397 intermittent on other platforms (it's the  Patch v3 - Re-work this mochitest  patch)
Keywords: checkin-needed
Can this be closed?
Flags: needinfo?(ryanvm)
Yes, let's create a new bug for fixing and re-enabling these tests on B2G.
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Flags: needinfo?(ryanvm)
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: