[Webkit-unassigned] [Bug 156595] Display failing JSC stress tests in buildbot dashboard

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=156595

--- Comment #5 from Sri Vijayaraghavan <svijayaraghavan at apple.com> ---
(In reply to comment #4)
> Comment on attachment 276428 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=276428&action=review
> 
> Despite my many comments, this looks very good, almost ready for landing.
> 
> > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotIteration.js:49
> > +    this.javaScriptCoreStressTestResults = null;
> 
> 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.
Only stress test info is sent to BuildBot right now. 

I can modify the run-javascriptcore-tests script to include API test failures and build failures as well.

> > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTestResults.js:193
> > +        this.regressions = data.jscStressFailList;
> > +        console.assert(data.numJSCStressFailures === this.regressions.length);
> 
> 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?
Count is redundant, I'll get rid of it. The JSON might further change once I add the non-stress JSC results as well.

> > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTesterQueueView.js:108
> > +                    } else if (failedStep.name === "jscore-test") {
> 
> As a separate patch, we should also teach the dashboard how to display
> failures in the popover when there are failures on multiple kinds.
OK

> > 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));
> 
> I don't think that JSC tests have the concept of "too many failures" at this
> time
Will be removed, thanks!

> > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTesterQueueView.js:268
> > +        this._addIterationHeadingToPopover(iteration, content, "jscore stress test failures");
> 
> 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.
I'll change the text.

There's no link to the JSC test stdio (the heading does include a link to the build page though). It would be trivial to add, but we need to figure out a good place in the UI for it. Perhaps to the right of the build link?

> > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTesterQueueView.js:360
> > +        if (iteration.javaScriptCoreStressTestResults)
> > +            var content = this._popoverContentForJavaScriptCoreStressTestRegressions(iteration);
> > +        else {
> 
> Is there anything here that could be shared rather than copy/pasted?
Do you mean a reusable function that displays a loading indicator until content is loaded?

I would like to make server-side changes (i.e. run-javascriptcore-tests JSON content changes, including non-stress failures, plus the buildbot change to make it live) first. I've been resorting to weird hackery to check if the front-end changes work, but it will be much faster if the back-end works.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160419/09c5dd03/attachment-0001.html>