test_gallery_crop_photo is incorrectly asserting

RESOLVED FIXED

Status

Firefox OS
Gaia::UI Tests
P2
normal
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: zac, Assigned: dwong)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
The test_gallery_crop_photo is using assertLess to compare two strings.

This should be resolved to compare numbers or the assert changed entirely to assert that the image has been cropped.
(Reporter)

Updated

4 years ago
Priority: -- → P2
(Assignee)

Comment 1

4 years ago
I think I've come up with a solution for now by comparing the rendered areas of the images. As it's cropping a landscape photo to a portrait on a portrait phone the rendered area should increase as the rendered height is increased to fit the image. Though it might be too fragile as choosing different crop settings or images could break it. Is there an easy way to get the actual image heights and widths?
Assignee: nobody → dwong
(Assignee)

Comment 2

4 years ago
Created attachment 8437818 [details] [review]
Proposed pr that adjusts assert to compare rendered image area
Attachment #8437818 - Flags: review?(zcampbell)
(Reporter)

Comment 3

4 years ago
Comment on attachment 8437818 [details] [review]
Proposed pr that adjusts assert to compare rendered image area

I think your hunch is correct that just looking at the size could be unreliable.

It might be safer just to return the scale as an [x, y] tuple which can then be asserted.
Attachment #8437818 - Flags: review?(zcampbell) → review-
(Assignee)

Comment 4

4 years ago
Thanks Zac, I'll look into it, though I will wait on the status of Bug 1023305 as it seems to attempt to address the issue as well
(Assignee)

Comment 5

4 years ago
I came back to this since I realized the 1.4 crop assert was a separate issue. But would you prefer just doing an assert less directly on the list/tuple or comparing them elementwise? It seems the scale's x, y properties x==y always.
Flags: needinfo?(zcampbell)
(Assignee)

Comment 6

4 years ago
Created attachment 8440161 [details] [review]
New PR adjusting current_scale to return a list of floats to represent scale x,y isntead of string

Erm, I forget that Python compares it elementwise anyways. Here is the new pull with the changes you asked for.
Attachment #8437818 - Attachment is obsolete: true
Attachment #8440161 - Flags: review?(zcampbell)
Flags: needinfo?(zcampbell)

Comment 7

4 years ago
Comment on attachment 8440161 [details] [review]
New PR adjusting current_scale to return a list of floats to represent scale x,y isntead of string

I am going to give this an r+
Attachment #8440161 - Flags: review+
(Reporter)

Comment 8

3 years ago
Comment on attachment 8440161 [details] [review]
New PR adjusting current_scale to return a list of floats to represent scale x,y isntead of string

r+ thanks Dylan, sorry about the delay in reviewing it!
Attachment #8440161 - Flags: review?(zcampbell) → review+
(Reporter)

Comment 9

3 years ago
Merged:
https://github.com/mozilla-b2g/gaia/commit/32c6a65f74ecf7d5d2d33c09fe2e85d31200a5b0
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.