Dashboard cleanup: Remove globals g_buildersThatFailedToLoad and g_staleBuilders
authorjparent@chromium.org <jparent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Jan 2013 20:33:08 +0000 (20:33 +0000)
committerjparent@chromium.org <jparent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Jan 2013 20:33:08 +0000 (20:33 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@139097 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/TestResultServer/static-dashboards/dashboard_base.js
Tools/TestResultServer/static-dashboards/flakiness_dashboard_unittests.js
Tools/TestResultServer/static-dashboards/loader.js
Tools/TestResultServer/static-dashboards/loader_unittests.js

index 745b3b40ce25fe6370397d740c5c783d5a2a05b8..8ba7171bd566f299830535881c27baa22946ff1f 100644 (file)
@@ -1,3 +1,27 @@
+2013-01-08  Julie Parent  <jparent@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  <zandobersek@gmail.com>
 
         [EFL][GTK] Make the PulseAudioSanitizer an object on the EflPort, GtkPort
index 2420bb1a6ed73775b273f18d88fd873745491f68..9f85c30ea533adad553d8f1b825e0a85fc070f9a 100644 (file)
@@ -429,8 +429,6 @@ function isTipOfTreeWebKitBuilder()
 
 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 @@ function addError(errorMsg)
     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 @@ function showErrors()
     errors.innerHTML = g_errorMessages;
 }
 
-function addBuilderLoadErrors()
-{
-    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()
+function resourceLoadingComplete(errorMsgs)
 {
     g_resourceLoader = null;
-    addBuilderLoadErrors();
+    
+    if (errorMsgs)
+        addError(errorMsgs)
+
     handleLocationChange();
 }
 
index 9b168f285cc1a01acef80e6ce091854e849606f5..b572c7a2c7497b8fb84e0a5e26a1930016c2783d 100644 (file)
@@ -597,13 +597,6 @@ test('diffStates', 5, function() {
     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']}});
index 849c9c9ae9074b6ddac6601d3bc9cb2e958bc403..ed3c4f7898aeedb874dc8226f407fb04bc63e3fc 100644 (file)
@@ -64,6 +64,9 @@ loader.Loader = function()
         this._loadResultsFiles,
         this._loadExpectationsFiles,
     ];
+
+    this._buildersThatFailedToLoad = [];
+    this._staleBuilders = [];
 }
 
 loader.Loader.prototype = {
@@ -75,7 +78,7 @@ loader.Loader.prototype = {
     {
         var loadingStep = this._loadingSteps.shift();
         if (!loadingStep) {
-            resourceLoadingComplete();
+            resourceLoadingComplete(this._getLoadingErrorMessages());
             return;
         }
         loadingStep.apply(this);
@@ -150,7 +153,7 @@ loader.Loader.prototype = {
                 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 @@ loader.Loader.prototype = {
         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 @@ loader.Loader.prototype = {
                     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;
     }
 }
 
index cb5b78b6ff3b4c437d9b43bf8c992176d3f47b63..86d7beec1bc7a24fa038802d83dae957fbc63348 100644 (file)
@@ -113,9 +113,6 @@ test('results file failing to load', 2, function() {
     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 @@ test('results file failing to load', 2, function() {
     currentBuilders()[builder2] = true;
     resourceLoader._handleResultsFileLoadError(builder2);
 
-    deepEqual(g_buildersThatFailedToLoad, [builder1, builder2]);
+    deepEqual(resourceLoader._buildersThatFailedToLoad, [builder1, builder2]);
     equal(resourceLoadCount, 2);
 
 });
@@ -149,4 +146,11 @@ test('Default builder gets set.', 3, function() {
     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