Title: [235425] trunk/LayoutTests
Revision
235425
Author
simon.fra...@apple.com
Date
2018-08-28 08:51:55 -0700 (Tue, 28 Aug 2018)

Log Message

More results.html cleanup
https://bugs.webkit.org/show_bug.cgi?id=189038

Reviewed by Zalan Bujtas.

Use a map of table-id to SectionBuilderClass to drive the table builder class selection,
rather than hardcoding the builder class; this will allow for SectionBuilders to stay alive
longer in future, so they can be used to build the expanded state of each row.

Refactor the code that generates the expand link and test name, to de-duplicate some HTML strings,
and let SectionBuilders control whether their rows are expandable and test names linkifyable.

Put a "data-test-name" attribute on each row so we can easily map from HTML elements to
TestResults in future.

The test result change is a progression; there is nothing to show for a test with missing results,
so the row should not be expandable.

* fast/harness/results-expected.txt:
* fast/harness/results.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (235424 => 235425)


--- trunk/LayoutTests/ChangeLog	2018-08-28 15:45:32 UTC (rev 235424)
+++ trunk/LayoutTests/ChangeLog	2018-08-28 15:51:55 UTC (rev 235425)
@@ -1,3 +1,26 @@
+2018-08-28  Simon Fraser  <simon.fra...@apple.com>
+
+        More results.html cleanup
+        https://bugs.webkit.org/show_bug.cgi?id=189038
+
+        Reviewed by Zalan Bujtas.
+        
+        Use a map of table-id to SectionBuilderClass to drive the table builder class selection,
+        rather than hardcoding the builder class; this will allow for SectionBuilders to stay alive
+        longer in future, so they can be used to build the expanded state of each row.
+        
+        Refactor the code that generates the expand link and test name, to de-duplicate some HTML strings,
+        and let SectionBuilders control whether their rows are expandable and test names linkifyable.
+        
+        Put a "data-test-name" attribute on each row so we can easily map from HTML elements to
+        TestResults in future.
+        
+        The test result change is a progression; there is nothing to show for a test with missing results,
+        so the row should not be expandable.
+
+        * fast/harness/results-expected.txt:
+        * fast/harness/results.html:
+
 2018-08-27  Mark Lam  <mark....@apple.com>
 
         Fix exception throwing code so that topCallFrame and topEntryFrame stay true to their names.

Modified: trunk/LayoutTests/fast/harness/results-expected.txt (235424 => 235425)


--- trunk/LayoutTests/fast/harness/results-expected.txt	2018-08-28 15:45:32 UTC (rev 235424)
+++ trunk/LayoutTests/fast/harness/results-expected.txt	2018-08-28 15:51:55 UTC (rev 235425)
@@ -18,7 +18,7 @@
 Tests that had no expected results (probably new) (1): flag all
 
 test	results	image results	actual failure	expected failure	history
-+svg/batik/smallFonts.svg			missing		history
+svg/batik/smallFonts.svg			missing		history
 Tests that timed out (1): flag all
 
 +platform/mac/media/audio-session-category-video-paused.html	expected actual diff pretty diff	history

Modified: trunk/LayoutTests/fast/harness/results.html (235424 => 235425)


--- trunk/LayoutTests/fast/harness/results.html	2018-08-28 15:45:32 UTC (rev 235424)
+++ trunk/LayoutTests/fast/harness/results.html	2018-08-28 15:51:55 UTC (rev 235425)
@@ -444,6 +444,8 @@
         this.hasImageFailures = false;
         this.hasTextFailures = false;
 
+        this._testsByName = new Map;
+
         this._forEachTest(this._results.tests, '');
         this._forOtherCrashes(this._results.other_crashes);
     }
@@ -482,9 +484,16 @@
     {
         return this._results.has_wdiff;
     }
+    
+    testWithName(testName)
+    {
+        return this._testsByName.get(testName);
+    }
 
     _processResultForTest(testResult)
     {
+        this._testsByName.set(testResult.name, testResult);
+
         let test = testResult.name;
         if (testResult.hasStdErr())
             this.testsWithStderr.push(testResult);
@@ -589,28 +598,28 @@
         }
 
         if (this.testResults.crashTests.length)
