Switch results detail view over to new-style object-oriented UI widgets
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 14 Aug 2011 07:02:36 +0000 (07:02 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 14 Aug 2011 07:02:36 +0000 (07:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=66200

Reviewed by Dimitri Glazkov.

This patch replaces my goofy template-based UI for the results
comparison screen with new object-oriented UI widgets.

* BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/garden-o-matic.html:
* BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/main.css:
* BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/main.js:
* BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/run-unittests.html:
* BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/ui.js:
* BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/ui/results.js: Added.
* BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/ui_unittests.js:

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

Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/garden-o-matic.html
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/main.css
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/main.js
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/run-unittests.html
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/ui.js
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/ui/results.js [new file with mode: 0644]
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/ui_unittests.js
Tools/ChangeLog

index 98fdc49..beae3f4 100644 (file)
@@ -59,6 +59,7 @@ James, a web developer from Birmingham, UK.
 <script src="checkout.js"></script>
 <script src="results.js"></script>
 <script src="ui.js"></script>
+<script src="ui/results.js"></script>
 <script src="model.js"></script>
 <script src="main.js"></script>
 </body>
index 2aaedcb..600a0bd 100644 (file)
@@ -311,33 +311,18 @@ button.default:hover {
 }
 
 .results-detail .content {
-    height: 100%;
     background-color: white;
 }
 
-.results-detail .failure-details .missing-data {
-    font-style: italic;
-    text-align: center;
-    color: #555;
-}
-
-.results-detail .failure-details {
-    height: 100%;
-}
-
-.results-detail .failure-details iframe {
+.text-result {
     border: none;
     width: 100%;
-    height: 100%;
+    height: 400px; /* FIXME: How do we get a reasonable height here? */
 }
 
-.results-detail .failure-details img {
-  width: 100%;
-  height: auto;
-}
-
-.results-detail .failure-details td {
-    height: 100%;
+.image-result {
+    width: 100%;
+    height: auto;
 }
 
 /*** partytime ***/
index 0b7d385..828ef23 100644 (file)
@@ -118,7 +118,9 @@ function showResultsDetail(failureInfo)
 
         // FIXME: We should be passing the full list of failing builder names as the fourth argument.
         status.append(ui.failureDetailsStatus(failureInfo, [failureInfo.builderName]));
-        content.append(ui.failureDetails(resultsURLs));
+        var resultsGrid = new ui.results.ResultsGrid();
+        resultsGrid.addResults(resultsURLs);
+        content.append(resultsGrid);
 
         toggleButton($('.results-detail .actions .next'), g_resultsDetailsIterator.hasNext());
         toggleButton($('.results-detail .actions .previous'), g_resultsDetailsIterator.hasPrevious());
index 8db5f47..bfe69c5 100644 (file)
@@ -52,6 +52,7 @@ THE POSSIBILITY OF SUCH DAMAGE.
 <script src="results.js"></script>
 <script src="results_unittests.js"></script>
 <script src="ui.js"></script>
+<script src="ui/results.js"></script>
 <script src="ui_unittests.js"></script>
 <script src="model.js"></script>
 <script src="model_unittests.js"></script>
index 3425092..ac79211 100644 (file)
@@ -82,24 +82,6 @@ ui.failureDetailsStatus = function(failureInfo, builderNameList)
     return block;
 };
 
-ui.failureDetails = function(resultsURLs)
-{
-    var block = $('<table class="failure-details"><tbody><tr></tr></tbody></table>');
-
-    if (!resultsURLs.length)
-        $('tr', block).append($('<td><div class="missing-data">No data</div></td>'));
-
-    $.each(resultsURLs, function(index, resultURL) {
-        var kind = results.resultKind(resultURL);
-        var type = results.resultType(resultURL);
-        var fragment = (type == results.kImageType) ? '<img>' : '<iframe></iframe>';
-        var content = $(fragment).attr('src', resultURL).addClass(kind);
-        $('tr', block).append($('<td></td>').append(content));
-    });
-
-    return block;
-};
-
 function builderTableDataCells(resultNodesByBuilder)
 {
     if (!resultNodesByBuilder)
diff --git a/Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/ui/results.js b/Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/ui/results.js
new file mode 100644 (file)
index 0000000..ecebbd9
--- /dev/null
@@ -0,0 +1,114 @@
+var ui = ui || {};
+ui.results = ui.results || {};
+
+(function(){
+
+// FIXME: Rather than using table, should we be using something fancier?
+ui.results.Comparison = base.extends('table', {
+    init: function()
+    {
+        this.className = 'comparison';
+        this.innerHTML = '<thead><tr><th>Expected</th><th>Actual</th><th>Diff</th></tr></thead>' +
+                         '<tbody><tr><td class="expected"></td><td class="actual"></td><td class="diff"></td></tr></tbody>';
+    },
+    _selectorForKind: function(kind)
+    {
+        switch (kind) {
+        case results.kExpectedKind:
+            return '.expected';
+        case results.kActualKind:
+            return '.actual';
+        case results.kDiffKind:
+            return '.diff';
+        }
+        return '.unknown';
+    },
+    update: function(kind, result)
+    {
+        var selector = this._selectorForKind(kind);
+        $(selector, this).empty().append(result);
+        return result;
+    },
+});
+
+// We'd really like TextResult and ImageResult to extend a common Result base
+// class, but we can't seem to do that because they inherit from different
+// HTMLElements. We could have them inherit from <div>, but that seems lame.
+
+ui.results.TextResult = base.extends('iframe', {
+    init: function(url)
+    {
+        this.className = 'text-result';
+        this.src = url;
+    }
+});
+
+ui.results.ImageResult = base.extends('img', {
+    init: function(url)
+    {
+        this.className = 'image-result';
+        this.src = url;
+    }
+});
+
+function constructorForResultType(type)
+{
+    return (type == results.kImageType) ? ui.results.ImageResult : ui.results.TextResult;
+}
+
+ui.results.ResultsGrid = base.extends('div', {
+    init: function()
+    {
+        this.className = 'results-grid';
+    },
+    _addResult: function(comparison, constructor, resultsURLsByKind, kind)
+    {
+        var url = resultsURLsByKind[kind];
+        if (!url)
+            return;
+        comparison.update(kind, new constructor(url));
+    },
+    addComparison: function(resultType, resultsURLsByKind)
+    {
+        var comparison = new ui.results.Comparison();
+        var constructor = constructorForResultType(resultType);
+
+        this._addResult(comparison, constructor, resultsURLsByKind, results.kExpectedKind);
+        this._addResult(comparison, constructor, resultsURLsByKind, results.kActualKind);
+        this._addResult(comparison, constructor, resultsURLsByKind, results.kDiffKind);
+
+        this.appendChild(comparison);
+        return comparison;
+    },
+    addRow: function(resultType, url)
+    {
+        var constructor = constructorForResultType(resultType);
+        var view = new constructor(url);
+        this.appendChild(view);
+        return view;
+    },
+    addResults: function(resultsURLs)
+    {
+        var resultsURLsByTypeAndKind = {};
+
+        resultsURLsByTypeAndKind[results.kImageType] = {};
+        resultsURLsByTypeAndKind[results.kTextType] = {};
+
+        resultsURLs.forEach(function(url) {
+            resultsURLsByTypeAndKind[results.resultType(url)][results.resultKind(url)] = url;
+        });
+
+        $.each(resultsURLsByTypeAndKind, function(resultType, resultsURLsByKind) {
+            if ($.isEmptyObject(resultsURLsByKind))
+                return;
+            if (results.kUnknownKind in resultsURLsByKind) {
+                // This is something like "crash" that isn't a comparison.
+                this.addRow(resultType, resultsURLsByKind[results.kUnknownKind]);
+                return;
+            }
+            this.addComparison(resultType, resultsURLsByKind);
+        }.bind(this));
+    }
+});
+
+})();
index f87c4a2..5bed486 100644 (file)
@@ -73,40 +73,61 @@ test("failureDetailsStatus", 1, function() {
         '</span>');
 });
 
-test("failureDetails", 1, function() {
-    var testResults = ui.failureDetails([
+test("results.ResultsGrid", 1, function() {
+    var grid = new ui.results.ResultsGrid()
+    grid.addResults([
         'http://example.com/layout-test-results/foo-bar-diff.txt',
         'http://example.com/layout-test-results/foo-bar-expected.png',
         'http://example.com/layout-test-results/foo-bar-actual.png',
         'http://example.com/layout-test-results/foo-bar-diff.png',
     ]);
-    testResults.wrap('<wrapper></wrapper>');
-    equal(testResults.parent().html(),
-        '<table class="failure-details">' +
+    equal(grid.innerHTML,
+        '<table class="comparison">' +
+            '<thead>' +
+                '<tr>' +
+                    '<th>Expected</th>' +
+                    '<th>Actual</th>' +
+                    '<th>Diff</th>' +
+                '</tr>' +
+            '</thead>' +
             '<tbody>' +
                 '<tr>' +
-                    '<td><iframe src="http://example.com/layout-test-results/foo-bar-diff.txt" class="diff"></iframe></td>' +
-                    '<td><img src="http://example.com/layout-test-results/foo-bar-expected.png" class="expected"></td>' +
-                    '<td><img src="http://example.com/layout-test-results/foo-bar-actual.png" class="actual"></td>' +
-                    '<td><img src="http://example.com/layout-test-results/foo-bar-diff.png" class="diff"></td>' +
+                    '<td class="expected"><img class="image-result" src="http://example.com/layout-test-results/foo-bar-expected.png"></td>' +
+                    '<td class="actual"><img class="image-result" src="http://example.com/layout-test-results/foo-bar-actual.png"></td>' +
+                    '<td class="diff"><img class="image-result" src="http://example.com/layout-test-results/foo-bar-diff.png"></td>' +
                 '</tr>' +
             '</tbody>' +
-        '</table>');
-});
-
-test("failureDetails (empty)", 1, function() {
-    var testResults = ui.failureDetails([]);
-    testResults.wrap('<wrapper></wrapper>');
-    equal(testResults.parent().html(),
-        '<table class="failure-details">' +
+        '</table>' +
+        '<table class="comparison">' +
+            '<thead>' +
+                '<tr>' +
+                    '<th>Expected</th>' +
+                    '<th>Actual</th>' +
+                    '<th>Diff</th>' +
+                '</tr>' +
+            '</thead>' +
             '<tbody>' +
                 '<tr>' +
-                    '<td><div class="missing-data">No data</div></td>' +
+                    '<td class="expected"></td>' +
+                    '<td class="actual"></td>' +
+                    '<td class="diff"><iframe class="text-result" src="http://example.com/layout-test-results/foo-bar-diff.txt"></iframe></td>' +
                 '</tr>' +
             '</tbody>' +
         '</table>');
 });
 
+test("results.ResultsGrid (crashlog)", 1, function() {
+    var grid = new ui.results.ResultsGrid()
+    grid.addResults(['http://example.com/layout-test-results/foo-bar-crash-log.txt']);
+    equal(grid.innerHTML, '<iframe class="text-result" src="http://example.com/layout-test-results/foo-bar-crash-log.txt"></iframe>');
+});
+
+test("results.ResultsGrid (empty)", 1, function() {
+    var grid = new ui.results.ResultsGrid()
+    grid.addResults([]);
+    equal(grid.innerHTML, '');
+});
+
 test("summarizeFailure", 1, function() {
     var failureAnalysis = {
         "testName": "svg/dynamic-updates/SVGFETurbulenceElement-svgdom-baseFrequency-prop.html",
index 0bccf71..7ccbc55 100644 (file)
@@ -1,3 +1,21 @@
+2011-08-14  Adam Barth  <abarth@webkit.org>
+
+        Switch results detail view over to new-style object-oriented UI widgets
+        https://bugs.webkit.org/show_bug.cgi?id=66200
+
+        Reviewed by Dimitri Glazkov.
+
+        This patch replaces my goofy template-based UI for the results
+        comparison screen with new object-oriented UI widgets.
+
+        * BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/garden-o-matic.html:
+        * BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/main.css:
+        * BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/main.js:
+        * BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/run-unittests.html:
+        * BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/ui.js:
+        * BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/ui/results.js: Added.
+        * BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/ui_unittests.js:
+
 2011-08-13  Dimitri Glazkov  <dglazkov@chromium.org>
 
         garden-o-matic's analyzeUnexpectedFailures needs a completion callback.