Bug 15558 - DumpRenderTree should support automated printing tests
Summary: DumpRenderTree should support automated printing tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Printing (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on: 15565
Blocks: 15548
  Show dependency treegraph
 
Reported: 2007-10-18 14:05 PDT by Eric Seidel (no email)
Modified: 2008-07-23 16:41 PDT (History)
1 user (show)

See Also:


Attachments
a fix (30.85 KB, patch)
2007-10-18 15:01 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
improved fix (29.71 KB, patch)
2007-10-18 20:16 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Updated printing test patch (9.91 KB, patch)
2008-07-09 16:08 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2007-10-18 14:05:41 PDT
DumpRenderTree should support automated printing tests
Comment 1 Eric Seidel (no email) 2007-10-18 15:01:15 PDT
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.
Comment 2 Adam Roben (:aroben) 2007-10-18 15:05:40 PDT
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.
Comment 3 Eric Seidel (no email) 2007-10-18 15:12:44 PDT
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
Comment 4 Adam Roben (:aroben) 2007-10-18 15:15:29 PDT
(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.
Comment 5 Eric Seidel (no email) 2007-10-18 20:16:56 PDT
Created attachment 16728 [details]
improved fix
Comment 6 Adam Roben (:aroben) 2007-10-18 20:21:19 PDT
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.
Comment 7 Eric Seidel (no email) 2007-10-18 20:28:19 PDT
(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.
Comment 8 Eric Seidel (no email) 2007-10-19 02:05:45 PDT
Comment on attachment 16728 [details]
improved fix

Given my comments above, I'd like to mark this change r=? again.
Comment 9 Adam Roben (:aroben) 2007-10-19 10:48:57 PDT
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.
Comment 10 Eric Seidel (no email) 2007-10-19 10:56:53 PDT
Follow-up filed as bug 15565.
Comment 11 Eric Seidel (no email) 2007-10-21 01:42:27 PDT
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.
Comment 12 Eric Seidel (no email) 2007-11-20 16:15:10 PST
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.
Comment 13 Eric Seidel (no email) 2007-12-15 01:41:00 PST
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.
Comment 14 Eric Seidel (no email) 2007-12-15 01:41:21 PST
Comment on attachment 16728 [details]
improved fix

Clearing review flag until bug 15565 is fixed.
Comment 15 Eric Seidel (no email) 2008-07-09 16:08:14 PDT
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(-)
Comment 16 Eric Seidel (no email) 2008-07-09 16:19:11 PDT
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).
Comment 17 Mark Rowe (bdash) 2008-07-23 16:41:49 PDT
Comment on attachment 22192 [details]
Updated printing test patch

Clearing review flag as the bug is closed.