[webkit-reviews] review granted: [Bug 61061] TestFailures page blames arbitrary revisions for breaking flaky tests : [Attachment 99100] Teach TestFailures to detect possibly flaky tests and list them separately

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Adam Roben (:aroben)
<aroben at apple.com>'s request for review:
Bug 61061: TestFailures page blames arbitrary revisions for breaking flaky
tests
https://bugs.webkit.org/show_bug.cgi?id=61061

Attachment 99100: Teach TestFailures to detect possibly flaky tests and list
them separately
https://bugs.webkit.org/attachment.cgi?id=99100&action=review

------- Additional Comments from Daniel Bates <dbates at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=99100&action=review

This looks sane to me.

>
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/Layout
TestHistoryAnalyzer.js:36
>	* Preiodically calls callback until all current failures have been
explained. Callback is

Preiodically => Periodically

(Just thought to mention the misspelling even though this text isn't part of
the actual changes)

>
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/Layout
TestHistoryAnalyzer.js:66
>	* Each build contains just the failures that a) are still occuring on
the bots, and b) were new

occuring => occurring

(Just thought to mention the misspelling even though this text isn't part of
the actual changes)

>
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/Layout
TestHistoryAnalyzer.js:133
> +	       const minimumRequiredTestRunsWithoutInterestingChanges = 5;

Note: IE doesn't support const (*). I take it we don't expect to access this
web app from IE.

(*) The decision against const support in IE9 was remarked in
<http://blogs.msdn.com/b/ie/archive/2010/08/25/chakra-interoperability-means-mo
re-than-just-standards.aspx>.