Summary: | DumpRenderTree should support automated printing tests | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||||
Component: | Printing | Assignee: | Eric Seidel (no email) <eric> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | aroben | ||||||||
Priority: | P2 | ||||||||||
Version: | 523.x (Safari 3) | ||||||||||
Hardware: | Mac | ||||||||||
OS: | OS X 10.4 | ||||||||||
Bug Depends on: | 15565 | ||||||||||
Bug Blocks: | 15548 | ||||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2007-10-18 14:05:41 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 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.
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 (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. Created attachment 16728 [details]
improved fix
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.
(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 on attachment 16728 [details]
improved fix
Given my comments above, I'd like to mark this change r=? again.
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.
Follow-up filed as bug 15565. 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. 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. 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 on attachment 16728 [details] improved fix Clearing review flag until bug 15565 is fixed. 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(-)
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 on attachment 22192 [details]
Updated printing test patch
Clearing review flag as the bug is closed.
|