Closed Bug 1021027 Opened 7 years ago Closed 7 years ago

[Flame] Update locator in gallery - fullscreen image page

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: viorela, Unassigned)

References

Details

Attachments

(3 files)

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.
Assignee: nobody → florin.strugariu
Priority: -- → P1
Dylan, can you investigate?  Thanks!
Flags: needinfo?(dwong)
(In reply to Stephen Donner [:stephend] from comment #1)
> Dylan, can you investigate?  Thanks!

Yeah no problem! :)
Flags: needinfo?(dwong)
Attachment #8435269 - Flags: review?(viorela.ioia)
Attachment #8435269 - Flags: review?(viorela.ioia) → review+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
It failed on Buri, too. Whatever, it is fixed now :P
Assignee: florin.strugariu → nobody
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?
Flags: needinfo?(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?
Flags: needinfo?(dwong) → needinfo?(dave.hunt)
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.
Flags: needinfo?(dave.hunt)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Summary: [Flame] Investigate failure in gallery tests → [Flame] Update locator in gallery - fullscreen image page
Duplicate of this bug: 1023305
Depends on: 1021782
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
Attachment #8440648 - Flags: review?(viorela.ioia) → review+
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.
Attachment #8440648 - Flags: review?(zcampbell)
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.
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.  :)
Attachment #8450839 - Flags: review?(zcampbell)
Attachment #8450839 - Flags: review?(florin.strugariu)
Attachment #8450839 - Flags: review?(zcampbell)
Attachment #8450839 - Flags: review?(florin.strugariu)
Attachment #8450839 - Flags: review+
https://github.com/mozilla-b2g/gaia/commit/89b34cf1de809f322f328609923ba5a3caf2ded1
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.