Bug 84327 - [Qt] Make DRT not dump pixel results if test is calling layoutTestController.dumpAsText()
: [Qt] Make DRT not dump pixel results if test is calling layoutTestController....
Status: RESOLVED FIXED
: WebKit
Tools / Tests
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
: 84246
  Show dependency treegraph
 
Reported: 2012-04-19 04:11 PST by
Modified: 2012-04-23 00:10 PST (History)


Attachments
don't dump pixels if shouldDumpAsText is true (1.67 KB, patch)
2012-04-19 04:19 PST, Milian Wolff
ossy: review-
Review Patch | Details | Formatted Diff | Diff
don't dump pixels if shouldDumpAsText is true (1.62 KB, patch)
2012-04-19 05:44 PST, Milian Wolff
no flags Review Patch | Details | Formatted Diff | Diff
don't dump pixels if shouldDumpAsText is true (1.66 KB, patch)
2012-04-19 06:41 PST, Milian Wolff
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-04-19 04:11:51 PST
The DRT-Qt port creates pixel dumps even for test cases where this is not expected. I.e.:

./Tools/Scripts/run-webkit-tests -p LayoutTests/printing/page-break-orphans.html

-> creates a PNG of the page and complains about missing baseline

According to Dirk Pranke (https://lists.webkit.org/pipermail/webkit-dev/2012-April/020349.html) pixel dumps should not be created when layoutTestController.dumpAsText() was called. The attached patch does that and fixes the problem mentioned above.
------- Comment #1 From 2012-04-19 04:19:56 PST -------
Created an attachment (id=137875) [details]
don't dump pixels if shouldDumpAsText is true

patch that checks m_controller->shouldDumpAsText() and prevents the pixel-dump when that is true
------- Comment #2 From 2012-04-19 04:30:06 PST -------
(From update of attachment 137875 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=137875&action=review

Only small nitpicks, otherwise LGTM. Please update the patch and upload it with r?

> Tools/ChangeLog:5
> +        Qt: don't dump pixels if test should be dumped as text
> +
> +        [Qt] don't dump pixels if test should be dumped as text

Could you add the title (the new one, please) only once to the changelog?

> Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:951
>      // FIXME: All other ports don't dump pixels, if generatePixelResults is false.

Please remove this FIXME, we don't need anymore after your fix.
------- Comment #3 From 2012-04-19 05:44:41 PST -------
Created an attachment (id=137886) [details]
don't dump pixels if shouldDumpAsText is true

fix issues raised by Ossy
------- Comment #4 From 2012-04-19 05:51:24 PST -------
(From update of attachment 137886 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=137886&action=review

Something is wrong with Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp, because it can't be applied.

> Tools/ChangeLog:5
> +        [Qt] Make DRT not dump pixel results if test is calling layoutTestController.dumpAsText()
> +
> +        Reviewed by NOBODY (OOPS!).

[Qt] Make DRT not dump pixel results if test is calling layoutTestController.dumpAsText()
https://bugs.webkit.org/show_bug.cgi?id=84327

Reviewed by NOBODY (OOPS!).
------- Comment #5 From 2012-04-19 06:41:42 PST -------
Created an attachment (id=137893) [details]
don't dump pixels if shouldDumpAsText is true

- create patch that directly applies against master
- add bug url to changelog entry
------- Comment #6 From 2012-04-22 23:30:57 PST -------
(From update of attachment 137893 [details])
LGTM, r=me.
------- Comment #7 From 2012-04-23 00:10:44 PST -------
(From update of attachment 137893 [details])
Clearing flags on attachment: 137893

Committed r114875: <http://trac.webkit.org/changeset/114875>
------- Comment #8 From 2012-04-23 00:10:49 PST -------
All reviewed patches have been landed.  Closing bug.