Bug 84544 – [TestResultsServer] Impossible to inspect builds on non-Chromium builders

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Zan Dobersek]

In the timeline view on the http://test-results.appspot.com/ it is impossible to see details for any build when inspecting the layout tests results for non-Chromium builders.

TypeError occurs in dashboard_base.js when clicking on the dashboard with intent to inspect any build.
Stack trace obtained from the Web Inspector:

revisionLink (dashboard_base.js:954)
chromiumRevisionLink (dashboard_base.js:977)
updateBuildInspector (/dashboards/timeline_explorer.html#group=%40ToT%20-%20webkit.org&builder=Chromium%20Linux%20Release%20(Tests):300)
selectBuild (/dashboards/timeline_explorer.html#group=%40ToT%20-%20webkit.org&builder=Chromium%20Linux%20Release%20(Tests):248)
Dygraph.clickCallback (/dashboards/timeline_explorer.html#group=%40ToT%20-%20webkit.org&builder=Chromium%20Linux%20Release%20(Tests):224)
Dygraph.addEvent.o.dateWindow_ (dygraph-combined.js:1)
d (dygraph-combined.js:1)

It seems that in updateBuildInspector, Chromium revision link is being added despite not inspecting builds on a Chromium builder. shouldShowWebKitRevisionsOnly() returns false, and on further inspection it shows that isTipOfTreeWebKitBuilder() returns and undefined value which then gets converted to boolean value. Seems that isToTWebKit parameter of the BuilderGroup object is not properly assigned.

[Zan Dobersek]

Created attachment 138270 [details]
Patch

[Zan Dobersek]

CC-ing both author and reviewer from bug #82924 from which r112971 originates, introducing the problem.

[Ryosuke Niwa]

Comment on attachment 138270 [details]
Patch

Looks sane to me.

[WebKit Review Bot]

Comment on attachment 138270 [details]
Patch

Clearing flags on attachment: 138270

Committed r114868: <http://trac.webkit.org/changeset/114868>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ojan Vafai]

Comment on attachment 138270 [details]
Patch

Could you also add a test for this? The tests are in the poorly named flakiness_dashboard_tests.js. You run them by loading flakiness_dashboard.html#useTestData=true off your local filesystem.

[Zan Dobersek]

(In reply to comment #6)
> (From update of attachment 138270 [details])
> Could you also add a test for this? The tests are in the poorly named flakiness_dashboard_tests.js. You run them by loading flakiness_dashboard.html#useTestData=true off your local filesystem.

Could you please give some pointers on what exactly to test here? Simply the isToTWebKit attribute value that depends on whether BuilderGroup.TOT_WEBKIT or BuilderGroup.DEPS_WEBKIT is given as the first parameter to the BuilderGroup constructor or the complete builder lists loading, from loadBuildersList function in builders.js onwards, with the final isToTWebKit attribute value check?

[Ojan Vafai]

(In reply to comment #7)
> (In reply to comment #6)
> > (From update of attachment 138270 [details] [details])
> > Could you also add a test for this? The tests are in the poorly named flakiness_dashboard_tests.js. You run them by loading flakiness_dashboard.html#useTestData=true off your local filesystem.
> 
> Could you please give some pointers on what exactly to test here? Simply the isToTWebKit attribute value that depends on whether BuilderGroup.TOT_WEBKIT or BuilderGroup.DEPS_WEBKIT is given as the first parameter to the BuilderGroup constructor or the complete builder lists loading, from loadBuildersList function in builders.js onwards, with the final isToTWebKit attribute value check?

Yeah. The simple test was all I had in mind. Just call onBuilderListLoad with the right dummy arguments and check that builderGroups[groupName] has a BuilderGroup with the right isToTWebKit value.

[Zan Dobersek]

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > (From update of attachment 138270 [details] [details] [details])
> > > Could you also add a test for this? The tests are in the poorly named flakiness_dashboard_tests.js. You run them by loading flakiness_dashboard.html#useTestData=true off your local filesystem.
> > 
> > Could you please give some pointers on what exactly to test here? Simply the isToTWebKit attribute value that depends on whether BuilderGroup.TOT_WEBKIT or BuilderGroup.DEPS_WEBKIT is given as the first parameter to the BuilderGroup constructor or the complete builder lists loading, from loadBuildersList function in builders.js onwards, with the final isToTWebKit attribute value check?
> 
> Yeah. The simple test was all I had in mind. Just call onBuilderListLoad with the right dummy arguments and check that builderGroups[groupName] has a BuilderGroup with the right isToTWebKit value.

Patch coming up in #84713.