1161441 - [Gaia::UI Tests] test_gallery_flick.py failed

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

Description

[Shing Lyu [:shinglyu]]

*** Description
test_gallery_flick.py failed

*** Steps to Reproduce
Run test_gallery_flick.py failed

*** Expected Results
Test passes

*** Actual Results
The first flick works, then it stopped and timed-out

Error message:
TEST-UNEXPECTED-ERROR | test_gallery_flick.py TestGallery.test_gallery_full_screen_image_flicks | TimeoutException: TimeoutException: Timed out after 10.1 seconds


Traceback (most recent call last):
  File "/home/slyu/.virtualenvs/gaiauitest/local/lib/python2.7/site-packages/marionette_client-0.8.6-py2.7.egg/marionette/marionette_test.py", line 268, in run
    testMethod()
  File "/home/slyu/workspace/gaia/tests/python/gaia-ui-tests/gaiatest/tests/functional/gallery/test_gallery_flick.py", line 39, in test_gallery_full_screen_image_flicks
    image.flick_to_next_image()
  File "/home/slyu/workspace/gaia/tests/python/gaia-ui-tests/gaiatest/apps/gallery/regions/fullscreen_image.py", line 57, in flick_to_next_image
    self._flick_to_image('next')
  File "/home/slyu/workspace/gaia/tests/python/gaia-ui-tests/gaiatest/apps/gallery/regions/fullscreen_image.py", line 71, in _flick_to_image
    lambda m: abs(image.location['x']) >= image.size['width'])
  File "/home/slyu/.virtualenvs/gaiauitest/local/lib/python2.7/site-packages/marionette_client-0.8.6-py2.7.egg/marionette/wait.py", line 143, in until
    cause=last_exc)
TEST-INFO took 33290ms


*** Other Notes


*** Reproduction Frequency
3/3

*** Build
Device firmware (base) 	L1TC100118D0
Device firmware (date) 	04 May 2015 16:14:48
Device firmware (incremental) 	eng.cltbld.20150504.041436
Device firmware (release) 	4.4.2
Device identifier 	flame
Gaia date 	02 May 2015 02:23:46
Gaia revision 	8d14361337e6
Gecko build 	20150504002502
Gecko revision 	cb7cb6597c91
Gecko version 	37.0

Comment 1

[Martijn Wargers (dead)]

I can also reproduce this on mozilla-central on my Flame device. I can also manually reproduce this after the test has been run.
If I close the Gallery app and then reopen it, I can't reproduce it anymore. Also, after restarting, I can't reproduce.

QAnalyst, is someone from your team able to reproduce this, when running this automated test on mozilla-central?

Comment 2

[Martijn Wargers (dead)]

I'm wondering if this is a recent regression or if this is a regression in Marionette, perhaps.

Comment 3

[Martijn Wargers (dead)]

Ok, it works in a build with Build ID: 20150411160204, so I guess this is a regression in Gaia.

Comment 4

[Martijn Wargers (dead)]

Ok, this seems to have regressed between the 2015-04-29-01-02-05 and 2015-04-30-01-02-01 on the pvt builds of mozilla-central-flame-kk-eng.

Comment 5

[Martijn Wargers (dead)]

Could this somehow have been caused by bug 1159818? (new marionette-client)

Comment 6

[Martijn Wargers (dead)]

Build ID 20150429010205 has gecko revision https://hg.mozilla.org/mozilla-central/rev/4b9b12c248dc and gaia revision 6e35b0948c42a4398b8a5916015de167121683a1
Build ID 20150430010201 has gecko revision https://hg.mozilla.org/mozilla-central/rev/1ad65cbeb2f4 and gaia revision db8ea705c0fd1b1684807f5a8e837bb9a36a6f96

Comment 7

[Peter Bylenga [:PBylenga]]

Oliver re-opened bug 1149202 on Friday, I think this would be a dupe of that?

Comment 8

[Martijn Wargers (dead)]

Bug 1149202 already occured before this regressed. I'd rather not morph bug 1149202 into something that was caused by something else than what it was caused by originally.

Indeed, as Oliver mentioned in that other bug, I think this is a regression from bug 1158396 (it fits in the regression range and was also checked in on v2.2).

David, could you take a look at this? It's only reproducable with the automated test, but I don't see anything wrong with the test in itself, afaict:
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/functional/gallery/test_gallery_flick.py#39
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/apps/gallery/regions/fullscreen_image.py#68
http://mxr.mozilla.org/mozilla-central/source/testing/marionette/driver/marionette_driver/marionette.py#374
But maybe there is something wrong with the Marionette flick method that is being used?

Comment 9

[Martijn Wargers (dead)]

This is failing for me on b2g desktop too on the Mac, I assume it's passin on b2g desktop on Linux, though, because this supposedly running on treeherder, afaik: http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/functional/gallery/manifest.ini#14

Comment 10

[Martijn Wargers (dead)]

