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: http://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 5751265..820866c 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 e3c4314..d72c5e5 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 81a6e16..4c76e21 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 c759c97..8531dec 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 b293c4b..574fa7e 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 44a92fb..c3dc754 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)