Attachment 358009 Details for Bug 192972 – Patch

[patch] Patch

bug-192972-20181221181335.patch (text/plain), 22.43 KB, created by dewei_zhu on 2018-12-21 18:13:36 PST

(hide)

Description:

Filename:

MIME Type:

Creator: dewei_zhu

Created: 2018-12-21 18:13:36 PST

Size: 22.43 KB

patch

obsolete

>Subversion Revision: 239485
>diff --git a/Websites/perf.webkit.org/ChangeLog b/Websites/perf.webkit.org/ChangeLog
>index 0936909761a24f17dcf3aa9b7a42ee14d24be505..2a29cbd61d22b5e672daf068825ce76151bdd94a 100644
>--- a/Websites/perf.webkit.org/ChangeLog
>+++ b/Websites/perf.webkit.org/ChangeLog
>@@ -1,3 +1,42 @@
>+2018-12-21  Dewei Zhu  <dewei_zhu@apple.com>
>+
>+        Add UI in analysis task page to show commit testability infomation.
>+        https://bugs.webkit.org/show_bug.cgi?id=192972
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add UI in custom analysis task configuration and customizable test group form to
>+        show testability information.
>+        Fix a bug in 'CustomAnalysisTaskConfigurator._updateCommitSetMap' that 'currentComparison' is incorrectly set.
>+
>+        * browser-tests/custom-analysis-task-configurator-tests.js: Added a unit test for the bug in
>+        'CustomAnalysisTaskConfigurator._updateCommitSetMap'.
>+        * browser-tests/index.html: Imported ''custom-analysis-task-configurator-tests.js'.
>+        * public/v3/components/custom-analysis-task-configurator.js: Added UI to show testability information.
>+        (CustomAnalysisTaskConfigurator):
>+        (CustomAnalysisTaskConfigurator.prototype._didUpdateSelectedPlatforms): Should reset commit set map related feild.
>+        (CustomAnalysisTaskConfigurator.prototype.render):
>+        (CustomAnalysisTaskConfigurator.prototype._updateCommitSetMap): Fixed a bug that 'currentComparison' is incorrectly set.
>+        (CustomAnalysisTaskConfigurator.prototype._computeCommitSet):
>+        (CustomAnalysisTaskConfigurator.prototype.async._fetchCommitsForConfiguration):
>+        (CustomAnalysisTaskConfigurator.prototype.async._resolveRevision):
>+        (CustomAnalysisTaskConfigurator.prototype._buildRevisionTable):
>+        (CustomAnalysisTaskConfigurator.prototype._buildTestabilityList):
>+        (CustomAnalysisTaskConfigurator.prototype._selectRepositoryGroup):
>+        (CustomAnalysisTaskConfigurator.prototype._buildRevisionInput):
>+        (CustomAnalysisTaskConfigurator.cssTemplate):
>+        * public/v3/components/customizable-test-group-form.js: Added UI to show testability information.
>+        (CustomizableTestGroupForm.prototype._renderCustomRevisionTable):
>+        (CustomizableTestGroupForm.prototype._constructTestabilityRows.):
>+        (CustomizableTestGroupForm.prototype._constructTestabilityRows):
>+        (CustomizableTestGroupForm.prototype._constructRevisionRadioButtons):
>+        Changing either revision editor or radio button should trigger a re-render as testability
>+        information for updated revision may change.
>+        (CustomizableTestGroupForm.cssTemplate):
>+        * public/v3/models/commit-set.js:
>+        (IntermediateCommitSet.prototype.commitsWithTestability): Renamed from 'commitsWithTestabilityWarnings'.
>+        (IntermediateCommitSet.prototype.commitsWithTestabilityWarnings): Deleted.
>+
> 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/custom-analysis-task-configurator-tests.js b/Websites/perf.webkit.org/browser-tests/custom-analysis-task-configurator-tests.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..c38545a6efc026178394320797f5b1ccedae92e8
>--- /dev/null
>+++ b/Websites/perf.webkit.org/browser-tests/custom-analysis-task-configurator-tests.js
>@@ -0,0 +1,32 @@
>+
>+describe('CustomAnalysisTaskConfigurator', () => {
>+    const scripts = ['instrumentation.js', '../shared/common-component-base.js', 'components/base.js', 'models/data-model.js', 'models/commit-log.js',
>+        'models/commit-set.js', 'models/repository.js', 'models/metric.js', 'models/triggerable.js', 'models/test.js', 'components/test-group-form.js', 'components/custom-analysis-task-configurator.js',
>+        'components/instant-file-uploader.js', 'lazily-evaluated-function.js'];
>+
>+    async function createCustomAnalysisTaskConfiguratorWithContext(context) {
>+        await context.importScripts(scripts, 'ComponentBase', 'DataModelObject', 'Repository', 'CommitLog', 'TriggerableRepositoryGroup',
>+            'CustomCommitSet', 'CustomAnalysisTaskConfigurator', 'MockRemoteAPI', 'LazilyEvaluatedFunction');
>+        const customAnalysisTaskConfigurator = new context.symbols.CustomAnalysisTaskConfigurator;
>+        context.document.body.appendChild(customAnalysisTaskConfigurator.element());
>+        return customAnalysisTaskConfigurator;
>+    }
>+
>+    it('Should not update commitSetMap if baseline is set and unmodified but comparison is null', async () => {
>+        const context = new BrowsingContext();
>+        const customAnalysisTaskConfigurator = await createCustomAnalysisTaskConfiguratorWithContext(context);
>+
>+        await waitForComponentsToRender(context);
>+        const repository = context.symbols.Repository.ensureSingleton(1, {name: 'WebKit'});
>+        const commitSet = new context.symbols.CustomCommitSet;
>+        commitSet.setRevisionForRepository(repository, '210948', null);
>+        customAnalysisTaskConfigurator._commitSetMap = {'Baseline': commitSet, 'Comparison': null};
>+        customAnalysisTaskConfigurator._repositoryGroupByConfiguration['Baseline'] = new context.symbols.TriggerableRepositoryGroup(1, {repositories: [{repository}]});
>+        customAnalysisTaskConfigurator._specifiedRevisions['Baseline'].set(repository, '210948');
>+
>+        const originalCommitSet = customAnalysisTaskConfigurator._commitSetMap;
>+        await customAnalysisTaskConfigurator._updateCommitSetMap();
>+
>+        expect(customAnalysisTaskConfigurator._commitSetMap).to.be(originalCommitSet);
>+    });
>+});
>\ No newline at end of file
>diff --git a/Websites/perf.webkit.org/browser-tests/index.html b/Websites/perf.webkit.org/browser-tests/index.html
>index 354047d3f9fb06f775df03a4c45332cbd4980213..62fe99278024a1acbca3bd490e45f654e86d6121 100644
>--- a/Websites/perf.webkit.org/browser-tests/index.html
>+++ b/Websites/perf.webkit.org/browser-tests/index.html
>@@ -27,6 +27,7 @@ mocha.setup('bdd');
> <script src="chart-revision-range-tests.js"></script>
> <script src="commit-log-viewer-tests.js"></script>
> <script src="test-group-form-tests.js"></script>
>+<script src="custom-analysis-task-configurator-tests.js"></script>
> <script src="customizable-test-group-form-tests.js"></script>
> <script src="markup-page-tests.js"></script>
> <script src="test-group-result-page-tests.js"></script>
>diff --git a/Websites/perf.webkit.org/public/v3/components/custom-analysis-task-configurator.js b/Websites/perf.webkit.org/public/v3/components/custom-analysis-task-configurator.js
>index 43f35b7ca5261485c2f8fe4c23f4f47aa3241ef4..35e8f6ac51a7d876daadb57b2d4d8b47ef283123 100644
>--- a/Websites/perf.webkit.org/public/v3/components/custom-analysis-task-configurator.js
>+++ b/Websites/perf.webkit.org/public/v3/components/custom-analysis-task-configurator.js
>@@ -7,16 +7,17 @@ class CustomAnalysisTaskConfigurator extends ComponentBase {
>         this._selectedTests = [];
>         this._triggerablePlatforms = [];
>         this._selectedPlatform = null;
>-        this._configurationNames = ['Baseline', 'Comparison'];
>         this._showComparison = false;
>+
>         this._commitSetMap = {};
>         this._specifiedRevisions = {'Baseline': new Map, 'Comparison': new Map};
>         this._patchUploaders = {'Baseline': new Map, 'Comparison': new Map};
>         this._customRootUploaders = {'Baseline': null, 'Comparison': null};
>-        this._fetchedRevisions = {'Baseline': new Map, 'Comparison': new Map};
>+        this._fetchedCommits = {'Baseline': new Map, 'Comparison': new Map};
>         this._repositoryGroupByConfiguration = {'Baseline': null, 'Comparison': null};
>-        this._updateTriggerableLazily = new LazilyEvaluatedFunction(this._updateTriggerable.bind(this));
>+        this._invalidRevisionForRepositoryByConfiguration = {'Baseline': new Map, 'Comparison': new Map};
> 
>+        this._updateTriggerableLazily = new LazilyEvaluatedFunction(this._updateTriggerable.bind(this));
>         this._renderTriggerableTestsLazily = new LazilyEvaluatedFunction(this._renderTriggerableTests.bind(this));
>         this._renderTriggerablePlatformsLazily = new LazilyEvaluatedFunction(this._renderTriggerablePlatforms.bind(this));
>         this._renderRepositoryPanesLazily = new LazilyEvaluatedFunction(this._renderRepositoryPanes.bind(this));
>@@ -55,9 +56,10 @@ class CustomAnalysisTaskConfigurator extends ComponentBase {
> 
>     _didUpdateSelectedPlatforms()
>     {
>+        this._specifiedRevisions = {'Baseline': new Map, 'Comparison': new Map};
>+        this._fetchedCommits = {'Baseline': new Map, 'Comparison': new Map};
>+        this._invalidRevisionForRepositoryByConfiguration = {'Baseline': new Map, 'Comparison': new Map};
>         this._updateCommitSetMap();
>-
>-        this.enqueueToRender();
>     }
> 
>     setCommitSets(baselineCommitSet, comparisonCommitSet)
>@@ -190,6 +192,11 @@ class CustomAnalysisTaskConfigurator extends ComponentBase {
>         const [triggerable, error] = this._updateTriggerableLazily.evaluate(this._selectedTests, this._selectedPlatform);
> 
>         this._renderRepositoryPanesLazily.evaluate(triggerable, error, this._selectedPlatform, this._repositoryGroupByConfiguration, this._showComparison);
>+
>+        this.renderReplace(this.content('baseline-testability'),
>+            this._buildTestabilityList(this._commitSetMap['Baseline'], 'Baseline', this._invalidRevisionForRepositoryByConfiguration['Baseline']));
>+        if (this._showComparison)
>+            this.renderReplace(this.content('comparison-testability'), this._buildTestabilityList(this._commitSetMap['Comparison'], 'Comparison', this._invalidRevisionForRepositoryByConfiguration['Comparison']));
>     }
> 
>     _renderTriggerableTests()
>@@ -304,12 +311,12 @@ class CustomAnalysisTaskConfigurator extends ComponentBase {
>             newComparison = null;
> 
>         const currentBaseline = this._commitSetMap['Baseline'];
>-        const currentComparison = this._commitSetMap['Baseline'];
>-        if (newBaseline == currentBaseline && newComparison == currentComparison)
>-            return; // Both of them are null.
>+        const currentComparison = this._commitSetMap['Comparison'];
>+        const areCommitSetsEqual = (commitSetA, commitSetB) => commitSetA == commitSetB || (commitSetA && commitSetB && commitSetA.equals(commitSetB));
>+        const sameBaselineConfig = areCommitSetsEqual(currentBaseline, newBaseline);
>+        const sameComparisionConfig = areCommitSetsEqual(currentComparison, newComparison);
> 
>-        if (newBaseline && currentBaseline && newBaseline.equals(currentBaseline)
>-            && newComparison && currentComparison && newComparison.equals(currentComparison))
>+        if (sameBaselineConfig && sameComparisionConfig)
>             return;
> 
>         this._commitSetMap = {'Baseline': newBaseline, 'Comparison': newComparison};
>@@ -331,8 +338,11 @@ class CustomAnalysisTaskConfigurator extends ComponentBase {
>         const commitSet = new CustomCommitSet;
>         for (let repository of repositoryGroup.repositories()) {
>             let revision = this._specifiedRevisions[configurationName].get(repository);
>-            if (!revision)
>-                revision = this._fetchedRevisions[configurationName].get(repository);
>+            if (!revision) {
>+                const commit = this._fetchedCommits[configurationName].get(repository);
>+                if (commit)
>+                    revision = commit.revision();
>+            }
>             if (!revision)
>                 return null;
>             let patch = null;
>@@ -355,6 +365,44 @@ class CustomAnalysisTaskConfigurator extends ComponentBase {
>         return commitSet;
>     }
> 
>+    async _fetchCommitsForConfiguration(configurationName)
>+    {
>+        const commitSet = this._commitSetMap[configurationName];
>+        if (!commitSet)
>+            return;
>+
>+        const specifiedRevisions = this._specifiedRevisions[configurationName];
>+        const fetchedCommits = this._fetchedCommits[configurationName];
>+        const invalidRevisionForRepository = this._invalidRevisionForRepositoryByConfiguration[configurationName];
>+
>+        await Promise.all(Array.from(commitSet.repositories()).map((repository) => {
>+            const revision = commitSet.revisionForRepository(repository);
>+            return this._resolveRevision(repository, revision, specifiedRevisions, invalidRevisionForRepository, fetchedCommits);
>+        }));
>+
>+        const latestCommitSet = this._commitSetMap[configurationName];
>+        if (commitSet != latestCommitSet)
>+            return;
>+        this.enqueueToRender();
>+    }
>+
>+    async _resolveRevision(repository, revision, specifiedRevisions, invalidRevisionForRepository, fetchedCommits)
>+    {
>+        try {
>+            const commits = await CommitLog.fetchForSingleRevision(repository, revision);
>+            console.assert(commits.length, 1);
>+            if (revision != specifiedRevisions.get(repository))
>+                return;
>+            invalidRevisionForRepository.delete(repository);
>+            fetchedCommits.set(repository, commits[0]);
>+        } catch (error) {
>+            if (revision != specifiedRevisions.get(repository))
>+                return;
>+            invalidRevisionForRepository.set(repository, revision);
>+            fetchedCommits.delete(repository);
>+        }
>+    }
>+
>     _renderRepositoryPanes(triggerable, error, platform, repositoryGroupByConfiguration, showComparison)
>     {
>         this.content('repository-configuration-error-pane').style.display = error ? null : 'none';
>@@ -414,7 +462,6 @@ class CustomAnalysisTaskConfigurator extends ComponentBase {
>     _buildRevisionTable(configurationName, repositoryGroups, currentGroup, platform, requiredRepositories, optionalRepositoryList, alwaysAcceptsCustomRoots)
>     {
>         const element = ComponentBase.createElement;
>-        const link = ComponentBase.createLink;
> 
>         const customRootsTBody = element('tbody', [
>             element('tr', [
>@@ -453,6 +500,23 @@ class CustomAnalysisTaskConfigurator extends ComponentBase {
>             )];
>     }
> 
>+    _buildTestabilityList(commitSet, configurationName, invalidRevisionForRepository)
>+    {
>+        const element = ComponentBase.createElement;
>+        const entries = [];
>+        for (const repository of commitSet ? commitSet.repositories() : []) {
>+            const commit = this._fetchedCommits[configurationName].get(repository);
>+            if (commit && commit.testability() && !invalidRevisionForRepository.has(repository))
>+                entries.push(element('li', `${commit.repository().name()} - "${commit.label()}": ${commit.testability()}`));
>+            if (invalidRevisionForRepository.has(repository))
>+                entries.push(element('li', `${repository.name()} - "${invalidRevisionForRepository.get(repository)}": Invalid revision`));
>+        }
>+        if (!entries.length)
>+            return [];
>+
>+        return entries;
>+    }
>+
>     _buildRepositoryGroupList(repositoryGroups, currentGroup, configurationName)
>     {
>         const element = ComponentBase.createElement;
>@@ -476,6 +540,7 @@ class CustomAnalysisTaskConfigurator extends ComponentBase {
>         clone[configurationName] = group;
>         this._repositoryGroupByConfiguration = clone;
>         this._updateCommitSetMap();
>+        this._fetchCommitsForConfiguration(configurationName);
>         this.enqueueToRender();
>     }
> 
>@@ -483,10 +548,19 @@ class CustomAnalysisTaskConfigurator extends ComponentBase {
>     {
>         const revision = this._specifiedRevisions[configurationName].get(repository) || '';
>         const element = ComponentBase.createElement;
>+        let scheduledUpdate = null;
>         const input = element('input', {value: revision, oninput: () => {
>             unmodifiedInput = null;
>-            this._specifiedRevisions[configurationName].set(repository, input.value);
>+            const revisionToFetch = input.value;
>+            this._specifiedRevisions[configurationName].set(repository, revisionToFetch);
>             this._updateCommitSetMap();
>+            if (scheduledUpdate)
>+                clearTimeout(scheduledUpdate);
>+            scheduledUpdate = setTimeout(() => {
>+                if (revisionToFetch == input.value)
>+                    this._fetchCommitsForConfiguration(configurationName);
>+                scheduledUpdate = null;
>+            }, 100);
>         }});
>         let unmodifiedInput = input;
> 
>@@ -494,7 +568,7 @@ class CustomAnalysisTaskConfigurator extends ComponentBase {
>             CommitLog.fetchLatestCommitForPlatform(repository, platform).then((commit) => {
>                 if (commit && unmodifiedInput) {
>                     unmodifiedInput.value = commit.revision();
>-                    this._fetchedRevisions[configurationName].set(repository, commit.revision());
>+                    this._fetchedCommits[configurationName].set(repository, commit);
>                     this._updateCommitSetMap();
>                 }
>             });
>@@ -521,6 +595,7 @@ class CustomAnalysisTaskConfigurator extends ComponentBase {
>             <section id="baseline-configuration-pane" class="pane">
>                 <h2>3. Configure Baseline</h2>
>                 <table id="baseline-revision-table" class="revision-table"></table>
>+                <ul id="baseline-testability"></ul>
>             </section>
>             <section id="specify-comparison-pane" class="pane">
>                 <button id="specify-comparison-button">Configure to Compare</button>
>@@ -528,6 +603,7 @@ class CustomAnalysisTaskConfigurator extends ComponentBase {
>             <section id="comparison-configuration-pane" class="pane">
>                 <h2>4. Configure Comparison</h2>
>                 <table id="comparison-revision-table" class="revision-table"></table>
>+                <ul id="comparison-testability"></ul>
>             </section>`;
>     }
> 
>@@ -674,6 +750,12 @@ class CustomAnalysisTaskConfigurator extends ComponentBase {
>                 font-size: 1.2rem;
>                 font-weight: inherit;
>             }
>+
>+            #baseline-testability li,
>+            #comparison-testability li {
>+                color: #c33;
>+                width: 20rem;
>+            }
> `;
>     }
> }
>diff --git a/Websites/perf.webkit.org/public/v3/components/customizable-test-group-form.js b/Websites/perf.webkit.org/public/v3/components/customizable-test-group-form.js
>index a1cd2dc3e3797cfda9ffb6020b08a02eab62e0d2..801d4f0d991ccf64bad211ec3bd942f150ccb11b 100644
>--- a/Websites/perf.webkit.org/public/v3/components/customizable-test-group-form.js
>+++ b/Websites/perf.webkit.org/public/v3/components/customizable-test-group-form.js
>@@ -135,7 +135,29 @@ class CustomizableTestGroupForm extends TestGroupForm {
>             element('thead',
>                 element('tr',
>                     [element('td', {colspan: 2}, 'Repository'), commitSetLabels.map((label) => element('td', {colspan: commitSetLabels.length + 1}, label)), element('td')])),
>-            this._constructTableBodyList(repositoryList, commitSetMap, ownedRepositoriesByRepository, this._hasIncompleteOwnedRepository, uncustomizedCommitSetMap)]);
>+            this._constructTableBodyList(repositoryList, commitSetMap, ownedRepositoriesByRepository, this._hasIncompleteOwnedRepository, uncustomizedCommitSetMap),
>+            this._constructTestabilityRows(commitSetMap)]);
>+    }
>+
>+    _constructTestabilityRows(commitSetMap)
>+    {
>+        const element = ComponentBase.createElement;
>+
>+        let hasCommitWithTestability = false;
>+        const commitSets = Array.from(commitSetMap.values());
>+        for (const commitSet of commitSets)
>+            hasCommitWithTestability |= !!commitSet.commitsWithTestability();
>+        if (!hasCommitWithTestability)
>+            return [];
>+
>+        const testabilityRows = [];
>+        for (const commitSet of commitSetMap.values()) {
>+            const rows = commitSet.commitsWithTestability().map((commit) =>
>+                element('tr', {class: 'testability'}, `${commit.repository().name()} - ${commit.label()}: ${commit.testability()}`));
>+            testabilityRows.push(element('td', {colspan: commitSetMap.size + 1}, rows));
>+        }
>+
>+        return element('tbody', element('tr', [element('td', {colspan: 2}), testabilityRows, element('td')]));
>     }
> 
>     _constructTableBodyList(repositoryList, commitSetMap, ownedRepositoriesByRepository, hasIncompleteOwnedRepository, uncustomizedCommitSetMap)
>@@ -255,8 +277,8 @@ class CustomizableTestGroupForm extends TestGroupForm {
>             onchange: () => {
>                 if (ownerRepository)
>                     return;
>-
>-                commitSetMap.get(columnLabel).updateRevisionForOwnerRepository(repository, revisionEditor.value).catch(
>+                commitSetMap.get(columnLabel).updateRevisionForOwnerRepository(repository, revisionEditor.value).then(
>+                    () => this.enqueueToRender(),
>                     () => {
>                         alert(`"${revisionEditor.value}" does not exist in "${repository.name()}".`);
>                         revisionEditor.value = revision;
>@@ -278,6 +300,7 @@ class CustomizableTestGroupForm extends TestGroupForm {
>                     revisionEditor.value = uncustomizedCommit ? uncustomizedCommit.revision() : '';
>                     if (uncustomizedCommit && uncustomizedCommit.ownerCommit())
>                         this._ownerRevisionMap.get(columnLabel).set(repository, uncustomizedCommit.ownerCommit().revision());
>+                    this.enqueueToRender();
>                 }});
>             nodes.push(element('td', element('label', [radioButton, labelToChoose])));
>         }
>@@ -336,6 +359,11 @@ class CustomizableTestGroupForm extends TestGroupForm {
>             #notify-on-completion-checkbox {
>                 margin-left: 0.4rem;
>             }
>+
>+            #custom-table tr.testability {
>+                text-align: left;
>+                color: #c33;
>+            }
>             `;
>     }
> 
>diff --git a/Websites/perf.webkit.org/public/v3/models/commit-set.js b/Websites/perf.webkit.org/public/v3/models/commit-set.js
>index 445b27017f248d9e5ac4c13b8755a0aad5722ca4..bb5b7f198cef39a14d33ffa1803d692f667aa8fa 100644
>--- a/Websites/perf.webkit.org/public/v3/models/commit-set.js
>+++ b/Websites/perf.webkit.org/public/v3/models/commit-set.js
>@@ -389,7 +389,7 @@ class IntermediateCommitSet {
>         return Promise.all(fetchingPromises);
>     }
> 
>-    commitsWithTestabilityWarnings() { return this.commits().filter((commit) => !!commit.testabilityWarning()); }
>+    commitsWithTestability() { return this.commits().filter((commit) => !!commit.testability()); }
>     commits() { return  Array.from(this._commitByRepository.values()); }
> 
>     _fetchCommitLogAndOwnedCommits(repository, revision)

Actions: View | Formatted Diff | Diff

Attachments on bug 192972: 357925 | 357926 | 358009 | 358048 | 358860 | 358878