Closed
Bug 1044984
Opened 10 years ago
Closed 9 years ago
Intermittent worker_test.js | Latin en_us worker Worker should throw if language doesnt exist | timeout of 10000ms exceeded
Categories
(Firefox OS Graveyard :: Gaia::Keyboard, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: cbook, Assigned: timdream)
References
()
Details
(Keywords: intermittent-failure)
Attachments
(4 files)
b2g_ubuntu64_vm b2g-inbound opt test gaia-unit on 2014-07-27 10:34:03 PDT for push 8b48f1036652
slave: tst-linux64-spot-563
https://tbpl.mozilla.org/php/getParsedLog.php?id=44681621&tree=B2g-Inbound
gaia-unit-tests TEST-UNEXPECTED-FAIL | keyboard/test/unit/worker_test.js | Latin en_us worker Worker should throw if language doesnt exist | timeout of 10000ms exceeded
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
All of the failures from today were caused by bug 1038988, which has been reverted.
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 14•10 years ago
|
||
Hi Jan,
My patch in bug 1038988 turns this intermittent failure into a permanent failure. The weird thing is, that patch is a css-only change, and this test is for workers. Any ideas why this test would be effected by such a change? Also, would you mind if I disabled this test while we investigate, since bug 1038988 is a 2.0 CAF blocker and won't affect keyboard workers.
Note: this test runs fine for me locally with my patch both in b2g-desktop and firefox nightly.
Flags: needinfo?(janjongboom)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Updated•10 years ago
|
Assignee: nobody → janjongboom
Flags: needinfo?(janjongboom)
Comment 17•10 years ago
|
||
Comment 18•10 years ago
|
||
Ran tests twice, no failures anymore.
Did some things:
1. The test to see if the file exist is wrong, as we get a byte array back nowadays. It's just not valid dict format so it'll fail later. Status codes are now implemented correctly so rely on that.
2. Wait until dict is loaded before moving from suiteSetup, removing timing issue
3. Make the XHR call async in the worker.
Attachment #8465373 -
Flags: review?(rlu)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 20•10 years ago
|
||
In the meantime, I'm disabling this test because bug 1038988 is a CAF blocker that blocks a CAF blocker, and I want to land today to make sure it doesn't break anything else by deadline date of Aug 1 (tomorrow). You'll have to update your PR to remove the early return. Sorry about that guys.
master: https://github.com/mozilla-b2g/gaia/commit/031b8cc197c92219285bebee068b90ce85d38cde
v2.0: https://github.com/mozilla-b2g/gaia/commit/1d1a47a1135d8ce44db9b88db4d2ea6f454cf0a8
Comment 21•10 years ago
|
||
Comment on attachment 8465373 [details] [review]
Patch
Jan, thanks for the patch.
However, since we've also change the xhr from sync to async, I could see a regression that the suggestion won't be updated when you switch to a new language that is just loaded.
STR
===
1. Kill the keyboard and make sure all dict have been cleaned up.
2. type some word to show the suggestion bar.
3. Press [En] to switch to another language => the suggestion won't change.
So, I'll r- this first.
Attachment #8465373 -
Flags: review?(rlu) → review-
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 23•10 years ago
|
||
Comment on attachment 8465373 [details] [review]
Patch
Oh, wow, didn't see that. Relying on sync XHR to keep code working looks like a bug to me though, but hey.
I made the code sync again, seems to work on device. Let's see what Try thinks. https://tbpl.mozilla.org/?rev=9d4212952c91b256aa34f6def111847191b0452b&tree=Gaia-Try
Attachment #8465373 -
Flags: review- → review?(rlu)
Comment 24•10 years ago
|
||
(In reply to Michael Henretty [:mhenretty] from comment #20)
> In the meantime, I'm disabling this test because bug 1038988 is a CAF
> blocker that blocks a CAF blocker, and I want to land today to make sure it
> doesn't break anything else by deadline date of Aug 1 (tomorrow). You'll
> have to update your PR to remove the early return. Sorry about that guys.
Sure, but you owe me a beer when I'm in SF next month.
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 29•10 years ago
|
||
(In reply to Jan Jongboom [:janjongboom] (Telenor) from comment #24)
> (In reply to Michael Henretty [:mhenretty] from comment #20)
> > In the meantime, I'm disabling this test because bug 1038988 is a CAF
> > blocker that blocks a CAF blocker, and I want to land today to make sure it
> > doesn't break anything else by deadline date of Aug 1 (tomorrow). You'll
> > have to update your PR to remove the early return. Sorry about that guys.
>
> Sure, but you owe me a beer when I'm in SF next month.
Deal
Comment 30•10 years ago
|
||
Comment on attachment 8465373 [details] [review]
Patch
r=me.
Thanks.
Attachment #8465373 -
Flags: review?(rlu) → review+
Comment 31•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 32•10 years ago
|
||
We need to remove the return statement at the top as well...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 33•10 years ago
|
||
Attachment #8467739 -
Flags: review?(rlu)
Comment 34•10 years ago
|
||
Comment on attachment 8467739 [details] [diff] [review]
Re-enable the test
Oh right.
Thanks.
Attachment #8467739 -
Flags: review?(rlu) → review+
Comment 35•10 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Updated•10 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 46•10 years ago
|
||
Jan, I think this test is failing often enough that we should disable it until we re-investigate what is going on. What do you think?
Flags: needinfo?(janjongboom)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 53•10 years ago
|
||
Yep ##&*$.
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 58•10 years ago
|
||
Julien, do we have a manifest for disabling unit tests in tbpl?
Flags: needinfo?(felash)
Comment 59•10 years ago
|
||
I think James helped me find the tbpl manifest. Submitted this PR to test it.
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 61•10 years ago
|
||
Yup, that did it. I disabled the test for now.
master: https://github.com/mozilla-b2g/gaia/commit/31cd87328f5fc76c60097d25354fe84bd6456542
Flags: needinfo?(felash)
Comment 62•10 years ago
|
||
Does it fail on Travis too?
Updated•10 years ago
|
Flags: needinfo?(janjongboom)
Updated•10 years ago
|
Keywords: leave-open
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 65•10 years ago
|
||
Kevin, this test should be disabled according to comment 61. How comes it is run by TC?
Flags: needinfo?(kgrandon)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 67•10 years ago
|
||
Moreoever the error is not the same as this bug's summary, no timeout is involved here.
Comment 68•10 years ago
|
||
After some quick investigation it appears that this test *was* disabled, then the file was moved in bug 972210 without updating the disabled test location. After that it was then removed from the disabled.json file here, probably because the location was wrong and it wasn't failing too much: https://github.com/mozilla-b2g/gaia/commit/4218ac24394a498e3a132a5ba18fcc38bff63545
In any case, the intermittent rate doesn't seem terrible (once per day so far). I don't mind if we disable it again or wait to monitor the failing rate over the next few days as we're still rolling out taskcluster updates.
Updated•10 years ago
|
Flags: needinfo?(kgrandon)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 107•9 years ago
|
||
latest is really bug 1165469 -- but the others are not.
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 189•9 years ago
|
||
Failure appears to have morphed, but still pretty regular.
Flags: needinfo?(kgrandon)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 199•9 years ago
|
||
This has an assignee already, so asking Tim or Jan to look into it (otherwise we could consider disabling the test).
Flags: needinfo?(timdream)
Flags: needinfo?(kgrandon)
Flags: needinfo?(janjongboom)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 207•9 years ago
|
||
I tried to analyze the code of this test, and I don't understand several things.
1. Why is it so complicated to get messages from the worker?
=> https://github.com/mozilla-b2g/gaia/blob/a47e0c58dd1340a17be7cc96ccd90fcad15a922b/apps/keyboard/test/unit/imes/latin/worker_test.js#L454-L469
2. The test says "should throw if language doesn't exist" but the truth is that it really throws because we don't pass enough arguments and "buffer" is undefined here:
=> https://github.com/mozilla-b2g/gaia/blob/a47e0c58dd1340a17be7cc96ccd90fcad15a922b/apps/keyboard/js/imes/latin/predictions.js#L109
Hey Tim, Jan, do you know why 1 has been done this way, and don't you think we should just remove the test? Actually I don't see any code in worker.js that checks whether the language is known. Maybe it's been done somewhere else ?
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 209•9 years ago
|
||
We should disable that again until there is someone available to work on the dictionary again. I will make sure that happens before we do feature work on it.
Flags: needinfo?(timdream)
Assignee | ||
Comment 210•9 years ago
|
||
Keeping this on my backlog too ... look like the tests was not updated when I remove XHR from worker.js, according to comment 207.
Flags: needinfo?(timdream)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 213•9 years ago
|
||
Assignee | ||
Comment 214•9 years ago
|
||
Comment on attachment 8637111 [details] [review]
[gaia] timdream:keyboard-worker-test > mozilla-b2g:master
Julien, assuming you have read the file already and since there is no other keyboard developer left, would you do the honor and review this?
I realized we reuse the worker instance in each of the test, probably an attempt to speed things up, so I changed it -- should get rid of the instability. Also for the test you talked about, I reworded it so we know it throw because the dictionary is not set.
Flags: needinfo?(timdream)
Attachment #8637111 -
Flags: review?(felash)
Assignee | ||
Updated•9 years ago
|
Assignee: janjongboom → timdream
Status: REOPENED → ASSIGNED
Comment 215•9 years ago
|
||
Comment on attachment 8637111 [details] [review]
[gaia] timdream:keyboard-worker-test > mozilla-b2g:master
r=me with nits
Most nits are optional (we can do them later) but I'd like to insist on the comment to remove the listener in onWorkerMessage: https://github.com/mozilla-b2g/gaia/pull/31070/files#r35429136
I retriggered the test several times in treeherder and it didn't fail once. So I think it's safe to assume the intermittency is fixed, and that's what is important here. All the other nits can be done later :)
Attachment #8637111 -
Flags: review?(felash) → review+
Updated•9 years ago
|
Flags: needinfo?(janjongboom)
Updated•9 years ago
|
Keywords: leave-open
Assignee | ||
Comment 216•9 years ago
|
||
I added a simple |worker.onmessage = null| for addressing your review comment. I agree with you switching to Promise is better for async tests, but not today.
master: https://github.com/mozilla-b2g/gaia/commit/b10968e87933365014f9b2b890301c0596609db3
Status: ASSIGNED → RESOLVED
Closed: 10 years ago → 9 years ago
Resolution: --- → FIXED
Comment hidden (Legacy TBPL/Treeherder Robot) |
You need to log in
before you can comment on or make changes to this bug.
Description
•