On Jan 23, 2015, at 10:01 AM, Ryosuke Niwa <rn...@webkit.org> wrote: > > > On Friday, January 23, 2015, Darin Adler <da...@apple.com > <mailto:da...@apple.com>> wrote: > > On Jan 22, 2015, at 10:36 PM, Alexey Proskuryakov <a...@webkit.org > > <javascript:;>> wrote: > > > >> 22 янв. 2015 г., в 17:57, Darin Adler <da...@apple.com <javascript:;>> > >> написал(а): > >> > >> What about the test I cited? > >> > >> svg/css/svg-resource-fragment-identifier-img-src.html > > > > This particular test is buggy - it is a hidpi test, so it dumps results as > > a 1600x1200 image, but its -expected.html is not hidpi, and is dumped as > > 800x600, so hashes are obviously different. I now filed > > <https://bugs.webkit.org/show_bug.cgi?id=140815 > > <https://bugs.webkit.org/show_bug.cgi?id=140815>> about this test. > > > > When we compare pixels, we draw both images into bitmaps of the same size, > > so they become similar enough for ImageDiff to consider them identical. > > > > Earlier today, Simon and I agreed that we should just silence the error > > message, because it only tells us about minor color differences that are > > inevitable when comparing compositing vs. non-compositing. Looks like it > > tells us about more actionable issues, too. Also, I just found > > <https://bugs.webkit.org/show_bug.cgi?id=69444 > > <https://bugs.webkit.org/show_bug.cgi?id=69444>>, and I think that its > > rationale applies in this case, too. > > > > So we should probably keep this error message. I'm not sure whether we > > should make it a hard error though. > > I think that to improve things we should make an informative message for this > particular mistake where the expected file has the wrong size or resolution. > > For me, the bad thing about the current message is not simply that it’s a > false positive, but that it’s an unclear error message that covers too many > different possibilities. > > I’m not sure that keeping things as is will be a good strategy. A message > that is often expected but sometimes indicates a real problem is not good for > the project. The average engineer has no idea whether to ignore these or act > on them!
Agreed. The size difference certainly needs to be a failure. > > We could add a new test expectation like ImageDiff to suppress these or we > could expose a new internals or testRunner methods to mark a test as such. I don’t think that addresses the issue. Darin’s point is that if a developer sees this output (possibly when running tests after making a change), it needs to describe something actionable. Just suppressing this output for existing tests doesn’t help when the output crops up unexpectedly. Simon
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