-            this.containerElement.appendChild(this.buildOneSection(this.testResults.crashTests, CrashingTestsSectionBuilder));
+            this.containerElement.appendChild(this.buildOneSection(this.testResults.crashTests, 'crash-tests-table'));
 
         if (this.testResults.crashOther.length)
-            this.containerElement.appendChild(this.buildOneSection(this.testResults.crashOther, OtherCrashesSectionBuilder));
+            this.containerElement.appendChild(this.buildOneSection(this.testResults.crashOther, 'other-crash-tests-table'));
 
         if (this.testResults.failingTests.length)
-            this.containerElement.appendChild(this.buildOneSection(this.testResults.failingTests, FailingTestsSectionBuilder));
+            this.containerElement.appendChild(this.buildOneSection(this.testResults.failingTests, 'results-table'));
 
         if (this.testResults.missingResults.length)
-            this.containerElement.appendChild(this.buildOneSection(this.testResults.missingResults, TestsWithMissingResultsSectionBuilder));
+            this.containerElement.appendChild(this.buildOneSection(this.testResults.missingResults, 'missing-table'));
 
         if (this.testResults.timeoutTests.length)
-            this.containerElement.appendChild(this.buildOneSection(this.testResults.timeoutTests, TimedOutTestsSectionBuilder));
+            this.containerElement.appendChild(this.buildOneSection(this.testResults.timeoutTests, 'timeout-tests-table'));
 
         if (this.testResults.testsWithStderr.length)
-            this.containerElement.appendChild(this.buildOneSection(this.testResults.testsWithStderr, TestsWithStdErrSectionBuilder));
+            this.containerElement.appendChild(this.buildOneSection(this.testResults.testsWithStderr, 'stderr-table'));
 
         if (this.testResults.flakyPassTests.length)
-            this.containerElement.appendChild(this.buildOneSection(this.testResults.flakyPassTests, FlakyPassTestsSectionBuilder));
+            this.containerElement.appendChild(this.buildOneSection(this.testResults.flakyPassTests, 'flaky-tests-table'));
 
         if (this.testResults.usesExpectationsFile() && this.testResults.unexpectedPassTests.length)
-            this.containerElement.appendChild(this.buildOneSection(this.testResults.unexpectedPassTests, UnexpectedPassTestsSectionBuilder));
+            this.containerElement.appendChild(this.buildOneSection(this.testResults.unexpectedPassTests, 'passes-table'));
 
         if (this.testResults.hasHttpTests) {
             let httpdAccessLogLink = document.createElement('p');
@@ -625,6 +634,21 @@
         
         this.updateTestlistCounts();
     }
+
+    static sectionBuilderClassForTableID(tableID)
+    {
+        const idToBuilderClassMap = {
+            'crash-tests-table' : CrashingTestsSectionBuilder,
+            'other-crash-tests-table' : OtherCrashesSectionBuilder,
+            'results-table' : FailingTestsSectionBuilder,
+            'missing-table' : TestsWithMissingResultsSectionBuilder,
+            'timeout-tests-table' : TimedOutTestsSectionBuilder,
+            'stderr-table' : TestsWithStdErrSectionBuilder,
+            'flaky-tests-table' : FlakyPassTestsSectionBuilder,
+            'passes-table' : UnexpectedPassTestsSectionBuilder,
+        };
+        return idToBuilderClassMap[tableID];
+    }
     
     setupSorting()
     {
@@ -662,11 +686,12 @@
             Utils.parentOfType(document.getElementById('unexpected-results'), 'label').style.display = 'none';
     }
 
-    buildOneSection(tests, sectionBuilderClass)
+    buildOneSection(tests, tableID)
     {
         TestResults.sortByName(tests);
         
-        let sectionBuilder = new sectionBuilderClass(tests, this);
+        let sectionBuilderClass = TestResultsController.sectionBuilderClassForTableID(tableID);
+        let sectionBuilder = new sectionBuilderClass(tests, tableID, this);
         return sectionBuilder.build();
     }
 
