- Revision
- 139097
- Author
- jpar...@chromium.org
- Date
- 2013-01-08 12:33:08 -0800 (Tue, 08 Jan 2013)
Log Message
Dashboard cleanup: Remove globals g_buildersThatFailedToLoad and g_staleBuilders
https://bugs.webkit.org/show_bug.cgi?id=106356
g_buildersThatFailedToLoad and g_staleBuilders were globals defined in
dashboard_base, assigned by Loader, and used only by dashboard_base to
create error messages. Moved the variables to be privates on the Loader
object, moved error message creation to _getLoadingErrorMessages on the
Loader object, and now pass the errors back to dashboard base via the
resourceLoadingComplete callback.
Also removed the now unused clearError function, it was only being used
by unit tests to clean up global state.
Reviewed by Dirk Pranke.
* TestResultServer/static-dashboards/dashboard_base.js:
(resourceLoadingComplete):
* TestResultServer/static-dashboards/flakiness_dashboard_unittests.js:
* TestResultServer/static-dashboards/loader.js:
(.):
* TestResultServer/static-dashboards/loader_unittests.js:
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (139096 => 139097)
--- trunk/Tools/ChangeLog 2013-01-08 20:31:37 UTC (rev 139096)
+++ trunk/Tools/ChangeLog 2013-01-08 20:33:08 UTC (rev 139097)
@@ -1,3 +1,27 @@
+2013-01-08 Julie Parent <jpar...@chromium.org>
+
+ Dashboard cleanup: Remove globals g_buildersThatFailedToLoad and g_staleBuilders
+ https://bugs.webkit.org/show_bug.cgi?id=106356
+
+ g_buildersThatFailedToLoad and g_staleBuilders were globals defined in
+ dashboard_base, assigned by Loader, and used only by dashboard_base to
+ create error messages. Moved the variables to be privates on the Loader
+ object, moved error message creation to _getLoadingErrorMessages on the
+ Loader object, and now pass the errors back to dashboard base via the
+ resourceLoadingComplete callback.
+
+ Also removed the now unused clearError function, it was only being used
+ by unit tests to clean up global state.
+
+ Reviewed by Dirk Pranke.
+
+ * TestResultServer/static-dashboards/dashboard_base.js:
+ (resourceLoadingComplete):
+ * TestResultServer/static-dashboards/flakiness_dashboard_unittests.js:
+ * TestResultServer/static-dashboards/loader.js:
+ (.):
+ * TestResultServer/static-dashboards/loader_unittests.js:
+
2013-01-08 Zan Dobersek <zandober...@gmail.com>
[EFL][GTK] Make the PulseAudioSanitizer an object on the EflPort, GtkPort
Modified: trunk/Tools/TestResultServer/static-dashboards/dashboard_base.js (139096 => 139097)
--- trunk/Tools/TestResultServer/static-dashboards/dashboard_base.js 2013-01-08 20:31:37 UTC (rev 139096)
+++ trunk/Tools/TestResultServer/static-dashboards/dashboard_base.js 2013-01-08 20:33:08 UTC (rev 139097)
@@ -429,8 +429,6 @@
var g_resultsByBuilder = {};
var g_expectationsByPlatform = {};
-var g_staleBuilders = [];
-var g_buildersThatFailedToLoad = [];
// TODO(aboxhall): figure out whether this is a performance bottleneck and
// change calling code to understand the trie structure instead if necessary.
@@ -472,11 +470,6 @@
g_errorMessages += errorMsg + '<br>';
}
-// Clear out error and warning messages.
-function clearErrors()
-{
- g_errorMessages = '';
-}
// If there are errors, show big and red UI for errors so as to be noticed.
function showErrors()
@@ -499,19 +492,13 @@
errors.innerHTML = g_errorMessages;
}
-function addBuilderLoadErrors()
+function resourceLoadingComplete(errorMsgs)
{
- if (g_buildersThatFailedToLoad.length)
- addError('ERROR: Failed to get data from ' + g_buildersThatFailedToLoad.toString() + '.');
-
- if (g_staleBuilders.length)
- addError('ERROR: Data from ' + g_staleBuilders.toString() + ' is more than 1 day stale.');
-}
-
-function resourceLoadingComplete()
-{
g_resourceLoader = null;
- addBuilderLoadErrors();
+
+ if (errorMsgs)
+ addError(errorMsgs)
+
handleLocationChange();
}
Modified: trunk/Tools/TestResultServer/static-dashboards/flakiness_dashboard_unittests.js (139096 => 139097)
--- trunk/Tools/TestResultServer/static-dashboards/flakiness_dashboard_unittests.js 2013-01-08 20:31:37 UTC (rev 139096)
+++ trunk/Tools/TestResultServer/static-dashboards/flakiness_dashboard_unittests.js 2013-01-08 20:33:08 UTC (rev 139097)
@@ -597,13 +597,6 @@
deepEqual(diffStates(oldState, newState), {a: 1, b: 2});
});
-test('addBuilderLoadErrors', 1, function() {
- clearErrors();
- g_buildersThatFailedToLoad = ['builder1', 'builder2'];
- g_staleBuilders = ['staleBuilder1'];
- addBuilderLoadErrors();
- equal(g_errorMessages, 'ERROR: Failed to get data from builder1,builder2.<br>ERROR: Data from staleBuilder1 is more than 1 day stale.<br>');
-});
test('builderGroupIsToTWebKitAttribute', 2, function() {
var dummyMaster = new builders.BuilderMaster('Chromium', 'dummyurl', {'layout-tests': {'builders': ['WebKit Linux', 'WebKit Linux (dbg)', 'WebKit Mac10.7', 'WebKit Win']}});
Modified: trunk/Tools/TestResultServer/static-dashboards/loader.js (139096 => 139097)
--- trunk/Tools/TestResultServer/static-dashboards/loader.js 2013-01-08 20:31:37 UTC (rev 139096)
+++ trunk/Tools/TestResultServer/static-dashboards/loader.js 2013-01-08 20:33:08 UTC (rev 139097)
@@ -64,6 +64,9 @@
this._loadResultsFiles,
this._loadExpectationsFiles,
];
+
+ this._buildersThatFailedToLoad = [];
+ this._staleBuilders = [];
}
loader.Loader.prototype = {
@@ -75,7 +78,7 @@
{
var loadingStep = this._loadingSteps.shift();
if (!loadingStep) {
- resourceLoadingComplete();
+ resourceLoadingComplete(this._getLoadingErrorMessages());
return;
}
loadingStep.apply(this);
@@ -150,7 +153,7 @@
continue;
if ((Date.now() / 1000) - lastRunSeconds > ONE_DAY_SECONDS)
- g_staleBuilders.push(builderName);
+ this._staleBuilders.push(builderName);
if (json_version >= 4)
builds[builderName][TESTS_KEY] = flattenTrie(builds[builderName][TESTS_KEY]);
@@ -162,7 +165,7 @@
console.error('Failed to load results file for ' + builderName + '.');
// FIXME: loader shouldn't depend on state defined in dashboard_base.js.
- g_buildersThatFailedToLoad.push(builderName);
+ this._buildersThatFailedToLoad.push(builderName);
// Remove this builder from builders, so we don't try to use the
// data that isn't there.
@@ -227,6 +230,17 @@
partial(function(platformName, xhr) {
console.error('Could not load expectations file for ' + platformName);
}, platformWithExpectations));
+ },
+ _getLoadingErrorMessages: function()
+ {
+ var errorMsgs = '';
+ if (this._buildersThatFailedToLoad.length)
+ errorMsgs += 'ERROR: Failed to get data from ' + this._buildersThatFailedToLoad.toString() + '.<br>';
+
+ if (this._staleBuilders.length)
+ errorMsgs +='ERROR: Data from ' + this._staleBuilders.toString() + ' is more than 1 day stale.<br>';
+
+ return errorMsgs;
}
}
Modified: trunk/Tools/TestResultServer/static-dashboards/loader_unittests.js (139096 => 139097)
--- trunk/Tools/TestResultServer/static-dashboards/loader_unittests.js 2013-01-08 20:31:37 UTC (rev 139096)
+++ trunk/Tools/TestResultServer/static-dashboards/loader_unittests.js 2013-01-08 20:33:08 UTC (rev 139097)
@@ -113,9 +113,6 @@
resetGlobals();
loadBuildersList('@ToT - chromium.org', 'layout-tests');
- // FIXME: loader shouldn't depend on state defined in dashboard_base.js.
- g_buildersThatFailedToLoad = [];
-
var resourceLoader = new loader.Loader();
var resourceLoadCount = 0;
resourceLoader._handleResourceLoad = function() {
@@ -130,7 +127,7 @@
currentBuilders()[builder2] = true;
resourceLoader._handleResultsFileLoadError(builder2);
- deepEqual(g_buildersThatFailedToLoad, [builder1, builder2]);
+ deepEqual(resourceLoader._buildersThatFailedToLoad, [builder1, builder2]);
equal(resourceLoadCount, 2);
});
@@ -149,4 +146,11 @@
var newDefaultBuilder = currentBuilderGroup().defaultBuilder();
ok(newDefaultBuilder, "There should still be a default builder.");
notEqual(newDefaultBuilder, defaultBuilder, "Default builder should not be the old default builder");
+});
+
+test('addBuilderLoadErrors', 1, function() {
+ var resourceLoader = new loader.Loader();
+ resourceLoader._buildersThatFailedToLoad = ['builder1', 'builder2'];
+ resourceLoader._staleBuilders = ['staleBuilder1'];
+ equal(resourceLoader._getLoadingErrorMessages(), 'ERROR: Failed to get data from builder1,builder2.<br>ERROR: Data from staleBuilder1 is more than 1 day stale.<br>');
});
\ No newline at end of file
