Closed Bug 886602 Opened 11 years ago Closed 10 years ago

Several homescreen dragdrop tests fail in TBPL

Categories

(Firefox OS Graveyard :: Gaia::Homescreen, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::Homescreen
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED DUPLICATE of bug 946728

People

(Reporter: jgriffin, Assigned: julienw)

References

Details

Attachments

(3 files)

Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10588
358 bytes, text/html
mdas
: review+
Details
Patch v1
190 bytes, text/html
jmcf
: review+
Details
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10642
358 bytes, text/html
julienw
: review+
Details 
https://tbpl.mozilla.org/php/getParsedLog.php?id=24525182&tree=Cedar#error10

11:37:01     INFO -  gaia-unit-tests TEST-PASS  | dragdrop.js > The dock has been initialized correctly | Dock [app5, app6] >
11:37:01     INFO -  gaia-unit-tests TEST-UNEXPECTED-FAIL | dragdrop.js > Dragging app1 to app2 | Page [app2, app1, app3, app4] | Dock [app5, app6] >  | Error: expected {} to equal {} (http://test-agent.gaiamobile.org:8080/common/test/helper.js?time=1372099007391:30)
11:37:01     INFO -  gaia-unit-tests INFO       | stack trace:
11:37:01     INFO -      Error: Error: expected {} to equal {} (http://test-agent.gaiamobile.org:8080/common/test/helper.js?time=1372099007391:30)
11:37:01     INFO -          at onerror (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4959)
11:37:01     INFO -  gaia-unit-tests TEST-UNEXPECTED-FAIL | dragdrop.js > Dragging app1 to app3 | Page [app2, app3, app1, app4] | Dock [app5, app6] >  | Error: expected {} to equal {} (http://test-agent.gaiamobile.org:8080/common/test/helper.js?time=1372099007391:30)
11:37:01     INFO -  gaia-unit-tests INFO       | stack trace:
11:37:01     INFO -      Error: Error: expected {} to equal {} (http://test-agent.gaiamobile.org:8080/common/test/helper.js?time=1372099007391:30)
11:37:01     INFO -          at onerror (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4959)
11:37:02     INFO -  gaia-unit-tests TEST-UNEXPECTED-FAIL | dragdrop.js > Dragging app1 to app4 | Page [app2, app3, app4, app1] | Dock [app5, app6] >  | Error: expected {} to equal {} (http://test-agent.gaiamobile.org:8080/common/test/helper.js?time=1372099007391:30)
11:37:02     INFO -  gaia-unit-tests INFO       | stack trace:
11:37:02     INFO -      Error: Error: expected {} to equal {} (http://test-agent.gaiamobile.org:8080/common/test/helper.js?time=1372099007391:30)
11:37:02     INFO -          at onerror (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4959)
11:37:02     INFO -  gaia-unit-tests TEST-PASS  | dragdrop.js > Dragging app1 to app2 | Page [app1, app2, app3, app4] | Dock [app5, app6] >
11:37:02     INFO -  gaia-unit-tests TEST-UNEXPECTED-FAIL | dragdrop.js > Dragging app1 to dock (from left) | Page [app2, app3, app4] | Dock [app1, app5, app6] >  | Error: expected {} to equal {} (http://test-agent.gaiamobile.org:8080/common/test/helper.js?time=1372099007391:30)
11:37:02     INFO -  gaia-unit-tests INFO       | stack trace:
11:37:02     INFO -      Error: Error: expected {} to equal {} (http://test-agent.gaiamobile.org:8080/common/test/helper.js?time=1372099007391:30)
11:37:02     INFO -          at onerror (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4959)
11:37:02     INFO -  gaia-unit-tests TEST-UNEXPECTED-FAIL | dragdrop.js > Dragging app1 to app5 | Page [app2, app3, app4] | Dock [app5, app1, app6] >  | Error: expected {} to equal {} (http://test-agent.gaiamobile.org:8080/common/test/helper.js?time=1372099007391:30)
11:37:02     INFO -  gaia-unit-tests INFO       | stack trace:
11:37:02     INFO -      Error: Error: expected {} to equal {} (http://test-agent.gaiamobile.org:8080/common/test/helper.js?time=1372099007391:30)
11:37:02     INFO -          at onerror (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4959)
11:37:02     INFO -  gaia-unit-tests TEST-UNEXPECTED-FAIL | dragdrop.js > Dragging app1 to app6 | Page [app2, app3, app4] | Dock [app5, app6, app1] >  | Error: expected {} to equal {} (http://test-agent.gaiamobile.org:8080/common/test/helper.js?time=1372099007391:30)
11:37:02     INFO -  gaia-unit-tests INFO       | stack trace:
11:37:02     INFO -      Error: Error: expected {} to equal {} (http://test-agent.gaiamobile.org:8080/common/test/helper.js?time=1372099007391:30)
11:37:02     INFO -          at onerror (http://test-agent.gaiamobile.org:8080/common/vendor/mocha/mocha.js:4959)
Attachment #766963 - Flags: review?(mdas)
Note that I'm quite afraid that this test failure denotes that this may not work as expected on moz central ?

Cristian, do you have an idea why this could not work on moz central ?
Flags: needinfo?(crdlc)
No idea and with that info less :(
Flags: needinfo?(crdlc)
Did it work on the past? Or Did it never work?
I think it never worked on TBPL, but it works for me on Firefox Nightly.

In TBPL the tests are run in b2g desktop so there must be something different...
ok, I gonna try to test it on B2G desktop, thanks
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
I've just downloaded B2G-Desktop http://ftp.mozilla.org/pub/mozilla.org/b2g/nightly/latest-mozilla-central/b2g-23.0a1.multi.mac64.dmg. Then, I ran this one with the debug-profile and ran all tests. It was stopped in this trace "Keyboard layout changed to newLanguage". If I only run dragdrop_test.js from the test-agent app, it works fine with errors. Any suggestion?
You should use the v24 one, which is the current moz central. The one you used is about 1.5 months old ;)
jujujuju I started downloading v24 15 minutes ago just in case
Not really.

Jonathan, what could be different ?
If I would make a patch [1] only for testing something, could it be tested in TBPL? I guess that I've gotten the problem but I am not sure because I cannot reproduce the problem. I guess that 
the animate method is invoked after this [2], so while assertions are performed the DOM is not ready. The patch increases the timeout in order to know if it is the problem

[1]

index 1d571a7..8167f1a 100644
--- a/apps/homescreen/test/unit/dragdrop_test.js
+++ b/apps/homescreen/test/unit/dragdrop_test.js
@@ -81,7 +81,7 @@ suite('dragdrop.js >', function() {
             doEnd(node, x, y, done);
         });
       }
-    }, 50);
+    }, 300);
   }
 
   function start(target, x, y) {

[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/test/unit/dragdrop_test.js#L76
Maybe to reproduce you need to test on a very low-end machine. In the past I could reproduce such problems with an Atom netbook for example (but since I'm in Taipei I can't try now).

Can you try to use sinon fake timers so that we can have a synchronous test instead ?

see https://github.com/mozilla-b2g/gaia/blob/master/apps/system/test/unit/update_manager_test.js#L400 and https://github.com/mozilla-b2g/gaia/blob/master/apps/system/test/unit/update_manager_test.js#L444 for some examples, and http://sinonjs.org/docs/#clock for the docs. Basically you shouldn't need to restore the clock because it's done automatically at the end of the test, but you might need it if your teardowns depend on a working setTimeout.

Converting the update manager tests to using sinon's fake timers made them very reliable.
Attached file Patch v1 — 
Theoretically drag&drop tests should work fine in everywhere with this patch. I've fixed some race conditions and deleted asynchronous calls with invented timeouts like 50. Right now, start event, then move event and wait for page ready event, after that, release finger and wait for dragend event. Well, I need to say that the previous code was working fine in my laptop so I am not completely sure that it will fix the bug although it should be the correct way! Thanks
Attachment #767153 - Flags: review?(jmcf)
too bad you didn't try to use sinon ;) but at least it still works for me, it works for travis, so I hope it will work for tbpl too !
Attachment #766963 - Flags: review?(mdas) → review+
> If I would make a patch [1] only for testing something, could it be tested in 
> TBPL?

Sadly, not.  We don't have a gaia-try yet, although we have on planned in bug 868605.

Errors that don't reproduce locally on the same build are likely to be timing-related; the test slaves used by TBPL are not very fast.
So, I just disabled this in https://github.com/mozilla-b2g/gaia/commit/cfd1957e3d55225d173b6832e81dec5940f7445e, but it looks like maybe I pulled the trigger too soon.  Would you like these re-enabled?
Jonathan, yep please, at least let's try the new code once it lands.
Ok, will re-enable in this bug once the fix lands.
Attachment #767153 - Flags: review?(jmcf) → review+
https://github.com/mozilla-b2g/gaia/commit/11115a5cfe4c6574d2a0f7ad37f85288596895a9

The only way to know if the bug is fixed or not is landing this one. We will need to re-open this bug if the bug still would be reproducible
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
please re-enable and cross fingers :)
Flags: needinfo?(jgriffin)
(In reply to Cristian Rodriguez de la Cruz (:crdlc) from comment #20)
> 
> The only way to know if the bug is fixed or not is landing this one. 

or find a 10-year old computer with Linux ;) I'm sure you have one somewhere !
Attachment #767853 - Flags: review?(crdlc)
Comment on attachment 767853 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10642

r=me

please go for it :)
Attachment #767853 - Flags: review?(crdlc) → review+
Enabled in https://github.com/mozilla-b2g/gaia/commit/dec64d9634d9cb2abe63ae33e8ad7efbfbbeca7f.

Will kick off a new run on cedar now.
Flags: needinfo?(jgriffin)
> or find a 10-year old computer with Linux ;) I'm sure you have one somewhere !

I don't have computers my friend at home, waiting results. Thanks for the help!
Actually, I need to kick off a new run.  I forgot that the gaia change would have to propagate from birch to m-c (in the form of gaia.json) before I could merge to cedar.  I'll start a new run shortly.
There are different errors after this patch landed:

12:56:26     INFO -  gaia-unit-tests TEST-UNEXPECTED-FAIL | dragdrop.js > Dragging app1 to app2 | Page [app2, app1, app3, app4] | Dock [app5, app6] >  | timeout of 2000ms exceeded
12:56:28     INFO -  gaia-unit-tests TEST-UNEXPECTED-FAIL | dragdrop.js > Dragging app1 to app3 | Page [app2, app3, app1, app4] | Dock [app5, app6] >  | timeout of 2000ms exceeded
12:56:30     INFO -  gaia-unit-tests TEST-UNEXPECTED-FAIL | dragdrop.js > Dragging app1 to app4 | Page [app2, app3, app4, app1] | Dock [app5, app6] >  | timeout of 2000ms exceeded
12:56:32     INFO -  gaia-unit-tests TEST-UNEXPECTED-FAIL | dragdrop.js > Dragging app1 to app2 | Page [app1, app2, app3, app4] | Dock [app5, app6] >  | timeout of 2000ms exceeded
12:56:34     INFO -  gaia-unit-tests TEST-UNEXPECTED-FAIL | dragdrop.js > Dragging app1 to dock | Page [app2, app3, app4] | Dock [app1, app5, app6] >  | timeout of 2000ms exceeded
12:56:37     INFO -  gaia-unit-tests TEST-UNEXPECTED-FAIL | dragdrop.js > Dragging app1 to app5 | Page [app2, app3, app4] | Dock [app5, app1, app6] >  | timeout of 2000ms exceeded
12:56:39     INFO -  gaia-unit-tests TEST-UNEXPECTED-FAIL | dragdrop.js > Dragging app1 to app6 | Page [app2, app3, app4] | Dock [app5, app6, app1] >  | timeout of 2000ms exceeded
12:56:41     INFO -  gaia-unit-tests TEST-UNEXPECTED-FAIL | dragdrop.js > Dragging app1 to grid | Page [app1, app2, app3, app4] | Dock [app5, app6] >  | timeout of 2000ms exceeded

full log:

https://tbpl.mozilla.org/php/getParsedLog.php?id=24671057&tree=Cedar
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Cristian, I'll take this when I'll be in Paris next week so that I can use my shitty netbook to reproduce the timing issues.
Assignee: crdlc → felash
Hi Julien,

This patch reduces 300ms each test, it could be interesting.

index 3c842b9..0e4d154 100644
--- a/apps/homescreen/test/unit/dragdrop_test.js
+++ b/apps/homescreen/test/unit/dragdrop_test.js
@@ -16,7 +16,8 @@ mocksHelperForDragDrop.init();
 
 suite('dragdrop.js >', function() {
 
-  var wrapperNode, page, dock, realElementFromPoint, dragabbleIcon;
+  var wrapperNode, page, dock, realElementFromPoint, dragabbleIcon,
+      realDraggingTransition;
 
   function sendTouchEvent(type, node, coords) {
     var touch = document.createTouch(window, node, 1,
@@ -125,6 +126,9 @@ suite('dragdrop.js >', function() {
 
     dragabbleIcon =
                 document.querySelector('li[data-manifest-u-r-l="http://app1"]');
+
+    realDraggingTransition = DRAGGING_TRANSITION;
+    DRAGGING_TRANSITION = '-moz-transform 1ms';
   });
 
   suiteTeardown(function() {
@@ -132,6 +136,7 @@ suite('dragdrop.js >', function() {
     document.body.removeChild(wrapperNode);
 
     HTMLDocument.prototype.elementFromPoint = realElementFromPoint;
+    DRAGGING_TRANSITION = realDraggingTransition;
   });
 
   test('The page has been initialized correctly | ' +
Cristian looks decided to fix this in bug 946728 ;)
Depends on: 946728
I have some days off :( We have to wait for me or another person could do it too, no problem
it was fixed :)
Status: REOPENED → RESOLVED
Closed: 11 years ago → 10 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: