Several homescreen dragdrop tests fail in TBPL

RESOLVED DUPLICATE of bug 946728

Status

Firefox OS
Gaia::Homescreen
RESOLVED DUPLICATE of bug 946728
5 years ago
4 years ago

People

(Reporter: jgriffin, Assigned: julienw)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
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)
(Reporter)

Comment 1

5 years ago
Created attachment 766963 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10588

Pointer to Github pull-request
(Reporter)

Updated

5 years ago
Attachment #766963 - Flags: review?(mdas)
(Assignee)

Comment 2

5 years ago
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?
(Assignee)

Comment 5

5 years ago
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?
(Assignee)

Comment 8

5 years ago
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
I ran all test with http://ftp.mozilla.org/pub/mozilla.org/b2g/nightly/latest-mozilla-central/b2g-24.0a1.multi.mac64.dmg and 3467 tests completed and OK. Any idea?
(Assignee)

Comment 11

5 years ago
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
(Assignee)

Comment 13

5 years ago
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.
Created attachment 767153 [details]
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)
(Assignee)

Comment 15

5 years ago
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+
(Reporter)

Comment 16

5 years ago
> 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.
(Reporter)

Comment 17

5 years ago
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?
(Assignee)

Comment 18

5 years ago
Jonathan, yep please, at least let's try the new code once it lands.
(Reporter)

Comment 19

5 years ago
Ok, will re-enable in this bug once the fix lands.

Updated

5 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
please re-enable and cross fingers :)
Flags: needinfo?(jgriffin)
(Assignee)

Comment 22

5 years ago
(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 !
(Reporter)

Comment 23

5 years ago
Created attachment 767853 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10642

Pointer to Github pull-request
(Reporter)

Updated

5 years ago
Attachment #767853 - Flags: review?(crdlc)
(Assignee)

Comment 24

5 years ago
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+
(Reporter)

Comment 25

5 years ago
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!
Finally, was it fixed?

https://tbpl.mozilla.org/php/getParsedLog.php?id=24576476&tree=Cedar
(Reporter)

Comment 28

5 years ago
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.
(Reporter)

Comment 29

5 years ago
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 → ---
(Assignee)

Comment 30

5 years ago
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 | ' +
(Reporter)

Comment 32

4 years ago
Disabled in https://github.com/mozilla-b2g/gaia/commit/8a52616318758661c6c75bded3fc33881bd665fb so we can get a green run in TBPL
(Assignee)

Comment 33

4 years ago
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
(Assignee)

Comment 35

4 years ago
it was fixed :)
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 946728
You need to log in before you can comment on or make changes to this bug.