Bug 15558 – DumpRenderTree should support automated printing tests

• Component: Printing
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

DumpRenderTree should support automated printing tests

[Eric Seidel (no email)]

Created attachment 16724 [details]
a fix

This isn't a perfect solution, but it works pretty well, and enables a whole new class of tests for DRT.

[Adam Roben (:aroben)]

Comment on attachment 16724 [details]
a fix

+        * platform/win/Skipped: skip printing tests on Windows.
+        * printing: Added.
+        * printing/media-queries-print-expected.txt: Added.
+        * printing/media-queries-print.html: Added.

We should probably just make the printing tests Mac-only for now, since only Mac DRT has support for it.

Seems like we should give the expected results a .pdf extension.

[Eric Seidel (no email)]

I've added a comment locally explaining why I used an intermediate file:

    // Sadly we have to dump to a file and then read from that file again
    // +[NSPrintOperation PDFOperationWithView:insideRect:] requires a rect and prints to a single page
    // likewise +[NSView dataWithPDFInsideRect:] also prints to a single continuous page
    // The goal of this function is to test "real" printing across multiple pages.
    // FIXME: It's possible there might be printing SPI to let us print a multi-page PDF to an NSData object

[Adam Roben (:aroben)]

(In reply to comment #2)
> (From update of attachment 16724 [details] [edit])
> We should probably just make the printing tests Mac-only for now, since only
> Mac DRT has support for it.

Strike that. The tests themselves aren't inherently platform-specific, so should stay at the top level. The  results should move beneath platform/mac, and we need to add the printing directory to platform/qt/Skipped as well.

[Eric Seidel (no email)]

Created attachment 16728 [details]
improved fix

[Adam Roben (:aroben)]

Comment on attachment 16728 [details]
improved fix

+# No support for printAsPDF() in DRT
+printing
+

Could you stick this up near the top of the file, near where we skip webarchive?

Index: LayoutTests/platform/mac/printing/media-queries-print-expected.txt

I think we should have a .pdf extension for this so that it's easily viewable in Preview. We may have to change run-webkit-tests to know to use diff on .pdf files.

[Eric Seidel (no email)]

(In reply to comment #6)
> (From update of attachment 16728 [details] [edit])
> +# No support for printAsPDF() in DRT
> +printing
> +
> 
> Could you stick this up near the top of the file, near where we skip
> webarchive?
> 
> Index: LayoutTests/platform/mac/printing/media-queries-print-expected.txt
> 
> I think we should have a .pdf extension for this so that it's easily viewable
> in Preview. We may have to change run-webkit-tests to know to use diff on .pdf
> files.

Sadlly you ran away from IRC.

I put it at the bottom because it doesn't yet have a radar.  That seemed to be the organization of the file.

webarchives don't currently have a .webarchive extension when we dump them.  I agree that they should have a .pdf extension, but I feel that's outside the scope of this bug.  I'll file a separate bug about it.

[Eric Seidel (no email)]

Comment on attachment 16728 [details]
improved fix

Given my comments above, I'd like to mark this change r=? again.

[Adam Roben (:aroben)]

Comment on attachment 16728 [details]
improved fix

r=me. Would be nice to file a bug saying that we'd like the results to be .pdf though.

[Eric Seidel (no email)]

Follow-up filed as bug 15565.

[Eric Seidel (no email)]

I'm still thinking about implementing dumping to .pdf instead of .txt.  That would make comparing changed values much easier.  I've looked at it briefly, but have not come to any solid conclusions.  I'll make a decision and either land this or clear the flag within the next couple days.

[Eric Seidel (no email)]

Ok, I still haven't landed this. I agree with Aroben, it really needs to dump them as a .pdf file for them to be useful to folks.  Bleh.

[Eric Seidel (no email)]

Blocking this on bug 15565, and clearing the review'd flag.  Once bug 15565 is complete then we can clean up this patch and land.

[Eric Seidel (no email)]

Comment on attachment 16728 [details]
improved fix

Clearing review flag until bug 15565 is fixed.

[Eric Seidel (no email)]

Created attachment 22192 [details]
Updated printing test patch

 LayoutTests/ChangeLog                              |   11 ++++++
 LayoutTests/platform/gtk/Skipped                   |    1 +
 LayoutTests/platform/qt/Skipped                    |   15 +--------
 LayoutTests/platform/win/Skipped                   |    4 ++
 .../printing/media-queries-print-expected.pdf      |  Bin 0 -> 13628 bytes
 LayoutTests/printing/media-queries-print.html      |   25 ++++++++++++++
 WebKitTools/ChangeLog                              |   15 ++++++++
 .../DumpRenderTree/LayoutTestController.cpp        |    9 +++++
 WebKitTools/DumpRenderTree/LayoutTestController.h  |    4 ++
 WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm   |   36 ++++++++++++++++++++
 10 files changed, 106 insertions(+), 14 deletions(-)

[Eric Seidel (no email)]

Landed as r35078 based on aroben's original review.  There were no changes to the patch, just re-applied to TOT (And thanks to 15565, it magically spits out a .pdf file instead of a .txt file now).

[Mark Rowe (bdash)]

Comment on attachment 22192 [details]
Updated printing test patch

Clearing review flag as the bug is closed.