Bug 104941 – Dashboard cleanup: remove usage of global g_builders.

• Component: New Bugs
• Status: RESOLVED FIXED

[Julie Parent]

Dashboard cleanup: remove usage of global g_builders.

[Julie Parent]

Created attachment 179320 [details]
Patch

[Julie Parent]

Comment on attachment 179320 [details]
Patch

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

> Tools/TestResultServer/static-dashboards/loader_unittests.js:57
> +test('results files loading', 11, function() {

Total exceptions is 1 + 2 x numBuilders.  So before it was 1 + 2 * 2 = 5, now 1 + 2 * 5 = 11, since we use an expanded builder list.

> Tools/TestResultServer/static-dashboards/loader_unittests.js:72
> +        var builderName = /builder=([\w ().]+)&/.exec(url)[1];

Now that we use real builder names, they can contain more than just alphanumerics, aka, "WebKit Linux (dbg)" has '(' and ')', and "WebKit Mac10.7" has '.'

[Dirk Pranke]

Comment on attachment 179320 [details]
Patch

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

> Tools/ChangeLog:12
> +        they were even bigger offenders of bad hygine, relying on global state

typo: hygiene.

> Tools/TestResultServer/static-dashboards/dashboard_base.js:191
> +            function() { return value in currentBuilderGroup().builders; });

I would probably add a currentBuilders() function that returns currentBuilderGroup().builders and use that below.

>> Tools/TestResultServer/static-dashboards/loader_unittests.js:57
>> +test('results files loading', 11, function() {
> 
> Total exceptions is 1 + 2 x numBuilders.  So before it was 1 + 2 * 2 = 5, now 1 + 2 * 5 = 11, since we use an expanded builder list.

It might be useful to note this in a comment.

[Dirk Pranke]

(looks fine otherwise).

[Dirk Pranke]

R+ if you agree w/ those changes.

[Julie Parent]

Created attachment 179324 [details]
Patch

[Julie Parent]

Addressed all comments.

[Julie Parent]

Just saw that some files were excluded from my change, fixing ...

[Dirk Pranke]

Comment on attachment 179324 [details]
Patch

thanks!

[Julie Parent]

Created attachment 179340 [details]
Patch

[Julie Parent]

Created attachment 179342 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 179342 [details]
Patch for landing

Rejecting attachment 179342 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

/mnt/git/webkit-commit-queue/Tools/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/15309423

[Julie Parent]

Created attachment 179345 [details]
Patch

[WebKit Review Bot]

Comment on attachment 179345 [details]
Patch

Clearing flags on attachment: 179345

Committed r137693: <http://trac.webkit.org/changeset/137693>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.