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

[Adam Barth]

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.

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

Adam