[webkit-dev] IMAGE+TEXT WAS: TestExpectations syntax changes, last call (for a while, at least) ...

[Ojan Vafai]

On Thu, Jun 14, 2012 at 9:00 PM, Adam Barth <abarth at webkit.org> wrote:

> On Thu, Jun 14, 2012 at 8:56 PM, Ojan Vafai <ojan at chromium.org> wrote:
> > On Thu, Jun 14, 2012 at 4:34 PM, Dirk Pranke <dpranke at chromium.org>
> wrote:
> >> On Thu, Jun 14, 2012 at 4:22 PM, Maciej Stachowiak <mjs at apple.com>
> wrote:
> >> > On Jun 14, 2012, at 1:47 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
> >> > On Thu, Jun 14, 2012 at 1:44 PM, Peter Kasting <pkasting at chromium.org
> >
> >> > wrote:
> >> >>
> >> >> On Thu, Jun 14, 2012 at 1:39 PM, Elliot Poger <epoger at chromium.org>
> >> >> wrote:
> >> >>>
> >> >>> Can someone please remind me why IMAGE+TEXT even exists?
> >> >>>
> >> >>> Wouldn't it be simpler to just mark a test as follows?
> >> >>>
> >> >>> IMAGE : allow image failure; go red if there is a text failure
> >> >>> TEXT: allow text failure; go red if there is an image failure
> >> >>> IMAGE TEXT: allow text and/or image failure
> >> >>
> >> >> The distinction is that IMAGE TEXT will allow image, text, or both to
> >> >> fail, thus making transitions among the three generate no events.
> >> >>  IMAGE+TEXT says specifically that we expect both to fail and that if
> >> >> one
> >> >> starts passing, someone should do something.  (For example, maybe
> >> >> someone
> >> >> checks in a partial rebaseline where they miss the image
> expectations.)
> >> >
> >> >
> >> > Not to bike-shed on anything, but I think we should rename Text and
> >> > Image to
> >> > TextOnly and ImageOnly. Every single person I know, including myself,
> >> > had
> >> > never got the distinction between IMAGE TEXT and IMAGE+TEXT without
> >> > someone
> >> > explaining it to him/her .
> >> >
> >> >
> >> > I think IMAGE+TEXT is not a very useful distinction from TEXT either.
> I
> >> > checked for uses of TEXT that is not IMAGE+TEXT in the Chromium
> >> > TextExpectations, and it seems that nearly all instances fall into one
> >> > of
> >> > the two following categories:
> >> >
> >> > 1) text-only test, so IMAGE+TEXT would not have different semantics
> from
> >> > TEXT (the vast majority)
> >> > 2) Flaky test that may actually pass, so distinguishing what happens
> >> > with
> >> > the image result is of limited utility (most of these are also
> text-only
> >> > tests; only a small subset even have an image result)
> >> >
> >> > Thus, I think Fail and ImageOnlyFail would be more useful and
> >> > understandable
> >> > categories than {TEXT, IMAGE, TEXT+IMAGE, TEXT IMAGE}. Fail would have
> >> > the
> >> > semantic that a text failure is expected, and image result if any can
> >> > either
> >> > pass or fail.
> >>
> >> This is perhaps true, but if it's okay I would like to treat that
> >> feature request separately from the other syntactic changes we've been
> >> discussing. So far the rest of the changes have not really implied any
> >> changes to how we actually track which changes fail and how (note that
> >> FAIL is different and we've fixed that separately from these changes
> >> as well).
> >
> >
> > Lets have the separate bikeshed. While this is less precise, I agree that
> > Fail and ImageOnlyFail would capture the vast majority use-case and
> remove a
> > frequent source of confusion and error. The big downside of this
> approach is
> > that a text-only failure that also starts failing the pixel result make
> > genuinely indicate a new bug. I think that happens rarely enough that
> I'm OK
> > with it for the added simplicity.
> >
> > A couple open questions:
> > -Does Fail also replace Audio? Seems reasonable to me.
>
> Yeah, audio tests can fail only in one way.
>
> > -What about reftest failures where there is no text comparison? I'd be
> fine
> > with saying you can do Fail or ImageOnlyFail and they mean the same thing
> > here.
>
> Similarly, I'd say that we should just Fail here.  Reftests can fail
> only in one way.
>

Seems like it will be a common error to mark a reftest failure as
ImageOnlyFail and then be confused why it's not working, no?


> In this view, ImageOnlyFail is a special case for pixel tests because
> they're so fragile.
>
> Adam
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120614/13bb9534/attachment.html>