Bug 81054 – Have webkit-patch rebaseline-test update test_expectations.txt

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ojan Vafai]

Have webkit-patch rebaseline-test update test_expectations.txt

[Ojan Vafai]

Created attachment 131746 [details]
Patch

[Ojan Vafai]

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.

[Adam Barth]

@dglazkov: Would you be willing to take a look at this patch since it involves editing test_expectation.txt files?

[Dirk Pranke]

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.

[Dimitri Glazkov (Google)]

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?

[Dirk Pranke]

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.

[Dirk Pranke]

Comment on attachment 131746 [details]
Patch

cq-'ing to address the comments one way or another.

[Ojan Vafai]

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!

[Ojan Vafai]

http://trac.webkit.org/changeset/110783

[Ojan Vafai]

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>

[Ojan Vafai]

Committed r111039: <http://trac.webkit.org/changeset/111039>