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

Glenn Adams glenn at skynav.com
Thu Mar 21 16:50:45 PDT 2013

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>

More information about the webkit-dev mailing list