RESOLVED FIXED 64619
TestFailures page thinks all tests passed in http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20(WebKit2%20Tests)/builds/13401 
https://bugs.webkit.org/show_bug.cgi?id=64619
Summary TestFailures page thinks all tests passed in http://build.webkit.org/builders...
Adam Roben (:aroben)
Reported 2011-07-15 13:10:19 PDT
TestFailures page thinks all tests passed in http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20(WebKit2%20Tests)/builds/13401
Attachments
Patch (4.91 KB, patch)
2011-07-15 13:10 PDT, Adam Roben (:aroben) 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Adam Roben (:aroben)
Comment 1 2011-07-15 13:10:43 PDT
Created attachment 101029 [details] Patch
Daniel Bates
Comment 2 2011-07-15 13:36:17 PDT
Comment on attachment 101029 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=101029&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/Builder.js:138 > + if (layoutTestStep.results[0] === 4) { I wish we could make this less magical. Maybe it would make things a bit clearer if results was a dictionary instead of an array so that we can refer to its values using keys instead of indices. Also, defining a named constant for 4 may help as well.
Daniel Bates
Comment 3 2011-07-15 13:38:42 PDT
(In reply to comment #2) > (From update of attachment 101029 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=101029&action=review > > > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/Builder.js:138 > > + if (layoutTestStep.results[0] === 4) { > > I wish we could make this less magical. Maybe it would make things a bit clearer if results was a dictionary instead of an array so that we can refer to its values using keys instead of indices. Also, defining a named constant for 4 may help as well. Feel free to land this as-is. If you were to choose to clean this up then it can be done in a follow up.
Adam Roben (:aroben)
Comment 4 2011-07-15 13:43:54 PDT
Comment on attachment 101029 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=101029&action=review >>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/Builder.js:138 >>> + if (layoutTestStep.results[0] === 4) { >> >> I wish we could make this less magical. Maybe it would make things a bit clearer if results was a dictionary instead of an array so that we can refer to its values using keys instead of indices. Also, defining a named constant for 4 may help as well. > > Feel free to land this as-is. If you were to choose to clean this up then it can be done in a follow up. This data structure is coming straight from buildbot's JSON API. But we could munge it to try to make it clearer. We can definitely use constants for return codes. I'll do that in a followup.
WebKit Review Bot
Comment 5 2011-07-15 15:42:26 PDT
Comment on attachment 101029 [details] Patch Clearing flags on attachment: 101029 Committed r91123: <http://trac.webkit.org/changeset/91123>
WebKit Review Bot
Comment 6 2011-07-15 15:42:30 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.