Bug 69590 - There should be a way to disable optimizer in webkit-patch rebaseline-expectations
Summary: There should be a way to disable optimizer in webkit-patch rebaseline-expecta...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
: 72860 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-10-06 18:29 PDT by Ryosuke Niwa
Modified: 2012-03-05 18:04 PST (History)
7 users (show)

See Also:


Attachments
Patch (6.33 KB, patch)
2012-03-05 17:50 PST, Dirk Pranke
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2011-10-06 18:29:31 PDT
There must be a way to disable expectation optimizer in webkit-patch rebaseline-expectations.

I often rebaseline then use pretty-diff to see test diffs because plain diff and wdiff hurt my eyes, but diffs are all messed when expected results are optimized.
Also, it erroneously puts render tree dumps in cross-platform directory.
Comment 1 Ryosuke Niwa 2011-10-06 18:31:14 PDT
I'd say that this is a requirement before we can remove rebaseline-chromium-webkit-tests.
Comment 2 Adam Barth 2011-10-06 18:32:00 PDT
Why is it wrong to put render tree dumps in the cross-platform directory when they're the same on most (all?) platforms?
Comment 3 Adam Barth 2011-10-06 18:32:38 PDT
I think the real issue is that there isn't a good way of viewing the effects of changing expectations, which is something we should fix.
Comment 4 Ryosuke Niwa 2011-10-06 18:35:31 PDT
(In reply to comment #2)
> Why is it wrong to put render tree dumps in the cross-platform directory when they're the same on most (all?) platforms?

Because that'll make missing results to failures on some ports.
(In reply to comment #3)

> I think the real issue is that there isn't a good way of viewing the effects of changing expectations, which is something we should fix.

Right but I don't want to learn how to use new tools. pretty-diff works as is, and I'd like to keep using that.
Comment 5 Adam Barth 2011-10-06 18:37:59 PDT
Right, pretty-diff should help you understand how a patch will change results on various platforms.
Comment 6 Ryosuke Niwa 2011-10-06 18:39:42 PDT
Optimizing expected results is great but that can be done after I've verified the correctness of rebaselines.
Comment 7 Dirk Pranke 2012-03-05 17:50:42 PST
Created attachment 130251 [details]
Patch
Comment 8 Ryosuke Niwa 2012-03-05 17:52:40 PST
Comment on attachment 130251 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130251&action=review

> Tools/Scripts/webkitpy/tool/commands/rebaseline.py:206
> +        if options.optimize:

I think it's better to do an early return here.
Comment 9 Adam Barth 2012-03-05 17:53:28 PST
Comment on attachment 130251 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130251&action=review

> Tools/Scripts/webkitpy/tool/commands/rebaseline.py:160
> +            optparse.make_option('--optimize', action='store_true', default=True,
> +                help=('Optimize/de-dup the expectations after rebaselining (default).')),
> +            optparse.make_option('--no-optimize', dest='optimize', action='store_false',
> +                help=('Do not optimize/de-dup the expectations after rebaselining '
> +                      'You can use "webkit-patch optimize-baselines" to optimize separately.')),

For webkit-patch, we usually just provide a flag for the non-default setting.
Comment 10 Dirk Pranke 2012-03-05 17:54:23 PST
*** Bug 72860 has been marked as a duplicate of this bug. ***
Comment 11 Dirk Pranke 2012-03-05 18:04:32 PST
Committed r109831: <http://trac.webkit.org/changeset/109831>