The buildbot dashboard should be compatible with buildbot-0.8.9

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Dana Burkart]

The dashboard should be compatible with 0.8.9 as well as older versions of buildbot to support both internal and open source infrastructure. Specifically, the logic for retrieving revisions from the buildbot instance is broken when used against buildbot-0.8.9.

[Dana Burkart]

Created attachment 241879 [details]
Patch to make the dashboard compatible with 0.8.9 revision info

[WebKit Commit Bot]

Attachment 241879 [details] did not pass style-queue:


ERROR: Tools/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dana Burkart]

Created attachment 241880 [details]
Add bug to changelog entry

[Dana Burkart]

Created attachment 241883 [details]
Simplify previous patch to isolate the issue

[Mark Rowe (bdash)]

Comment on attachment 241883 [details]
Simplify previous patch to isolate the issue

View in context: https://bugs.webkit.org/attachment.cgi?id=241883&action=review

> Tools/ChangeLog:3
> +        Make the dashboard compatible with buildbot-0.8.9 revision info.

This doesn't really describe the issue since the dashboard was already compatible with v0.8.9. This is teaching it to cope with the fact that different masters may have different names for the codebases.

> Tools/ChangeLog:9
> +        (isMultiCodebaseGotRevisionProperty):

This is no longer modified in your patch.

> Tools/ChangeLog:10
> +        (parseRevisionProperty): Changed.

Needs a description of what's changed and why.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotIteration.js:104
> +    if (typeof value === "object") {

Shouldn't this check be: if (isMultiCodebaseGotRevisionProperty(property)) {

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotIteration.js:109
> +        if (key in value) {
> +            value = value[key];
> +        } else {
> +            value = value[fallbackKey];
> +        }

This could be:
value = value[key] || value[fallbackKey];

Or if you want to be paranoid about falsey values:

value = (key in value) ? value[key] : value[fallbackKey];

[Dana Burkart]

Created attachment 241894 [details]
Dashboard patch

Updated patch with Mark's suggestions.

[Mark Rowe (bdash)]

Comment on attachment 241894 [details]
Dashboard patch

View in context: https://bugs.webkit.org/attachment.cgi?id=241894&action=review

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotIteration.js:106
> +    if (isMultiCodebaseGotRevisionProperty(property)) {
> +        value = (key in value) ? value[key] : value[fallbackKey];
> +    }

The braces are not necessary around the body of this if statement since it's only a single line.

[Dana Burkart]

Committed in r176361.