<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:ap@webkit.org" title="Alexey Proskuryakov <ap@webkit.org>"> <span class="fn">Alexey Proskuryakov</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - Display failing JSC stress tests in buildbot dashboard"
href="https://bugs.webkit.org/show_bug.cgi?id=156595">bug 156595</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #276428 Flags</td>
<td>review?
</td>
<td>review-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Display failing JSC stress tests in buildbot dashboard"
href="https://bugs.webkit.org/show_bug.cgi?id=156595#c4">Comment # 4</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Display failing JSC stress tests in buildbot dashboard"
href="https://bugs.webkit.org/show_bug.cgi?id=156595">bug 156595</a>
from <span class="vcard"><a class="email" href="mailto:ap@webkit.org" title="Alexey Proskuryakov <ap@webkit.org>"> <span class="fn">Alexey Proskuryakov</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=276428&action=diff" name="attach_276428" title="Patch">attachment 276428</a> <a href="attachment.cgi?id=276428&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=276428&action=review">https://bugs.webkit.org/attachment.cgi?id=276428&action=review</a>
Despite my many comments, this looks very good, almost ready for landing.
<span class="quote">> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotIteration.js:49
> + this.javaScriptCoreStressTestResults = null;</span >
Is "stress test" the correct name for this test suite? We simply execute run-javascriptcore-tests, there doesn't seem to be anything about "stress" there.
<span class="quote">> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTestResults.js:193
> + this.regressions = data.jscStressFailList;
> + console.assert(data.numJSCStressFailures === this.regressions.length);</span >
I think that before having this go live, we should take another look at JSON content, to make it better match WebKit naming conventions.
Would something like this make sense?
numJSCStressFailures -> failureCount
jscStressFailList -> failures
But also, why do we need a separate count when there it can be taken from the array?
<span class="quote">> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTesterQueueView.js:108
> + } else if (failedStep.name === "jscore-test") {</span >
As a separate patch, we should also teach the dashboard how to display failures in the popover when there are failures on multiple kinds.
<span class="quote">> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTesterQueueView.js:109
> + var status = new StatusLineView(messageElement, StatusLineView.Status.Bad, this._testStepFailureDescription(failedStep), failedStep.tooManyFailures ? failedStep.failureCount + "\uff0b" : failedStep.failureCount, iteration.queue.buildbot.javaScriptCoreStressTestFailuresURLForIteration(iteration));</span >
I don't think that JSC tests have the concept of "too many failures" at this time
<span class="quote">> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTesterQueueView.js:268
> + this._addIterationHeadingToPopover(iteration, content, "jscore stress test failures");</span >
Once again, unsure about the name.
I don't see any link from the popover to detailed test results, is there one? It's useful to have an easy way to see the actual output.
We'll probably need a different UI than for layout tests, because we don't have individual result files here.
<span class="quote">> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTesterQueueView.js:360
> + if (iteration.javaScriptCoreStressTestResults)
> + var content = this._popoverContentForJavaScriptCoreStressTestRegressions(iteration);
> + else {</span >
Is there anything here that could be shared rather than copy/pasted?</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>