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

[Glenn Adams]

On Thu, Mar 21, 2013 at 5:40 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

> 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.
>

A hypothesis is not an assumption. It needs to be tested further. Be sure I
am not making any unwarranted assumption.


>
> 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.
>

That's my platform, so I have to manage with it.


>
> 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.
>

I will if possible, but I'm just saying it may not be possible on the
initial landing in all cases. If it isn't then I expect to have to add
rebaseline expectations and then take responsibility for following up on
them ASAP.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20130321/ead5fee4/attachment.html>