Closed Bug 1546744 Opened 5 months ago Closed 5 months ago

Screenshots test does not assert matching bitmaps correctly

Categories

(GeckoView :: General, defect, P1)

Unspecified
All
defect

Tracking

(firefox66 wontfix, firefox67 wontfix, firefox68 fixed)

RESOLVED FIXED
mozilla68
Tracking Status
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- fixed

People

(Reporter: eeejay, Assigned: fluffyemily)

Details

(Whiteboard: [geckoview:fenix:m5])

Attachments

(2 files)

I'm not sure why, but the assert call in ScreenshotTest.assertScreenshotResult does not actually fail the test when the bitmaps differ. I replaced it with this:

assertThat("Screenshots are the same", it.sameAs(comparisonImage), equalTo(true))

And I see a couple of failures.

Assignee: nobody → etoop
Priority: -- → P1
Whiteboard: [geckoview:fenix:m5]

There is something wiggy here withy Bitmap.sameAs.

When I run the following code, all assertions pass:

assertThat("Widths are the same", comparisonImage.width, equalTo(it.width))
assertThat("Heights are the same", comparisonImage.height, equalTo(it.height))
assertThat("Byte counts are the same", comparisonImage.byteCount, equalTo(it.byteCount))
assertThat("Configs are the same", comparisonImage.config, equalTo(it.config))
val comparisonPixels: ByteBuffer = ByteBuffer.allocate(comparisonImage.byteCount)
comparisonImage.copyPixelsToBuffer(comparisonPixels)
val itPixels: ByteBuffer = ByteBuffer.allocate(it.byteCount)
it.copyPixelsToBuffer(itPixels)
assertThat("Bytes are the same", comparisonPixels, equalTo(itPixels))

but when I run the following for the same Bitmaps, the result is false.

assertThat("Screenshots are the same", it.sameAs(comparisonImage), equalTo(true))

As the former is all we want to assert (that the correct dimension, bytes and configurations are present) and the requirement that they are the same according to Bitmap.sameAs it not necessary (given the completely different methods of generating the images) I will update the tests to break down the comparison to just those elements we care about.

Pushed by etoop@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f348b1ddb6c6
Ensure we are testing for the correct things when validating screenshots r=eeejay
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.