Bug 122232

Summary:

http://build.webkit.org/dashboard doesn't report failures for bindings tests, possibly more

Product:

WebKit

Reporter:

Alexey Proskuryakov <ap>

Component:

Tools / Tests

Assignee:

Timothy Hatcher <timothy>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, timothy

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

122283

Attachments:

Description	Flags
Patch	ap: review+, ap: commit-queue-

Alexey Proskuryakov

Reported 2013-10-02 13:22:46 PDT

This build is red because of bindings-generation-tests, but dashboard says that it's green: <http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK1%20%28Tests%29/builds/10527>. Long term direction is that we should unify most types of tests and have all issues reported by one script, but we are nowhere near that.

Attachments
Patch (8.95 KB, patch) 2013-10-02 15:04 PDT, Timothy Hatcher	ap: review+ ap: commit-queue-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Timothy Hatcher

Comment 1 2013-10-02 15:04:35 PDT

Created attachment 213204 [details] Patch

Alexey Proskuryakov

Comment 2 2013-10-02 15:14:54 PDT

Comment on attachment 213204 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213204&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTesterQueueView.js:70 > + var bindingTestResults = iteration.bindingTestResults || {failureCount: 0}; Hmm, shouldn't this be {errorOccurred: false}?

Timothy Hatcher

Comment 3 2013-10-04 22:29:06 PDT

Comment on attachment 213204 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213204&action=review >> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTesterQueueView.js:70 >> + var bindingTestResults = iteration.bindingTestResults || {failureCount: 0}; > > Hmm, shouldn't this be {errorOccurred: false}? No, this makes sure failureCount is 0 when adding totalFailures. Otherwise undefined causes a NaN total.

Alexey Proskuryakov

Comment 4 2013-10-04 22:37:14 PDT

Comment on attachment 213204 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213204&action=review >>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTesterQueueView.js:70 >>> + var bindingTestResults = iteration.bindingTestResults || {failureCount: 0}; >> >> Hmm, shouldn't this be {errorOccurred: false}? > > No, this makes sure failureCount is 0 when adding totalFailures. Otherwise undefined causes a NaN total. totalFailures uses errorOccurred: 88 var totalFailures = layoutTestResults.failureCount + javascriptTestResults.failureCount + pythonTestResults.failureCount + perlTestResults.failureCount + bindingTestResults.errorOccurred;

Timothy Hatcher

Comment 5 2013-10-04 22:41:51 PDT

Comment on attachment 213204 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213204&action=review >>>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTesterQueueView.js:70 >>>> + var bindingTestResults = iteration.bindingTestResults || {failureCount: 0}; >>> >>> Hmm, shouldn't this be {errorOccurred: false}? >> >> No, this makes sure failureCount is 0 when adding totalFailures. Otherwise undefined causes a NaN total. > > totalFailures uses errorOccurred: > > 88 var totalFailures = layoutTestResults.failureCount + javascriptTestResults.failureCount + pythonTestResults.failureCount + perlTestResults.failureCount + bindingTestResults.errorOccurred; Ah, yep! Fill fix. Good catch.

Timothy Hatcher

Comment 6 2013-10-07 11:51:07 PDT

http://trac.webkit.org/changeset/157052

Note You need to log in before you can comment on or make changes to this bug.