761423 - screenshot() method should save JPEG images and offer tests details about the taken image

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: example screenshot (level: 0.3)

Right now we store screenshots as PNG files which are quite large. With bug 564388 fixed in Firefox 7 we can now even store JPEG files. I have checked different levels of quality and 0.3 is probably a good fit between quality and size. I will attach an example image to this bug. With that change we drop the image size from 74kb to 32kb.

Further we should return details of the image to the callee of screenshot so it can be even re-used in tests.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: example screenshot (level: 0.4)

Just for reference one more image with level 0.4 which is 35kb in size. What do you think is the level we should use? My upcoming patch will use 0.3 for the first round.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v1

Pointer to Github pull-request

Comment 3

[Jeff Hammel]

Comment on attachment 629993 [details]
Patch v1

This is probably fine.  Note that since jpeg is lossy compression and png is not, if we're ever doing pixel by pixel comparisons, jpeg will probably not suffice.  We currently do not

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 629993 [details]
Patch v1

We never intended to use Mozmill as an image comparison tool. If that's the case in the future we can add an option which allows to specify the type and compression of images.

So I have checked the test and figured that more stuff has to be tested. While implementing those changes the test failed because the current save action leaves the file open and the test can't delete it. So I have changed all to the new NetUtils async copy action which is more safe as the WebBrowserPersist one.

Further I did:
* Renaming the screenShot -> screenshot method
* Make the utils.takeScreenshot() method independent so it doesn't implicitly saves the screenshot to disk. This looked way broken.

Comment 5

[Jeff Hammel]

Comment on attachment 629993 [details]
Patch v1

looks fine

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Pushed:
https://github.com/mozautomation/mozmill/commit/eefed2195ef72d229c4253680c9e08c176ae5e0e