[webkit-dev] Please don't land patches without rebaselining tests for at least one platform

[Ryosuke Niwa]

On Thu, Mar 21, 2013 at 4:36 PM, Glenn Adams <glenn at skynav.com> wrote:

>
> On Thu, Mar 21, 2013 at 5:10 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
>
>> On Thu, Mar 21, 2013 at 4:02 PM, Glenn Adams <glenn at skynav.com> wrote:
>>
>>> On Thu, Mar 21, 2013 at 12:49 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
>>>
>>>> Lately, I've encountering changesets that only add lines to
>>>> TestExpectations and then never baseline tests for any platform.
>>>>
>>>
>>> This (never rebaseline tests for any platform in that changeset) may not
>>> be possible depending on circumstances. For example, I do my dev work on
>>> MBP Retina, which produces different baselines than the platforms used for
>>> mac test bots. As a result, I sometimes have no choice but to land a
>>> changeset without a new baseline and then use garden-o-matic after-the-fact
>>> to land a new baseline.
>>>
>>
>> Then how are you verifying that your patch is correct? How are reviewers
>> supposed to review such a patch?
>>
>> Uploading a rendering engine patch without first verifying that tests are
>> still passing and new tests are generating results as expected sounds like
>> a bad idea to me.
>>
>
> Of course I am verifying that tests are still passing and that new tests
> are generating results as expected. I do this by running NRWT without the
> patch, running it with the patch, then comparing results. This generally
> allows me to see new failures, which I manually review to determine the
> nature of the difference. If the difference is simply minor pixel
> positioning deltas (in text or image output), then I operate on the
> tentative hypothesis that a rebaseline is needed for the given test, unless
> I happen to be making a change that could be attributed to cause the delta.
>

That sounds like a dangerous assumption to make. Due to the DPI
differences, things like anti-aliasing will behave quite differently on
Retina MBP.

In general, I don't recommend people running and relying on layout tests on
Retina MBP especially if you work on the rendering engine at this time.

Or do you have an alternative in mind that would work in my case? Note that
>>> it really isn't practical for me to ask another dev to build my patch
>>> before landing in order to provide me a new baseline that i can add to the
>>> patch.
>>>
>>
>> As I've announced on another thread, EWS now uploads actual results on
>> Bugzilla so this shouldn't be an issue anymore.
>>
>
> But EWS is only testing on a couple of platform/ports yes? Presumably this
> will still impact qt/gtk/efl and perhaps others where EWS is just building
> and not testing?
>

That's fine. I'm asking that you include rebaselines for at least one
platform in the case that wasn't clear.

- R. Niwa
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20130321/b70a4ca2/attachment.html>