@@ -789,6 +814,11 @@
         return '<a class=test-link _onclick_="controller.checkServerIsRunning(event)" href="" + this.layoutTestURL(testResult) + '">' + testResult.name + '</a><span class=flag _onclick_="controller.unflag(this)"> \u2691</span>';
     }
     
+    expandButtonSpan()
+    {
+        return '<span class=expand-button _onclick_="controller.toggleExpectations(this)"><span class=expand-button-text>+</span></span>';
+    }
+    
     static resultLink(testPrefix, suffix, contents)
     {
         return '<a class=result-link href="" + testPrefix + suffix + '" data-prefix="' + testPrefix + '">' + contents + '</a> ';
@@ -881,26 +911,11 @@
             this._updateExpandedState(existingResultsRow, true);
             return;
         }
-    
-        let newRow = document.createElement('tr');
-        newRow.className = 'results-row';
-        let newCell = document.createElement('td');
-        newCell.colSpan = row.querySelectorAll('td').length;
 
-        let resultLinks = row.querySelectorAll('.result-link');
-        let hasTogglingImages = false;
-        for (let link of resultLinks) {
-            let result;
-            if (link.textContent == 'images') {
-                hasTogglingImages = true;
-                result = TestResultsController._togglingImage(link.getAttribute('data-prefix'));
-            } else
-                result = TestResultsController._resultIframe(link.href);
+        let testName = row.getAttribute('data-test-name');
+        let testResult = this.testResults.testWithName(testName);
 
-            Utils.appendHTML(newCell, result);    
-        }
-
-        newRow.appendChild(newCell);
+        let newRow = TestResultsController._buildExpandedRowForTest(testResult, row);
         parentTbody.appendChild(newRow);
 
         this._updateExpandedState(newRow, true);
@@ -984,6 +999,31 @@
         testNavigator.onlyShowUnexpectedFailuresChanged();
     }
 
+    static _buildExpandedRowForTest(testResult, row)
+    {
+        let newRow = document.createElement('tr');
+        newRow.className = 'results-row';
+        let newCell = document.createElement('td');
+        newCell.colSpan = row.querySelectorAll('td').length;
+
+        // FIXME: migrate more of code to using testResult for building the expanded content.
+        let resultLinks = row.querySelectorAll('.result-link');
+        let hasTogglingImages = false;
+        for (let link of resultLinks) {
+            let result;
+            if (link.textContent == 'images') {
+                hasTogglingImages = true;
+                result = TestResultsController._togglingImage(link.getAttribute('data-prefix'));
+            } else
+                result = TestResultsController._resultIframe(link.href);
+
+            Utils.appendHTML(newCell, result);    
+        }
+
+        newRow.appendChild(newCell);
+        return newRow;
+    }
+
     static _resultIframe(src)
     {
         // FIXME: use audio tags for AUDIO tests?
@@ -1069,11 +1109,12 @@
 
 class SectionBuilder {
     
-    constructor(tests, resultsController)
+    constructor(tests, tableID, resultsController)
     {
         this._tests = tests;
         this._table = null;
         this._resultsController = resultsController;
+        this._tableID = tableID;
     }
 
     build()
@@ -1110,8 +1151,9 @@
             tbody.classList.add('expected');
         
         let row = document.createElement('tr');
+        row.setAttribute('data-test-name', testResult.name);
         tbody.appendChild(row);
-        
+
         let testNameCell = document.createElement('td');
         this.fillTestCell(testResult, testNameCell);
         row.appendChild(testNameCell);
@@ -1138,7 +1180,13 @@
     
     fillTestCell(testResult, cell)
     {
-        cell.innerHTML = '<span class=expand-button _onclick_="controller.toggleExpectations(this)"><span class=expand-button-text>+</span></span>' + this._resultsController.testLink(testResult);
+        let testLink = this.linkifyTestName() ? this._resultsController.testLink(testResult) : testResult.name;
+        if (this.rowsAreExpandable()) {
+            cell.innerHTML = this._resultsController.expandButtonSpan() + testLink;
+            return;
+        }
+
+        cell.innerHTML = testLink;
     }
 
     fillTestResultCell(testResult, cell)
@@ -1152,7 +1200,21 @@
         return historyCell;
     }
     
-    tableID() { return ''; }
+    tableID()
+    {
+        return this._tableID;
+    }
+    
+    rowsAreExpandable()
+    {
+        return true;
+    }
+    
+    linkifyTestName()
+    {
+        return true;
+    }
+
     sectionTitle() { return ''; }
 };
 
