Attachment 358168 Details for Bug 193064 – Patch

[patch] Patch

bug-193064-20190101001136.patch (text/plain), 8.25 KB, created by dewei_zhu on 2019-01-01 00:11:37 PST

(hide)

Description:

Filename:

MIME Type:

Creator: dewei_zhu

Created: 2019-01-01 00:11:37 PST

Size: 8.25 KB

patch

obsolete

>Subversion Revision: 239567
>diff --git a/Websites/perf.webkit.org/ChangeLog b/Websites/perf.webkit.org/ChangeLog
>index 0936909761a24f17dcf3aa9b7a42ee14d24be505..4c7fd28640043295b67777013cc2826ad4c8668c 100644
>--- a/Websites/perf.webkit.org/ChangeLog
>+++ b/Websites/perf.webkit.org/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-12-31  Dewei Zhu  <dewei_zhu@apple.com>
>+
>+        Test group results page from notification should not failed even for completed build type build requests.
>+        https://bugs.webkit.org/show_bug.cgi?id=193064
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Should show 'Build completed' or 'Build failed' for build type build requests.
>+
>+        * browser-tests/test-group-result-page-tests.js: Added a unit test to guard this bug.
>+        * tools/js/test-group-result-page.js: Show 'Build completed' or 'Build failed' for build type build requests.
>+
> 2018-12-14  Dewei Zhu  <dewei_zhu@apple.com>
> 
>         Extend commits table to contain testability information.
>diff --git a/Websites/perf.webkit.org/browser-tests/test-group-result-page-tests.js b/Websites/perf.webkit.org/browser-tests/test-group-result-page-tests.js
>index eea1e4b7a0702e221db6748f54e3d4606f029cf7..085e1b6b34dec5abd898413602d0d2f434adb4cd 100644
>--- a/Websites/perf.webkit.org/browser-tests/test-group-result-page-tests.js
>+++ b/Websites/perf.webkit.org/browser-tests/test-group-result-page-tests.js
>@@ -6,7 +6,7 @@ describe('TestGroupResultPage', () => {
>             'models/data-model.js', 'models/metric.js', '../shared/statistics.js',], 'TestGroupResultPage', 'Metric');
>     }
> 
>-    async function prepareTestGroupResultPage(context, resultA, resultB)
>+    async function prepareTestGroupResultPage(context, buildRequestsA, buildRequestsB)
>     {
>         const [TestGroupResultPage, Metric] = await importMarkupComponent(context);
> 
>@@ -20,15 +20,15 @@ describe('TestGroupResultPage', () => {
> 
>         const mockTestGroup = {
>             requestedCommitSets: () => ['A', 'B'],
>-            test: () => ({test: () => 'speeodmeter-2', name: () => 'speedometer-2'}),
>+            test: () => ({test: () => 'speedometer-2', name: () => 'speedometer-2'}),
>             labelForCommitSet: (commitSet) => commitSet,
>-            requestsForCommitSet: (commitSet) => ({'A': resultA, 'B': resultB}[commitSet]),
>+            requestsForCommitSet: (commitSet) => ({'A': buildRequestsA, 'B': buildRequestsB}[commitSet]),
>             compareTestResults: (...args) => ({isStatisticallySignificant: true, changeType: 'worse'}),
>             name: () => 'mock-test-group',
>         };
> 
>         const mockAnalysisResults = {
>-            viewForMetric: (metric) => ({resultForRequest: (buildRequest) => (buildRequest === null ? null : {value: buildRequest})})
>+            viewForMetric: (metric) => ({resultForRequest: (buildRequest) => (buildRequest.value === null ? null : {value: buildRequest.value})})
>         };
> 
>         const page = new TestGroupResultPage('test');
>@@ -40,9 +40,19 @@ describe('TestGroupResultPage', () => {
>         return page;
>     }
> 
>+    function prepareBuildRequests(buildRequestValues, isTestBooleanList, hasCompletedBooleanList)
>+    {
>+        return Array.from(buildRequestValues.entries()).map((entry) => {
>+            const [index, value] = entry;
>+            const isTest = isTestBooleanList ? isTestBooleanList[index] : true;
>+            const hasCompleted = hasCompletedBooleanList ? hasCompletedBooleanList[index] : true;
>+            return {value, isTest: () => isTest, isBuild: () => !isTest, hasCompleted: () => hasCompleted};
>+        });
>+    }
>+
>     it('should render failed test group with empty bar', async () => {
>         const context = new BrowsingContext();
>-        const page = await prepareTestGroupResultPage(context, [null, 3, 5], [2, 4, 6]);
>+        const page = await prepareTestGroupResultPage(context, prepareBuildRequests([null, 3, 5]), prepareBuildRequests([2, 4, 6]));
>         await page.enqueueToRender();
>         const document = context.document;
>         document.open();
>@@ -56,7 +66,7 @@ describe('TestGroupResultPage', () => {
>         const context = new BrowsingContext();
>         const resultA = [1, 3, 5];
>         const resultB = [2, 4, 6];
>-        const page = await prepareTestGroupResultPage(context, resultA, resultB);
>+        const page = await prepareTestGroupResultPage(context, prepareBuildRequests(resultA), prepareBuildRequests(resultB));
>         page.enqueueToRender();
>         const document = context.document;
>         document.open();
>@@ -77,4 +87,27 @@ describe('TestGroupResultPage', () => {
>             previousNodeWidth = currentNodeWidth;
>         }
>     });
>+
>+    it('should not render rows for build requests those are not test', async () => {
>+        const context = new BrowsingContext();
>+        const resultA = [null, 1, 3, 5];
>+        const isTestBooleanListA = [false, true, true, true];
>+        const isBuildBooleanListA = [true, true, true, true];
>+        const resultB = [null, 2, 4, 6];
>+        const isTestBooleanListB = [false, true, true, true];
>+        const isBuildBooleanListB = [false, true, true, true];
>+        const page = await prepareTestGroupResultPage(context,
>+            prepareBuildRequests(resultA, isTestBooleanListA, isBuildBooleanListA),
>+            prepareBuildRequests(resultB, isTestBooleanListB, isBuildBooleanListB));
>+        page.enqueueToRender();
>+        const document = context.document;
>+        document.open();
>+        document.write(page.generateMarkup());
>+        document.close();
>+
>+        const barNodes = context.document.querySelectorAll('.result-cell');
>+        expect(barNodes.length).to.be(8);
>+        expect(barNodes[0].textContent).to.be('Build completed');
>+        expect(barNodes[4].textContent).to.be('Build failed');
>+    });
> });
>\ No newline at end of file
>diff --git a/Websites/perf.webkit.org/tools/js/test-group-result-page.js b/Websites/perf.webkit.org/tools/js/test-group-result-page.js
>index 9af21f14ea8d86a08c863102e74df93e6fd52ac0..7fe25ec85d4558f701067c7065668e552a17927b 100644
>--- a/Websites/perf.webkit.org/tools/js/test-group-result-page.js
>+++ b/Websites/perf.webkit.org/tools/js/test-group-result-page.js
>@@ -31,10 +31,14 @@ class TestGroupResultPage extends MarkupPage {
>         for (const commitSet of testGroup.requestedCommitSets())
>         {
>             const buildRequestsForCommitSet = testGroup.requestsForCommitSet(commitSet);
>-            const results = buildRequestsForCommitSet.map((buildRequest) => analysisResultsView.resultForRequest(buildRequest));
>+            const buildTypeRequests = buildRequestsForCommitSet.filter((buildRequest) => buildRequest.isBuild());
>+            const testTypeRequests = buildRequestsForCommitSet.filter((buildRequest) => buildRequest.isTest());
>+
>+            const buildTypeResults = buildTypeRequests.map((buildRequest) => ({isBuild: true, hasCompleted: buildRequest.hasCompleted()}));
>+            const results = [...buildTypeResults, ...testTypeRequests.map((buildRequest) => analysisResultsView.resultForRequest(buildRequest))];
>             resultsByCommitSet.set(commitSet, results);
>             for (const result of results) {
>-                if (!result)
>+                if (!result || result.isBuild)
>                     continue;
>                 maxValue = Math.max(maxValue, result.value);
>                 minValue = Math.min(minValue, result.value);
>@@ -100,8 +104,16 @@ class TestGroupResultPage extends MarkupPage {
>             const label = testGroup.labelForCommitSet(commitSet);
> 
>             for (const result of results) {
>-                const cellValue = result ? formatValue(result.value, result.interval).join('') : 'Failed';
>-                const barWidth = result ? widthForValue(result.value) : 0;
>+                let cellValue = null;
>+                let barWidth = 0;
>+                if (!result)
>+                    cellValue = 'failed';
>+                else if (result.isBuild)
>+                    cellValue = 'Build ' + (result.hasCompleted ? 'completed' : 'failed');
>+                else {
>+                    cellValue = formatValue(result.value, result.interval).join('');
>+                    barWidth = widthForValue(result.value);
>+                }
>                 tableRows.push(this._constructTableRow(cellValue, barWidth, firstRow, results.length, label, averageColumnContents));
>                 firstRow = false;
>             }

Flags:

rniwa: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193064: 358168