1021027 - [Flame] Update locator in gallery - fullscreen image page

Status: RESOLVED FIXED

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

Description

[Viorela Ioia [:viorela]]

Gallery tests are failing in the latest v2.0 build. Probably we just need to update a locator.
We should investigate and fix this issue.

Traceback (most recent call last):
  File "/home/viorelaioia/.virtualenvs/gaia/lib/python2.7/site-packages/marionette_client-0.7.7-py2.7.egg/marionette/marionette_test.py", line 163, in run
    testMethod()
  File "/home/viorelaioia/WebQA/gaia/tests/python/gaia-ui-tests/gaiatest/tests/functional/gallery/test_gallery_crop_photo.py", line 23, in test_gallery_crop_photo
    image = gallery.tap_first_gallery_item()
  File "/home/viorelaioia/WebQA/gaia/tests/python/gaia-ui-tests/gaiatest/apps/gallery/app.py", line 49, in tap_first_gallery_item
    return NextView(self.marionette)
  File "/home/viorelaioia/WebQA/gaia/tests/python/gaia-ui-tests/gaiatest/apps/gallery/regions/fullscreen_image.py", line 22, in __init__
    self.wait_for_element_displayed(*self._current_image_locator)
  File "/home/viorelaioia/WebQA/gaia/tests/python/gaia-ui-tests/gaiatest/apps/base.py", line 42, in wait_for_element_displayed
    lambda m: m.find_element(by, locator).is_displayed())
  File "/home/viorelaioia/.virtualenvs/gaia/lib/python2.7/site-packages/marionette_client-0.7.7-py2.7.egg/marionette/wait.py", line 143, in until
    cause=last_exc)
TimeoutException: TimeoutException: Timed out after 30.0 seconds

I'm able to reproduce the failure by running the automated tests.

Comment 1

[Stephen Donner [:stephend] Not actively reading bugmail]

Dylan, can you investigate?  Thanks!

Comment 2

[Dylan Chun Wong [:dwong]]

(In reply to Stephen Donner [:stephend] from comment #1)
> Dylan, can you investigate?  Thanks!

Yeah no problem! :)

Comment 3

[Dylan Chun Wong [:dwong]]

Attachment: PR that fixes locator for gallery

Comment 4

[Viorela Ioia [:viorela]]

Merged: https://github.com/mozilla-b2g/gaia/commit/01d49e5cc670e2146be3bd523f0b7132a88178d0

Comment 5

[Walter Chen[:ypwalter][:wachen]]

It failed on Buri, too. Whatever, it is fixed now :P

Comment 6

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

I just noticed this because I needed to bring this fix into a branch, and I'm concerned that the fix is fragile. The locator is now '#frames > div.frame[style ~= "translateX(0px);"] > img[style ~= "block;"]' which means we're going to pick the first direct img child of the frame that's in view with the word block in it's style attribute.

The best fix here would be to get a class added to the current image so we can simply locate it as img.current or similar. That would of course require us to raise a bug and politely ask a gaia developer (ideally one actively working on the Gallery app) to work on it. Failing that, we could locate all images and return the one with the correct location (most likely x = 0).

Also, the commit message for this fix was 'Bug 1021027 - [Flame] Investigate failure in gallery tests', which does not reflect the change.

Dylan: If you agree, would you mind following this up with another bug to address these concerns?

Comment 7

[Dylan Chun Wong [:dwong]]

(In reply to Dave Hunt (:davehunt) from comment #6)
> I just noticed this because I needed to bring this fix into a branch, and
> I'm concerned that the fix is fragile. The locator is now '#frames >
> div.frame[style ~= "translateX(0px);"] > img[style ~= "block;"]' which means
> we're going to pick the first direct img child of the frame that's in view
> with the word block in it's style attribute.
> 
> The best fix here would be to get a class added to the current image so we
> can simply locate it as img.current or similar. That would of course require
> us to raise a bug and politely ask a gaia developer (ideally one actively
> working on the Gallery app) to work on it. Failing that, we could locate all
> images and return the one with the correct location (most likely x = 0).
> 
> Also, the commit message for this fix was 'Bug 1021027 - [Flame] Investigate
> failure in gallery tests', which does not reflect the change.
> 
> Dylan: If you agree, would you mind following this up with another bug to
> address these concerns?

I agree Dave, I've filed Bug 1021782, but I wasn't sure what keywords and such to add, so edit it as you see fit. For the commit message is there anything I can do about that?

Comment 8

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

Thanks Dylan, nothing we can do now about the commit message, but just take care in the future that your message indicates the change. Quite often this can be the same as the bug summary but in some cases that doesn't make sense. We should also be checking commit messages in the reviews.

Comment 10

[Florin Strugariu [:Bebe]]

Attachment: Uplift to 1.4 Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20566

Comment 11

[Viorela Ioia [:viorela]]

Comment on attachment 8440648 [details] [review]
Uplift to 1.4 Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20566

LGTM, but we need to update the locator when Bug 1021782 is fixed

Comment 12

[Viorela Ioia [:viorela]]

Merged on v1.4: https://github.com/mozilla-b2g/gaia/commit/b74d001900a354b2240e3ad3a250a9dffede8902
We keep this bug open until Bug 1021782 is fixed. Then we will update the locator on all branches.

Comment 13

[Dylan Chun Wong [:dwong]]

Now that bug 1021782 has landed the locator can be updated to "div.frames.current > img.image-view" though I don't have my device on me currently to test it. It did work when using "#frames > .current > img:nth-of-type(1)" Though at the time I didn't notice that they also added the image-view class. Anyone is free to make a PR to update in the mean time, otherwise I will in the morning when I have a device to test with.

Comment 14

[Viorela Ioia [:viorela]]

The commit with the changes is not in the current build, so I didn't update the locator for now (the test would have failed).
It will probably land in the next build.  :)

Comment 15

[Viorela Ioia [:viorela]]

Attachment: Github PR: https://github.com/mozilla-b2g/gaia/pull/21385

Comment 16

[Florin Strugariu [:Bebe]]

https://github.com/mozilla-b2g/gaia/commit/89b34cf1de809f322f328609923ba5a3caf2ded1