Add a commit log viewer next to the analysis results viewer
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 20 May 2017 00:03:24 +0000 (00:03 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 20 May 2017 00:03:24 +0000 (00:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=172399

Reviewed by Chris Dumez.

Add a commit log viewer next to the analysis results viewer, which visualizes the A/B testing results.

Also linkify the revisions in the table that shows the status of each A/B testing job,
and allow the prefix of "r" when associating a Subversion revision.

Finally, Fixed a bug that the list of commits associated with the analysis task were not re-rendered
when the list was updated by the user.

* public/v3/components/analysis-results-viewer.js:
(AnalysisResultsViewer): Added _selectorRadioButtonList as an instance variable. It's a list of radio
buttons to select a configuration (A/B) with a commit set. It's added to update the checked status of
radio buttons upon changing the currently selected test group.
(AnalysisResultsViewer.prototype.setTestGroups): Update the selected range to that of the currently
selected group.
(AnalysisResultsViewer.prototype.render): Fill _selectorRadioButtonList with radio buttons.

* public/v3/components/commit-log-viewer.js:
(CommitLogViewer): Added _showRepositoryName as an instance variable.
(CommitLogViewer.prototype.setShowRepositoryName): Added.
(CommitLogViewer.prototype.render): Hide the repository name when _showRepositoryName is false. This
is used in the newly added commit log viewer for the analysis results since we're showing a select
element with all the names of repositories above this component.

* public/v3/components/test-group-revision-table.js:
(TestGroupRevisionTable.prototype._buildCommitCell): Linkify the revisions if possible.

* public/v3/models/analysis-task.js:
(AnalysisTask.prototype.associateCommit): Strip "r" at the beginning for a Subversion like r12345
since that's the format we use to show to the user. This makes copy & paste easier.

* public/v3/pages/analysis-task-page.js:
(AnalysisTaskResultsPane): Added a bunch of new instance variables to show and update the commit log
viewer next to the analysis results viewer.
(AnalysisTaskResultsPane.prototype.setPoints): Create the list of repositories to show details.
(AnalysisTaskResultsPane.prototype.didConstructShadowTree): Re-render when the current selected test
group changes since that may have updated the selected range for A/B testing. Also re-render when
a new repository is selected to show details.
(AnalysisTaskResultsPane.prototype.render): Update the list of repositories and the commit log viewer.
(AnalysisTaskResultsPane.prototype._renderRepositoryList): Renders the list of repositories.
(AnalysisTaskResultsPane.prototype._updateCommitViewer): Updates the commit log viewer given the range
selected in the analysis results viewer.
(AnalysisTaskResultsPane.htmlTemplate): Updated the template.
(AnalysisTaskResultsPane.cssTemplate): Ditto.
(AnalysisTaskTestGroupPane.cssTemplate): Add a little space between the list of results and the table
of A/B testing jobs with revisions.
(AnalysisTaskPage.prototype.render): Fixed the bug that the list of commits associated with the task
is not updated when the list changes the task or the start point never changed when the list of commits
associated with the task changed. Make the lazily evaluated function compare the actual list of commits
so that it will invoke _renderCauseAndFixes when the list changes.
(AnalysisTaskPage.prototype._renderCauseAndFixes): Now renders a specific list.

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/v3/components/analysis-results-viewer.js
Websites/perf.webkit.org/public/v3/components/commit-log-viewer.js
Websites/perf.webkit.org/public/v3/components/test-group-revision-table.js
Websites/perf.webkit.org/public/v3/models/analysis-task.js
Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js

index 575126560f13292325eae5ceb64888483681bd52..820866cd83e96a05b8d4ae3b1822aa78b8ef7c0e 100644 (file)
@@ -1,3 +1,61 @@
+2017-05-19  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Add a commit log viewer next to the analysis results viewer
+        https://bugs.webkit.org/show_bug.cgi?id=172399
+
+        Reviewed by Chris Dumez.
+
+        Add a commit log viewer next to the analysis results viewer, which visualizes the A/B testing results.
+
+        Also linkify the revisions in the table that shows the status of each A/B testing job,
+        and allow the prefix of "r" when associating a Subversion revision.
+
+        Finally, Fixed a bug that the list of commits associated with the analysis task were not re-rendered
+        when the list was updated by the user.
+
+        * public/v3/components/analysis-results-viewer.js:
+        (AnalysisResultsViewer): Added _selectorRadioButtonList as an instance variable. It's a list of radio
+        buttons to select a configuration (A/B) with a commit set. It's added to update the checked status of
+        radio buttons upon changing the currently selected test group.
+        (AnalysisResultsViewer.prototype.setTestGroups): Update the selected range to that of the currently
+        selected group.
+        (AnalysisResultsViewer.prototype.render): Fill _selectorRadioButtonList with radio buttons.
+
+        * public/v3/components/commit-log-viewer.js:
+        (CommitLogViewer): Added _showRepositoryName as an instance variable.
+        (CommitLogViewer.prototype.setShowRepositoryName): Added.
+        (CommitLogViewer.prototype.render): Hide the repository name when _showRepositoryName is false. This
+        is used in the newly added commit log viewer for the analysis results since we're showing a select
+        element with all the names of repositories above this component.
+
+        * public/v3/components/test-group-revision-table.js:
+        (TestGroupRevisionTable.prototype._buildCommitCell): Linkify the revisions if possible.
+
+        * public/v3/models/analysis-task.js:
+        (AnalysisTask.prototype.associateCommit): Strip "r" at the beginning for a Subversion like r12345
+        since that's the format we use to show to the user. This makes copy & paste easier.
+
+        * public/v3/pages/analysis-task-page.js:
+        (AnalysisTaskResultsPane): Added a bunch of new instance variables to show and update the commit log
+        viewer next to the analysis results viewer.
+        (AnalysisTaskResultsPane.prototype.setPoints): Create the list of repositories to show details.
+        (AnalysisTaskResultsPane.prototype.didConstructShadowTree): Re-render when the current selected test
+        group changes since that may have updated the selected range for A/B testing. Also re-render when
+        a new repository is selected to show details.
+        (AnalysisTaskResultsPane.prototype.render): Update the list of repositories and the commit log viewer.
+        (AnalysisTaskResultsPane.prototype._renderRepositoryList): Renders the list of repositories.
+        (AnalysisTaskResultsPane.prototype._updateCommitViewer): Updates the commit log viewer given the range
+        selected in the analysis results viewer.
+        (AnalysisTaskResultsPane.htmlTemplate): Updated the template.
+        (AnalysisTaskResultsPane.cssTemplate): Ditto.
+        (AnalysisTaskTestGroupPane.cssTemplate): Add a little space between the list of results and the table
+        of A/B testing jobs with revisions.
+        (AnalysisTaskPage.prototype.render): Fixed the bug that the list of commits associated with the task
+        is not updated when the list changes the task or the start point never changed when the list of commits
+        associated with the task changed. Make the lazily evaluated function compare the actual list of commits
+        so that it will invoke _renderCauseAndFixes when the list changes.
+        (AnalysisTaskPage.prototype._renderCauseAndFixes): Now renders a specific list.
+
 2017-05-16  Ryosuke Niwa  <rniwa@webkit.org>
 
         Another build fix. Added a missing null check.
index e3c4314b72c02806cd3a003ceb4a0c0dc05351df..d72c5e59f868589251052affa3e5aed57e825571 100644 (file)
@@ -12,6 +12,7 @@ class AnalysisResultsViewer extends ResultsTable {
         this._selectedRange = {};
         this._expandedPoints = new Set;
         this._groupToCellMap = new Map;
+        this._selectorRadioButtonList = {};
 
         this._renderTestGroupsLazily = new LazilyEvaluatedFunction(this.renderTestGroups.bind(this));
     }
@@ -34,6 +35,13 @@ class AnalysisResultsViewer extends ResultsTable {
     {
         this._testGroups = testGroups;
         this._currentTestGroup = currentTestGroup;
+        if (currentTestGroup && this._rangeSelectorLabels.length) {
+            const commitSets = currentTestGroup.requestedCommitSets();
+            this._selectedRange = {
+                [this._rangeSelectorLabels[0]]: commitSets[0],
+                [this._rangeSelectorLabels[1]]: commitSets[1]
+            };
+        }
         this.enqueueToRender();
     }
 
@@ -52,6 +60,15 @@ class AnalysisResultsViewer extends ResultsTable {
         this._renderTestGroupsLazily.evaluate(this._testGroups,
             this._startPoint, this._endPoint, this._metric, this._analysisResultsView, this._expandedPoints);
 
+        for (const label of this._rangeSelectorLabels) {
+            const commitSet = this._selectedRange[label];
+            const list = this._selectorRadioButtonList[label] || [];
+            for (const item of list) {
+                if (item.commitSet.equals(commitSet))
+                    item.radio.checked = true;
+            }
+        }
+
         const selectedCell = this.content().querySelector('td.selected');
         if (selectedCell)
             selectedCell.classList.remove('selected');
@@ -98,6 +115,9 @@ class AnalysisResultsViewer extends ResultsTable {
 
         const [additionalColumnsByRow, columnCount] = AnalysisResultsViewer._layoutBlocks(rows.length, testGroups.map((group) => testGroupLayoutMap.get(group)));
 
+        for (const label of this._rangeSelectorLabels)
+            this._selectorRadioButtonList[label] = [];
+
         const element = ComponentBase.createElement;
         const buildHeaders = (headers) => {
             return [
@@ -111,12 +131,12 @@ class AnalysisResultsViewer extends ResultsTable {
                 this._rangeSelectorLabels.map((label) => {
                     if (!row.commitSet())
                         return element('td', '');
-                    const checked = this._selectedRange[label] == row.commitSet();
-                    const onchange = () => {
+                    const radio = element('input', {type: 'radio', name: label, onchange: () => {
                         this._selectedRange[label] = row.commitSet();
                         this.dispatchAction('rangeSelectorClick', label, row);
-                    };
-                    return element('td', element('input', {type: 'radio', name: label, checked, onchange}));
+                    }});
+                    this._selectorRadioButtonList[label].push({radio, commitSet: row.commitSet()});
+                    return element('td', radio);
                 }),
                 columns,
                 additionalColumnsByRow[rowIndex],
index 81a6e164c971a601f13b206240cb90182e218b7d..4c76e21190bb289be29f49d7aab4b3942eb9315a 100644 (file)
@@ -9,6 +9,13 @@ class CommitLogViewer extends ComponentBase {
         this._fetchingPromise = null;
         this._commits = null;
         this._renderCommitListLazily = new LazilyEvaluatedFunction(this._renderCommitList.bind(this));
+        this._showRepositoryName = true;
+    }
+
+    setShowRepositoryName(shouldShow)
+    {
+        this._showRepositoryName = shouldShow;
+        this.enqueueToRender();
     }
 
     view(repository, precedingRevision, lastRevision)
@@ -55,7 +62,7 @@ class CommitLogViewer extends ComponentBase {
 
     render()
     {
-        const shouldShowRepositoryName = this._repository && (this._commits || this._fetchingPromise);
+        const shouldShowRepositoryName = this._repository && (this._commits || this._fetchingPromise) && this._showRepositoryName;
         this.content('repository-name').textContent = shouldShowRepositoryName ? this._repository.name() : '';
         this.content('spinner-container').style.display = this._fetchingPromise ? null : 'none';
         this._renderCommitListLazily.evaluate(this._commits);
index c759c9792de9444620a36e9607edbc5943b2d98e..8531dec9e358e670408ba11d846bc6e7fbed637d 100644 (file)
@@ -99,10 +99,19 @@ class TestGroupRevisionTable extends ComponentBase {
 
     _buildCommitCell(entry, repository)
     {
+        const element = ComponentBase.createElement;
+        const link = ComponentBase.createLink;
+
         if (entry.repositoriesToSkip.has(repository))
             return [];
         const commit = entry.commitSet.commitForRepository(repository);
-        return ComponentBase.createElement('td', {rowspan: entry.rowCountByRepository.get(repository)}, commit ? commit.label() : '');
+        let content = '';
+        if (commit) {
+            content = commit.label();
+            if (commit.url())
+                content = link(content, commit.url());
+        }
+        return element('td', {rowspan: entry.rowCountByRepository.get(repository)}, content);
     }
 
     _buildCustomRootsCell(entry)
index b293c4bcda325a6bbf23f6fe589fb9dd87b4b2e8..574fa7e9717173ed19149fd33e6d974a1c92b16d 100644 (file)
@@ -126,14 +126,16 @@ class AnalysisTask extends LabeledObject {
     {
         console.assert(kind == 'cause' || kind == 'fix');
         console.assert(repository instanceof Repository);
-        var id = this.id();
+        if (revision.startsWith('r'))
+            revision = revision.substring(1);
+        const id = this.id();
         return PrivilegedAPI.sendRequest('associate-commit', {
             task: id,
             repository: repository.id(),
             revision: revision,
             kind: kind,
-        }).then(function (data) {
-            return AnalysisTask.cachedFetch('/api/analysis-tasks', {id: id}, true)
+        }).then((data) => {
+            return AnalysisTask.cachedFetch('/api/analysis-tasks', {id}, true)
                 .then(AnalysisTask._constructAnalysisTasksFromRawData.bind(AnalysisTask));
         });
     }
index 44a92fb11976e477f46e6a3a199261b4b02bfda4..c3dc75404dc40193339cfe90ac65f34f4444c898 100644 (file)
@@ -61,11 +61,15 @@ class AnalysisTaskResultsPane extends ComponentBase {
     {
         super('analysis-task-results-pane');
         this._showForm = false;
+        this._repositoryList = [];
+        this._renderRepositoryListLazily = new LazilyEvaluatedFunction(this._renderRepositoryList.bind(this));
+        this._updateCommitViewerLazily = new LazilyEvaluatedFunction(this._updateCommitViewer.bind(this));
     }
 
     setPoints(startPoint, endPoint, metric)
     {
         const resultsViewer = this.part('results-viewer');
+        this._repositoryList = startPoint ? Repository.sortByNamePreferringOnesWithURL(startPoint.commitSet().repositories()) : [];
         resultsViewer.setPoints(startPoint, endPoint, metric);
         resultsViewer.enqueueToRender();
     }
@@ -91,10 +95,17 @@ class AnalysisTaskResultsPane extends ComponentBase {
     didConstructShadowTree()
     {
         const resultsViewer = this.part('results-viewer');
-        resultsViewer.listenToAction('testGroupClick', (testGroup) => this.dispatchAction('showTestGroup', testGroup));
+        resultsViewer.listenToAction('testGroupClick', (testGroup) => {
+            this.enqueueToRender();
+            this.dispatchAction('showTestGroup', testGroup)
+        });
         resultsViewer.setRangeSelectorLabels(['A', 'B']);
         resultsViewer.listenToAction('rangeSelectorClick', () => this.enqueueToRender());
 
+        const repositoryPicker = this.content('commit-viewer-repository');
+        repositoryPicker.addEventListener('change', () => this.enqueueToRender());
+        this.part('commit-viewer').setShowRepositoryName(false);
+
         this.part('form').listenToAction('startTesting', (repetitionCount, name, commitSetMap) => {
             this.dispatchAction('newTestGroup', name, repetitionCount, commitSetMap);
         });
@@ -102,7 +113,12 @@ class AnalysisTaskResultsPane extends ComponentBase {
 
     render()
     {
-        this.part('results-viewer').enqueueToRender();
+        const resultsViewer = this.part('results-viewer');
+
+        const repositoryPicker = this._renderRepositoryListLazily.evaluate(this._repositoryList);
+        const repository = Repository.findById(repositoryPicker.value);
+        const range = resultsViewer.selectedRange();
+        this._updateCommitViewerLazily.evaluate(repository, range['A'], range['B']);
 
         this.content('form').style.display = this._showForm ? null : 'none';
         if (!this._showForm)
@@ -116,14 +132,58 @@ class AnalysisTaskResultsPane extends ComponentBase {
         form.enqueueToRender();
     }
 
+    _renderRepositoryList(repositoryList)
+    {
+        const element = ComponentBase.createElement;
+        const selectElement = this.content('commit-viewer-repository');
+        this.renderReplace(selectElement,
+            repositoryList.map((repository) => {
+                return element('option', {value: repository.id()}, repository.label());
+            }));
+        return selectElement;
+    }
+
+    _updateCommitViewer(repository, preceedingCommitSet, lastCommitSet)
+    {
+        if (repository && preceedingCommitSet && lastCommitSet && !preceedingCommitSet.equals(lastCommitSet)) {
+            const precedingRevision = preceedingCommitSet.revisionForRepository(repository);
+            const lastRevision = lastCommitSet.revisionForRepository(repository);
+            if (precedingRevision && lastRevision && precedingRevision != lastRevision) {
+                this.part('commit-viewer').view(repository, precedingRevision, lastRevision);
+                return;
+            }
+        }
+        this.part('commit-viewer').view(null, null, null);
+    }
+
     static htmlTemplate()
     {
-        return `<analysis-results-viewer id="results-viewer"></analysis-results-viewer><customizable-test-group-form id="form"></customizable-test-group-form>`;
+        return `
+            <div id="results-container">
+                <analysis-results-viewer id="results-viewer"></analysis-results-viewer>
+                <div id="commit-pane">
+                    <select id="commit-viewer-repository"></select>
+                    <commit-log-viewer id="commit-viewer"></commit-log-viewer>
+                </div>
+            </div>
+            <customizable-test-group-form id="form"></customizable-test-group-form>
+        `;
     }
 
     static cssTemplate()
     {
         return `
+            #results-container {
+                position: relative;
+                text-align: center;
+            }
+            #commit-pane {
+                position: absolute;
+                width: 20rem;
+                height: 100%;
+                top: 0;
+                right: 0;
+            }
             #form {
                 margin: 0.5rem;
             }
@@ -359,7 +419,7 @@ class AnalysisTaskTestGroupPane extends ComponentBase {
             #test-group-details {
                 display: table-cell;
                 margin-bottom: 1rem;
-                padding: 0;
+                padding: 0 0.5rem;
                 margin: 0;
             }
 
@@ -564,7 +624,8 @@ class AnalysisTaskPage extends PageWithHeading {
         this.content().querySelector('.error-message').textContent = this._errorMessage || '';
 
         this._renderTaskNameAndStatusLazily.evaluate(this._task, this._task ? this._task.name() : null, this._task ? this._task.changeType() : null);
-        this._renderCauseAndFixesLazily.evaluate(this._startPoint, this._task);
+        this._renderCauseAndFixesLazily.evaluate(this._startPoint, this._task, this.part('cause-list'), this._task ? this._task.causes() : []);
+        this._renderCauseAndFixesLazily.evaluate(this._startPoint, this._task, this.part('fix-list'), this._task ? this._task.fixes() : []);
         this._renderRelatedTasksLazily.evaluate(this._task, this._relatedTasks);
 
         this.content('chart-pane').style.display = this._task && !this._task.isCustom() ? null : 'none';
@@ -607,7 +668,7 @@ class AnalysisTaskPage extends PageWithHeading {
             }));
     }
 
-    _renderCauseAndFixes(startPoint, task)
+    _renderCauseAndFixes(startPoint, task, list, commits)
     {
         const hasData = startPoint && task;
         this.content('cause-fix').style.display = hasData ? null : 'none';
@@ -622,13 +683,8 @@ class AnalysisTaskPage extends PageWithHeading {
                 'Disassociate this commit', this._dissociateCommit.bind(this, commit));
         }
 
-        const causeList = this.part('cause-list');
-        causeList.setKindList(repositoryList);
-        causeList.setList(task.causes().map((commit) => makeItem(commit)));
-
-        const fixList = this.part('fix-list');
-        fixList.setKindList(repositoryList);
-        fixList.setList(task.fixes().map((commit) => makeItem(commit)));
+        list.setKindList(repositoryList);
+        list.setList(commits.map((commit) => makeItem(commit)));
     }
 
     _showTestGroup(testGroup)