On Thu, Jun 14, 2012 at 9:02 PM, Ojan Vafai <o...@chromium.org> wrote: > On Thu, Jun 14, 2012 at 9:00 PM, Adam Barth <aba...@webkit.org> wrote: >> >> On Thu, Jun 14, 2012 at 8:56 PM, Ojan Vafai <o...@chromium.org> wrote: >> > On Thu, Jun 14, 2012 at 4:34 PM, Dirk Pranke <dpra...@chromium.org> >> > wrote: >> >> On Thu, Jun 14, 2012 at 4:22 PM, Maciej Stachowiak <m...@apple.com> >> >> wrote: >> >> > On Jun 14, 2012, at 1:47 PM, Ryosuke Niwa <rn...@webkit.org> wrote: >> >> > On Thu, Jun 14, 2012 at 1:44 PM, Peter Kasting >> >> > <pkast...@chromium.org> >> >> > wrote: >> >> >> >> >> >> On Thu, Jun 14, 2012 at 1:39 PM, Elliot Poger <epo...@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 > > _______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev