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

[Adam Barth]

On Thu, Jun 14, 2012 at 9:02 PM, Ojan Vafai <ojan at chromium.org> wrote:
> 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?

Maybe that can be solved with another name, like PixelOnlyFailure.

Adam


>> In this view, ImageOnlyFail is a special case for pixel tests because
>> they're so fragile.
>>
>> Adam
>
>