The way the flick is carried out by Marionette is somehow different than manually, because it can also be reproduced after a Marionette flick.

Comment 11

[Dave Hunt [:davehunt] [he/him] ⌚BST]

It's really not clear so far in this bug when this regressed. Please use the adhoc bitbar job in Jenkins to run the affected test(s) against the necessary range of builds to find a reliable regression range. We can then use this to help determine the cause.

Comment 12

[Martijn Wargers (dead)]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #6)
> Build ID 20150429010205 has gecko revision
> https://hg.mozilla.org/mozilla-central/rev/4b9b12c248dc and gaia revision
> 6e35b0948c42a4398b8a5916015de167121683a1
> Build ID 20150430010201 has gecko revision
> https://hg.mozilla.org/mozilla-central/rev/1ad65cbeb2f4 and gaia revision
> db8ea705c0fd1b1684807f5a8e837bb9a36a6f96

I rechecked this and I still get this same regression range.

Comment 13

[David Flanagan [:djf]]

To prevent a race, the Gallery app sets a `transitioning` flag when it is animating one image in and one image out.  It ignores things like user zoom gestures when this flag is set.

Bug 1158396 changed things in two ways.  First of all, it modified the app so that it ignores swipes while transitioning. It was a bug that it wasn't already doing that. So if you are doing two swipes in rapid succession, you'd see the first succeed, then the subsequent one would fail.  To fix this, you just need to wait a bit between swipes, I think.

If your test is already waiting between swipes, then the other change may have broken the tests. It used to be that the `transitioning` flag was cleared on a timer. But that turned out to be racy and I changed it to clear the flag on the transitionend event. If the marionette test framework is not delivering transitionend events, then that could be the source of the problem, too.

Sorry I broke this. I don't know if I just didn't look at the test results carefully before landing or what.

Is this enough information to fix the test? I know nothing at all about python marionette tests, so I'm hoping you can fix it, but let me know if you'd like my help.

Bug 1158396 has already been uplifted to 2.2, so I'd prefer to not back it out, but if you need to do that, I'll understand.

Comment 14

[GitHub Autolander Bot]

Attachment: [gaia] mwargers:1161441 > mozilla-b2g:master

Comment 15

[Martijn Wargers (dead)]

I triggered http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/824/ with no backout of the patch from bug 1158396.
And I triggered http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/825/ which has the backout of the patch from bug 1158396.

Comment 16

[Martijn Wargers (dead)]

