Bug 81054

Summary: Have webkit-patch rebaseline-test update test_expectations.txt
Product: WebKit Reporter: Ojan Vafai <ojan>
Component: New BugsAssignee: Ojan Vafai <ojan>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, adamk, dglazkov, dpranke
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch dpranke: review+, dpranke: commit-queue-

Description Ojan Vafai 2012-03-13 16:22:15 PDT
Have webkit-patch rebaseline-test update test_expectations.txt
Comment 1 Ojan Vafai 2012-03-13 16:24:51 PDT
Created attachment 131746 [details]
Patch
Comment 2 Ojan Vafai 2012-03-13 16:27:03 PDT
I could also imagine making this a separate webkit-patch command instead that garden-o-matic calls. I'm not sure that's better though. It seems if someone uses "webkit-patch rebaseline-test" manually, they'd still want test_expectations.txt to be updated.
Comment 3 Adam Barth 2012-03-13 16:31:24 PDT
@dglazkov: Would you be willing to take a look at this patch since it involves editing test_expectation.txt files?
Comment 4 Dirk Pranke 2012-03-13 16:37:38 PDT
the patch looks pretty good to me; but I haven't stared at it in detail. If dglazkov is otherwise occupied, I can review it.
Comment 5 Dimitri Glazkov (Google) 2012-03-14 15:01:39 PDT
Comment on attachment 131746 [details]
Patch

Oops, sorry. I don't know how I missed this. The test_expectations changes look great. I am not super-familiar with other parts. Dirk -- you wanna take a look?
Comment 6 Dirk Pranke 2012-03-14 15:14:49 PDT
Comment on attachment 131746 [details]
Patch

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

looks fine w/ the one change to use port.all_test_configuration().

> Tools/Scripts/webkitpy/tool/commands/rebaseline.py:113
> +        # FIXME: Support non-Chromium ports.

Nit: do you actually need this? what happens if you run a non-chromium port? I would expect it to be mostly fine.

> Tools/Scripts/webkitpy/tool/commands/rebaseline.py:124
> +        # FIXME: Do something more general here.

You should use port.all_test_configurations() and then just filter on the version.
Comment 7 Dirk Pranke 2012-03-14 15:15:08 PDT
Comment on attachment 131746 [details]
Patch

cq-'ing to address the comments one way or another.
Comment 8 Ojan Vafai 2012-03-14 16:33:59 PDT
Comment on attachment 131746 [details]
Patch

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

>> Tools/Scripts/webkitpy/tool/commands/rebaseline.py:113
>> +        # FIXME: Support non-Chromium ports.
> 
> Nit: do you actually need this? what happens if you run a non-chromium port? I would expect it to be mostly fine.

I suppose not. I'll remove it.

>> Tools/Scripts/webkitpy/tool/commands/rebaseline.py:124
>> +        # FIXME: Do something more general here.
> 
> You should use port.all_test_configurations() and then just filter on the version.

Oh, good idea!
Comment 9 Ojan Vafai 2012-03-14 16:38:51 PDT
http://trac.webkit.org/changeset/110783
Comment 10 Ojan Vafai 2012-03-14 17:02:11 PDT
Reverted r110783 for reason:

Caused existing test expectation lines to expand to one for each test configuration.

Committed r110789: <http://trac.webkit.org/changeset/110789>
Comment 11 Ojan Vafai 2012-03-16 12:03:06 PDT
Committed r111039: <http://trac.webkit.org/changeset/111039>