On Thu, Mar 21, 2013 at 4:50 PM, Glenn Adams <[email protected]> wrote:
> On Thu, Mar 21, 2013 at 5:40 PM, Ryosuke Niwa <[email protected]> wrote: > >> On Thu, Mar 21, 2013 at 4:36 PM, Glenn Adams <[email protected]> wrote: >> >>> >>> On Thu, Mar 21, 2013 at 5:10 PM, Ryosuke Niwa <[email protected]> wrote: >>> >>>> On Thu, Mar 21, 2013 at 4:02 PM, Glenn Adams <[email protected]> wrote: >>>> >>>>> On Thu, Mar 21, 2013 at 12:49 PM, Ryosuke Niwa <[email protected]>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. > I don't think operating based on a hypothesis is a good idea either. > 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. > I do have a Retina MBP too but I don't use it to work on the rendering engine precisely because of this issue. It's expected that every contributor has access to a machine where he/she can run layout tests. Retina MBP is not such a machine. 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. > What are cases where this is not possible? We need to fix that. - R. Niwa
_______________________________________________ webkit-dev mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-dev
