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

[Glenn Adams]

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.

I'm also relying upon EWS to catch a regression before asking for a review.


>
> 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?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20130321/fd11e913/attachment.html>