(In reply to David Flanagan [:djf] from comment #13)
> Bug 1158396 changed things in two ways.  First of all, it modified the app
> so that it ignores swipes while transitioning. It was a bug that it wasn't
> already doing that. So if you are doing two swipes in rapid succession,
> you'd see the first succeed, then the subsequent one would fail.  To fix
> this, you just need to wait a bit between swipes, I think.

I tried this. I also tried slowing down the swiping speed, but it all causes the whole swiping to be disabled for the next tried swipe (this is also happening when you do it by hand).

> If your test is already waiting between swipes, then the other change may
> have broken the tests. It used to be that the `transitioning` flag was
> cleared on a timer. But that turned out to be racy and I changed it to clear
> the flag on the transitionend event. If the marionette test framework is not
> delivering transitionend events, then that could be the source of the
> problem, too.

Why should the marionette test framework deliver the transitionend event. Isn't that event happening automatically when the image has finished transitioning?

> Sorry I broke this. I don't know if I just didn't look at the test results
> carefully before landing or what.

Not sure yet if your change caused this automated test failure.
Afaik, this is still passing on b2g desktop, apparently (although it doesn't for me, locally on Mac).

> Is this enough information to fix the test? I know nothing at all about
> python marionette tests, so I'm hoping you can fix it, but let me know if
> you'd like my help.

Thanks for you answer, I'll try to fix the test.

Comment 17

[Martijn Wargers (dead)]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #16)
> > Sorry I broke this. I don't know if I just didn't look at the test results
> > carefully before landing or what.
> 
> Not sure yet if your change caused this automated test failure.
> Afaik, this is still passing on b2g desktop, apparently (although it doesn't
> for me, locally on Mac).

Yep, it's still passing on b2g desktop (linux, that is), see Gip f15 on:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=45547d19c355
https://s3-us-west-2.amazonaws.com/taskcluster-public-artifacts/MwoQHITsSoSoP8sUV9XaQA/0/public/logs/live_backing.log

Comment 18

[Martijn Wargers (dead)]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #15)
> I triggered http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/824/
> with no backout of the patch from bug 1158396.
> And I triggered
> http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/825/ which has the
> backout of the patch from bug 1158396.

Apparently, bug 1158396 is not the cause of this, because the failure is also happening in there.

Comment 19

[Martijn Wargers (dead)]

This is a more narrow regression range:
https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/tinderbox-builds/b2g-inbound-flame-kk-eng/20150428132114/ fails

https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/tinderbox-builds/b2g-inbound-flame-kk-eng/20150428130914/ passes

No idea on how to get the checkins in that area.

Comment 20

[Martijn Wargers (dead)]

I guess it started happening with the checkin of http://hg.mozilla.org/integration/b2g-inbound/rev/06aac68a8319

Comment 21

[Martijn Wargers (dead)]

QAnalyst, if you guys could find out a regression range on mozilla-inbound for test_gallery_flick.py, that would be great:
https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/tinderbox-builds/mozilla-inbound-flame-kk-eng/

I would start with dates somewhere around 20150427163916 for instance.

Comment 22

[Peter Bylenga [:PBylenga]]

Sure thing.

Comment 23

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Based on comment 19 : http://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=c5607755d8fa&tochange=5ea16376a403

Comment 24

[Martijn Wargers (dead)]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #23)
> Based on comment 19 :
> http://hg.mozilla.org/integration/b2g-inbound/
> pushloghtml?fromchange=c5607755d8fa&tochange=5ea16376a403

Thanks, the pull request from bug 1158396 is in there. From comment 18, it would seem not to be the cause of this bug, but I don't see any other pull requests that could have caused this.
It makes me wonder if backing out the patch for bug 1158396 did work at all or if that isn't possible for testing on Jenkins adhoc testing.
Since it seems to be part of gecko (where the regression window is there), I would think the issue is somewhere caused by something in Gecko land, but the pushloghtml only shows Gaia changes.

Comment 25

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #24)
> (In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from
> comment #23)
> > Based on comment 19 :
> > http://hg.mozilla.org/integration/b2g-inbound/
> > pushloghtml?fromchange=c5607755d8fa&tochange=5ea16376a403
> 
> Thanks, the pull request from bug 1158396 is in there. From comment 18, it
> would seem not to be the cause of this bug, but I don't see any other pull
> requests that could have caused this.
> It makes me wonder if backing out the patch for bug 1158396 did work at all
> or if that isn't possible for testing on Jenkins adhoc testing.
> Since it seems to be part of gecko (where the regression window is there), I
> would think the issue is somewhere caused by something in Gecko land, but
> the pushloghtml only shows Gaia changes.

Jenkins only pulls the tests/harness from Gaia, so backing out a change will not influence what's on the device. Given the context of the change I would say this is highly likely to be the cause. Could you try the suggestions from comment 13? Try adding a sleep after the first flick, or look into whether Marionette is firing the transitionend event after the moveByOffset call.

If this is not happening when using the device manually, then it's almost certainly a Marionette bug.

Comment 26

[Martijn Wargers (dead)]

Ok, thanks, then my bet is still that this is somehow caused by bug 1158396.
I did already the suggestions from comment 13, that didn't help. After the first Marionette flick, the next flicks don't work, either by Marionette or manually.

I did some testing, adding an event listener in the document (using execute_script) and it really looks like the transitionend event is not firing when using a Marionette flick. As opposed to a manual flick, where the transitioned event does fire.

Comment 27

[GitHub Autolander Bot]

Attachment: [gaia] mwargers:1161441 > mozilla-b2g:master

Comment 28

[Martijn Wargers (dead)]

Comment on attachment 8602708 [details] [review]
[gaia] mwargers:1161441 > mozilla-b2g:master

This seems to succesfully workaround the issue.
Ive created bug 1163143 for the fact that a Marionette flick action doesn't generate the necessary transitionend event in this case.

Comment 29

[Martijn Wargers (dead)]

Regressionwindow-wanted is not necessary anymore, this is caused by the checkin of bug 1158396.

Comment 30

[Martijn Wargers (dead)]

Comment on attachment 8603529 [details] [review]
[gaia] mwargers:1161441 > mozilla-b2g:master

This is a workaround for this failure, until bug 1163143 gets fixed.

Comment 31

[Martijn Wargers (dead)]

It would also be needed for v2.2.

Comment 32

[GitHub Autolander Bot]

Attachment: [gaia] mwargers:1161441_v2.2 > mozilla-b2g:v2.2

Comment 33

[Martijn Wargers (dead)]

Comment on attachment 8604170 [details] [review]
[gaia] mwargers:1161441_v2.2 > mozilla-b2g:v2.2

This is the pull request for v2.2. Shing, can you drive this to the v2.2 tree. I still don't know what the current state of Treeherder is for v2.2. Perhaps it's working now and can it be checked in once Treeherder try is green?

Comment 34

[Martijn Wargers (dead)]

Ok, it looks like Treeherder v2.2 try seems all right now, because it seems all green.

Comment 35

[Shing Lyu [:shinglyu]]

Thank you!

Comment 36

[Martijn Wargers (dead)]

Oops, forgot to merge this: https://github.com/mozilla-b2g/gaia/commit/f61fd55280ee380f1fba0927009270a60e3fb83f

Merged v2.2 too: https://github.com/mozilla-b2g/gaia/commit/fa6e22045468f7f79158029cb0df0aa827b77dd4