@@ -1185,6 +1247,7 @@
             tbody.setAttribute('mismatchreftest', 'true');
 
         let row = document.createElement('tr');
+        row.setAttribute('data-test-name', testResult.name);
         tbody.appendChild(row);
         
         let testNameCell = document.createElement('td');
@@ -1290,22 +1353,23 @@
 };
 
 class FailingTestsSectionBuilder extends FailuresSectionBuilder {
-    tableID() { return 'results-table'; }
     sectionTitle() { return 'Tests that failed text/pixel/audio diff'; }
 };
 
 class TestsWithMissingResultsSectionBuilder extends FailuresSectionBuilder {
-    tableID() { return 'missing-table'; }
     sectionTitle() { return 'Tests that had no expected results (probably new)'; }
+
+    rowsAreExpandable()
+    {
+        return false;
+    }
 };
 
 class FlakyPassTestsSectionBuilder extends FailuresSectionBuilder {
-    tableID() { return 'flaky-tests-table'; }
     sectionTitle() { return 'Flaky tests (failed the first run and passed on retry)'; }
 };
 
 class UnexpectedPassTestsSectionBuilder extends SectionBuilder {
-    tableID() { return 'passes-table'; }
     sectionTitle() { return 'Tests expected to fail but passed'; }
 
     addTableHeader()
@@ -1315,20 +1379,18 @@
         this._table.appendChild(header);
     }
     
-    fillTestCell(testResult, cell)
+    fillTestResultCell(testResult, cell)
     {
-        cell.innerHTML = '<a class=test-link _onclick_="controller.checkServerIsRunning(event)" href="" + this._resultsController.layoutTestURL(testResult) + '">' + testResult.name + '</a><span class=flag _onclick_="controller.unflag(this)"> \u2691</span>';
+        cell.innerHTML = testResult.info.expected;
     }
 
-    fillTestResultCell(testResult, cell)
+    rowsAreExpandable()
     {
-        cell.innerHTML = testResult.info.expected;
+        return false;
     }
 };
 
-
 class TestsWithStdErrSectionBuilder extends SectionBuilder {
-    tableID() { return 'stderr-table'; }
     sectionTitle() { return 'Tests that had stderr output'; }
     hideWhenShowingUnexpectedResultsOnly() { return false; }
 
@@ -1339,7 +1401,6 @@
 };
 
 class TimedOutTestsSectionBuilder extends SectionBuilder {
-    tableID() { return 'timeout-tests-table'; }
     sectionTitle() { return 'Tests that timed out'; }
 
     fillTestResultCell(testResult, cell)
@@ -1350,7 +1411,6 @@
 };
 
 class CrashingTestsSectionBuilder extends SectionBuilder {
-    tableID() { return 'crash-tests-table'; }
     sectionTitle() { return 'Tests that crashed'; }
 
     fillTestResultCell(testResult, cell)
@@ -1361,13 +1421,7 @@
 };
 
 class OtherCrashesSectionBuilder extends SectionBuilder {
-    tableID() { return 'other-crash-tests-table'; }
     sectionTitle() { return 'Other crashes'; }
-    fillTestCell(testResult, cell)
-    {
-        cell.innerHTML = '<span class=expand-button _onclick_="controller.toggleExpectations(this)"><span class=expand-button-text>+</span></span>' + testResult.name;
-    }
-
     fillTestResultCell(testResult, cell)
     {
         cell.innerHTML = TestResultsController.resultLink(Utils.stripExtension(testResult.name), '-crash-log.txt', 'crash log');
@@ -1377,6 +1431,11 @@
     {
         return null;
     }
+
+    linkifyTestName()
+    {
+        return false;
+    }
 };
 
 class PixelZoomer {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to