[webkit-reviews] review denied: [Bug 43261] Move speech input layout tests to their own directory. : [Attachment 63090] Patch

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Satish Sampath
<satish at chromium.org>'s request for review:
Bug 43261: Move speech input layout tests to their own directory.
https://bugs.webkit.org/show_bug.cgi?id=43261

Attachment 63090: Patch
https://bugs.webkit.org/attachment.cgi?id=63090&action=review

------- Additional Comments from David Levin <levin at chromium.org>

> diff --git a/LayoutTests/platform/chromium/drt_expectations.txt
b/LayoutTests/platform/chromium/drt_expectations.txt
> index
735144ed754c4a48a0fe0bf9b796d68c459af9ad..cbf15ae99b529f2b11064d1d78fe1dd5d7516
b9d 100644
> --- a/LayoutTests/platform/chromium/drt_expectations.txt
> +++ b/LayoutTests/platform/chromium/drt_expectations.txt
> @@ -6,10 +6,8 @@
>  BUG_DRT : fast/backgrounds/animated-gif-as-background.html = IMAGE
>  
>  // Missing image resource
> -BUG_DRT MAC : fast/forms/input-appearance-searchandspeech.html = IMAGE
> -BUG_DRT MAC : fast/forms/input-appearance-speechbutton.html = IMAGE
> -BUG_DRT WIN LINUX : fast/forms/input-appearance-searchandspeech.html =
IMAGE+TEXT
> -BUG_DRT WIN LINUX : fast/forms/input-appearance-speechbutton.html =
IMAGE+TEXT
> +BUG_DRT : fast/speech/input-appearance-searchandspeech.html = IMAGE+TEXT
> +BUG_DRT : fast/speech/input-appearance-speechbutton.html = IMAGE+TEXT

I don't think this change is quite right.

It should be 

BUG_DRT : fast/speech/input-appearance-searchandspeech.html = IMAGE IMAGE+TEXT
BUG_DRT : fast/speech/input-appearance-speechbutton.html = IMAGE IMAGE+TEXT

but even then we will be listing failures for MAC, WIN, and LINUX that don't
occur on those platforms.

This would mean that the flakiness dashboard
(http://test-results.appspot.com/dashboards/flakiness_dashboard.html) would
show that there are extra modifiers for those platforms (IMAGE in WIN LINUX and
IMAGE+TEXT for MAC). This just leads to more noise in interpreting what is
going on and possible clean-up tasks for others as they may try to remove these
extra modifiers.

Lastly, it would hide if the test actually started failing differently on those
platforms (IMAGE+TEXT on MAC for instance), and this is part of the point of
modifiers as I understand it.


So I approved the previous patch which didn't have this issue.