Closed Bug 1364762 Opened 7 years ago Closed 3 years ago

Intermittent testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py TestScreenCaptureChrome.test_capture_full_area | AssertionError: [truncated]... != [truncated]...

Categories

(Testing :: Marionette Client and Harness, defect, P5)

Product:
Testing
Component:
Marionette Client and Harness
Version:
Version 3
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED INCOMPLETE

People

(Reporter: intermittent-bug-filer, Unassigned)

References

Details

(Keywords: bulk-close-intermittents, intermittent-failure, Whiteboard: [stockwell unknown])

Attachments

(2 files, 1 obsolete file)

screenshot1.png
16.43 KB, image/png
Details
screenshot2.png
16.43 KB, image/png
Details 
I wonder if this can be affected by recent UI changes to Firefox, and more recently, by https://bugzilla.mozilla.org/show_bug.cgi?id=1355890.  The robot icon that is shown in the address bar when the browser’s remote protocol is activated is a GIF of a robot that occasionally blinks.
When I let both PNGs graphically compare those are identically. I used http://huddle.github.io/Resemble.js/ for that. But binary wise we have a difference. I wonder if we better should not do it binary wise, or what that actually means. I will attach both files in a moment.
Maybe only the header is different (eg. different time) but the data is the same? It could easily happen because of large data to transfer between Firefox and the client, so that a second difference can happen.
So, the robot is animated and because of that we have differences in the image. See the rolling eyes. I think that you want to remove this animation. Maybe its better to file a new bug for this.
Whiteboard: [stockwell needswork]
I think this bug may have been fixed, via bug 1366366.
oh, this would be cool, I see the failure rate way down.
Comment on attachment 8871362 [details] [diff] [review]
temporarily disable this test everywhere

Marking as obsolete given that we don't need it anymore. I can have a look in the next days (when it happens the next time) if I can find out what the reason is for this test failure.
Attachment #8871362 - Attachment is obsolete: true
Attachment #8871362 - Flags: review?(gbrown)
Whiteboard: [stockwell needswork] → [stockwell unknown]
Whiteboard: [stockwell unknown] → [stockwell needswork]
Whiteboard: [stockwell needswork] → [stockwell unknown]
The recent issue for those failures seems to be the reload button in the toolbar which in one screenshot is disabled, and in the second enabled.

https://dxr.mozilla.org/mozilla-central/rev/cad53f061da634a16ea75887558301b77f65745d/testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py#194

In ScreenCaptureTestCase.setUp() we explicitly load `about:blank` but we somehow return from `get` for a different URL:

1496328967429	Marionette	TRACE	270 -> [0,7,"get",{"url":"about:blank"}]
1496328967436	Marionette	DEBUG	Received DOM event "beforeunload" for "http://127.0.0.1:38467/windowHandles.html"
1496328967449	Marionette	DEBUG	Received DOM event "pagehide" for "http://127.0.0.1:38467/windowHandles.html"
1496328967451	Marionette	DEBUG	Received DOM event "unload" for "http://127.0.0.1:38467/windowHandles.html"
++DOMWINDOW == 28 (0xd6db6800) [pid = 1661] [serial = 345] [outer = 0xd977cc00]
1496328967888	Marionette	DEBUG	Received DOM event "DOMContentLoaded" for "http://127.0.0.1:38467/windowHandles.html"
1496328967895	Marionette	DEBUG	Received DOM event "pageshow" for "http://127.0.0.1:38467/windowHandles.html"
1496328967902	Marionette	TRACE	270 <- [1,7,null,{}]

And as such the events for `about:blank` might still occur but after we returned from `get`. 

It's not something I currently understand.
I had a look at the test in question:

>     def test_capture_full_area(self):
>         # A full capture is not the outer dimensions of the window,
>         # but instead the bounding box of the window's root node (documentElement).
>         screenshot_full = self.marionette.screenshot()
>         screenshot_root = self.marionette.screenshot(element=self.document_element)
> 
>         self.assert_png(screenshot_full)
>         self.assert_png(screenshot_root)
>         self.assertEqual(screenshot_root, screenshot_full)
>         self.assertEqual(self.scale(self.get_element_dimensions(self.document_element)),
>                          self.get_image_dimensions(screenshot_full))

On the second last line, it’s not clear to me that we actually need to test that the screenshots are 100% equivalent.  It should be enough just to test that we get the right dimensions, which we do in the last line.

If it is indeed the case that the continued intermittents here are caused by changes to the Firefox UI, then I believe we are setting ourselves up for failure.  It’s not vital to the success of this test that the screenshots are the same; that’s not what we’re testing.
In that case we could have left the rolling eyes of the robot!
(In reply to Henrik Skupin (:whimboo) from comment #22)
> In that case we could have left the rolling eyes of the robot!

I suppose this is true.
(In reply to Andreas Tolfsen ‹:ato› from comment #23)
> > In that case we could have left the rolling eyes of the robot!
> 
> I suppose this is true.

Are you willed to fix that, and get your proposal implemented? I'm happy to review. Maybe reverting the rolling eyes might need further changes to other screenshot tests.
(In reply to Henrik Skupin (:whimboo) from comment #24)
> (In reply to Andreas Tolfsen ‹:ato› from comment #23)
> > > In that case we could have left the rolling eyes of the robot!
> > 
> > I suppose this is true.
> 
> Are you willed to fix that, and get your proposal implemented? I'm happy to
> review. Maybe reverting the rolling eyes might need further changes to other
> screenshot tests.

I would say patches are accepted on that.  I don’t consider reverting that
change a priority.
(In reply to Andreas Tolfsen ‹:ato› from comment #25)
> (In reply to Henrik Skupin (:whimboo) from comment #24)
> > (In reply to Andreas Tolfsen ‹:ato› from comment #23)
> > > > In that case we could have left the rolling eyes of the robot!
> > > 
> > > I suppose this is true.
> > 
> > Are you willed to fix that, and get your proposal implemented? I'm happy to
> > review. Maybe reverting the rolling eyes might need further changes to other
> > screenshot tests.
> 
> I would say patches are accepted on that.  I don’t consider reverting that
> change a priority.

Actually, after more consideration, are we sure we want an animated GIF in the
UI?  It sounds like a static image is qualitatively better.
We might want to leave the image as it is right now, given that we still don't know which other side-effects an animation could have. 

What I meant with a patch is what you proposed in comment 21 just to fix the test.
Status: NEW → RESOLVED
Closed: 7 years ago
Keywords: bulk-close-intermittents
Resolution: --- → INCOMPLETE
Status: RESOLVED → VERIFIED
Status: VERIFIED → REOPENED
Resolution: INCOMPLETE → ---
Summary: Intermittent test_screenshot.py TestScreenCaptureChrome.test_capture_full_area | AssertionError: [truncated]... != [truncated]... → Intermittent testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py TestScreenCaptureChrome.test_capture_full_area | AssertionError: [truncated]... != [truncated]...

No more failures since 4 months. Closing as incomplete.

Status: REOPENED → RESOLVED
Closed: 7 years ago → 4 years ago
Resolution: --- → INCOMPLETE

Most recent failure: https://treeherder.mozilla.org/logviewer?job_id=335752540&repo=autoland

Status: RESOLVED → REOPENED
Resolution: INCOMPLETE → ---

Very low frequency. P5 S4

Severity: normal → S4
Priority: -- → P5

https://wiki.mozilla.org/Bug_Triage#Intermittent_Test_Failure_Cleanup
For more information, please visit auto_nag documentation.

Status: REOPENED → RESOLVED
Closed: 4 years ago → 3 years ago
Resolution: --- → INCOMPLETE
Product: Testing → Remote Protocol
Moving bug to Testing::Marionette Client and Harness component per bug 1815831.
Component: Marionette → Marionette Client and Harness
Product: Remote Protocol → Testing
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: