[webkit-qt] printing unit tests

[Simon Hausmann]

On Friday, April 13, 2012 01:25:13 PM ext Milian Wolff wrote:
> On Thursday 12 April 2012 09:49:18 Simon Hausmann wrote:
> > On Wednesday, April 11, 2012 08:05:39 PM ext Milian Wolff wrote:
> > > Hey all,
> > > 
> > > I'm interested in improving the printing support of the QtWebKit port.
> > 
> > Excellent!
> > 
> > > I notice that a lot of unit tests are skipped and that apparently
> > > isPrinting() is not implemented/handled at all yet.
> > > 
> > > For quite some time now I'm trying to figure out what exactly would be
> > > required to get this done but it seems to be far from easy... Is there
> > > any
> > > documentation besides the chromium spagetthi test code on how the output
> > > should look like or when to output what (image, text, both, ...)?
> > 
> > AFAICS most of the tests (LayoutTests/printing/*-expected* files) verify
> > page breaks and margins using LayoutTestController hooks. I do see two
> > PDFs
> > as expected output, but I don't think we can make Qt output exactly those
> > 
> > :)
> 
> There are lots of expected images as well, but only for some ports, mostly
> mac and chromium under LayoutTests/platform/*/printing/

Right.
 
> > > As far as I can see, I will probably have to handle the isPrinting()
> > > mode
> > > in DumpRenderTreeQt.cpp ~ ::dump() in the if(m_dumpPixels) branch.
> > > Currently, that only paints the viewportSize() to an image. If I'm not
> > > mistaken, one needs to create a copy or QWebFrame::print, similar to
> > > chromiums'
> > > WebViewHost::paintPagesWithBoundaries. But what are the requirements on
> > > the
> > > output? Is there *any* documentation on what the tests should output?
> > > 
> > > Also, is it correct that such tests should be run with "--pixel-tests"?
> > > 
> > > If I run e.g.
> > > 
> > > python Tools/Scripts/new-run-webkit-tests LayoutTests/printing \
> > > 
> > >   --child-processes=1 --qt --verbose -p
> > > 
> > > this creates also images for tests which are not expected to create any
> > > images (like e.g. LayoutTests/printing/page-break-widows.html). How is
> > > this
> > > supposed to be handled, do the test bots know which test to run with
> > > with
> > > arguments? If I look at the TestShell.cpp code from chromium, I'm
> > > guessing
> > > that tests which output text should not output images (TestShell::dump
> > > ->
> > > shouldGeneratePixelResults) - do I interpret this correctly?
> > 
> > Ahh, now I see. Well, currently we don't generally work with pixel tests
> > to
> > be honest, largely because they require a lot of effort to maintain.
> 
> This would also explain why the Qt port has its own share of printing bugs
> ;-)

Sure... but pixel tests are not the only way to test this code.

> See below:
> > On a bigger scale, what would you like to improve with regards to
> > printing?
> > Any work towards improving our test coverage would be great IMHO, I
> > believe
> > we do skip some tests. There might be low-hanging fruit.
> 
> Most importantly it's missing to pass the page height to PrintContext::begin
> in QWebFrame::print. This leads to e.g. improper layouting for all the
> page- break-* css attributes - the unit tests are not run after all... So
> yeah that is a low-hanging fruit but just fixing it would imo not be
> enough. Instead, the unit tests should be run properly to find all the
> other outstanding bugs and missing functionality.

I agree, a fix is best combined with a unit test. But I don't think it should 
be a pixel test.
 

Simon