- Revision
- 217541
- Author
- [email protected]
- Date
- 2017-05-29 22:03:43 -0700 (Mon, 29 May 2017)
Log Message
Fix UI glitches with a custom analysis test group with a patch
https://bugs.webkit.org/show_bug.cgi?id=172694
Reviewed by Sam Weinig.
Fix the following UI glitches with perf try bots:
- Retrying an A/B testing with a patch fails.
- A patch specified in an test group does not get specified in the configurator.
- Drag & dropping a patch doesn't work.
- Results for custom analysis tasks don't get shown.
* public/api/test-groups.php:
(main): Fix a bug that test group's platform does not match that of the request'ed platform. Since each test
group is associated with platform, just use that instead of querying test_configurations. This resulted in
the configurator not being able to find a triggerable in some cases.
* public/v3/components/custom-analysis-task-configurator.js:
(CustomAnalysisTaskConfigurator):
(CustomAnalysisTaskConfigurator.prototype.setCommitSets): Add patches in the commit set.
(CustomAnalysisTaskConfigurator.prototype._setUploadedFilesToUploader): Now clears the exiting uploaded files
Also renamed from _setUploadedFilesIfEmpty.
(CustomAnalysisTaskConfigurator.prototype._setPatchFiles): Added.
(CustomAnalysisTaskConfigurator.prototype.didConstructShadowTree): We no longer update the list of roots
for the comparsion when a new root is added to the baseline.
(CustomAnalysisTaskConfigurator.prototype._configureComparison): Copy over the list of patches and roots when
starting to configure the comparsion.
* public/v3/components/instant-file-uploader.js:
(InstantFileUploader.prototype.clear): Added.
(InstantFileUploader.prototype.didConstructShadowTree): Added event handlers for dragover & drop events to
allow specifying a patch and root using drag & drop. Unfortunately, this still doesn't work in WebKit due to
a bug in our shadow DOM implementation.
(InstantFileUploader.prototype._didFileInputChange):
(InstantFileUploader.prototype._uploadFiles): Extracted from _didFileInputChange.
* public/v3/pages/analysis-task-page.js:
(AnalysisTaskTestGroupPane.prototype.setAnalysisResults): No longer takes metric.
(AnalysisTaskTestGroupPane.cssTemplate): Removed unused rules. Also disallow flexing on the list of test groups
to avoid the name of a test froup from overflowing on top of the results pane.
(AnalysisTaskPage.prototype._assignTestResultsIfPossible): Set setAnalysisResults even when metric is not set
as is the case for a custom analysis task.
(AnalysisTaskPage.prototype._retryCurrentTestGroup): Use createWithCustomConfiguration to allow retrying of
an A/B testing with a patch in a custom analysis task.
(AnalysisTaskPage.prototype._createTestGroupAfterVerifyingCommitSetList):
Modified Paths
Diff
Modified: trunk/Websites/perf.webkit.org/ChangeLog (217540 => 217541)
--- trunk/Websites/perf.webkit.org/ChangeLog 2017-05-29 18:20:26 UTC (rev 217540)
+++ trunk/Websites/perf.webkit.org/ChangeLog 2017-05-30 05:03:43 UTC (rev 217541)
@@ -1,3 +1,50 @@
+2017-05-29 Ryosuke Niwa <[email protected]>
+
+ Fix UI glitches with a custom analysis test group with a patch
+ https://bugs.webkit.org/show_bug.cgi?id=172694
+
+ Reviewed by Sam Weinig.
+
+ Fix the following UI glitches with perf try bots:
+ - Retrying an A/B testing with a patch fails.
+ - A patch specified in an test group does not get specified in the configurator.
+ - Drag & dropping a patch doesn't work.
+ - Results for custom analysis tasks don't get shown.
+
+ * public/api/test-groups.php:
+ (main): Fix a bug that test group's platform does not match that of the request'ed platform. Since each test
+ group is associated with platform, just use that instead of querying test_configurations. This resulted in
+ the configurator not being able to find a triggerable in some cases.
+
+ * public/v3/components/custom-analysis-task-configurator.js:
+ (CustomAnalysisTaskConfigurator):
+ (CustomAnalysisTaskConfigurator.prototype.setCommitSets): Add patches in the commit set.
+ (CustomAnalysisTaskConfigurator.prototype._setUploadedFilesToUploader): Now clears the exiting uploaded files
+ Also renamed from _setUploadedFilesIfEmpty.
+ (CustomAnalysisTaskConfigurator.prototype._setPatchFiles): Added.
+ (CustomAnalysisTaskConfigurator.prototype.didConstructShadowTree): We no longer update the list of roots
+ for the comparsion when a new root is added to the baseline.
+ (CustomAnalysisTaskConfigurator.prototype._configureComparison): Copy over the list of patches and roots when
+ starting to configure the comparsion.
+
+ * public/v3/components/instant-file-uploader.js:
+ (InstantFileUploader.prototype.clear): Added.
+ (InstantFileUploader.prototype.didConstructShadowTree): Added event handlers for dragover & drop events to
+ allow specifying a patch and root using drag & drop. Unfortunately, this still doesn't work in WebKit due to
+ a bug in our shadow DOM implementation.
+ (InstantFileUploader.prototype._didFileInputChange):
+ (InstantFileUploader.prototype._uploadFiles): Extracted from _didFileInputChange.
+
+ * public/v3/pages/analysis-task-page.js:
+ (AnalysisTaskTestGroupPane.prototype.setAnalysisResults): No longer takes metric.
+ (AnalysisTaskTestGroupPane.cssTemplate): Removed unused rules. Also disallow flexing on the list of test groups
+ to avoid the name of a test froup from overflowing on top of the results pane.
+ (AnalysisTaskPage.prototype._assignTestResultsIfPossible): Set setAnalysisResults even when metric is not set
+ as is the case for a custom analysis task.
+ (AnalysisTaskPage.prototype._retryCurrentTestGroup): Use createWithCustomConfiguration to allow retrying of
+ an A/B testing with a patch in a custom analysis task.
+ (AnalysisTaskPage.prototype._createTestGroupAfterVerifyingCommitSetList):
+
2017-05-26 Ryosuke Niwa <[email protected]>
Show patches applied in each A/B testing build requests
Modified: trunk/Websites/perf.webkit.org/public/api/test-groups.php (217540 => 217541)
--- trunk/Websites/perf.webkit.org/public/api/test-groups.php 2017-05-29 18:20:26 UTC (rev 217540)
+++ trunk/Websites/perf.webkit.org/public/api/test-groups.php 2017-05-30 05:03:43 UTC (rev 217541)
@@ -38,16 +38,9 @@
foreach ($test_groups as &$group) {
$group_id = $group['id'];
$group_by_id[$group_id] = &$group;
- $platforms = $db->query_and_fetch_all('SELECT DISTINCT(config_platform)
- FROM test_configurations, test_runs, build_requests
- WHERE run_config = config_id AND run_build = request_build AND request_group = $1', array($group_id));
- if ($platforms)
- $group['platform'] = $platforms[0]['config_platform'];
- else {
- $first_request = $db->select_first_row('build_requests', 'request', array('group' => $group_id), 'order');
- if ($first_request)
- $group['platform'] = $first_request['request_platform'];
- }
+ $first_request = $db->select_first_row('build_requests', 'request', array('group' => $group_id), 'order');
+ if ($first_request)
+ $group['platform'] = $first_request['request_platform'];
}
$build_requests = $build_requests_fetcher->results();
Modified: trunk/Websites/perf.webkit.org/public/v3/components/custom-analysis-task-configurator.js (217540 => 217541)
--- trunk/Websites/perf.webkit.org/public/v3/components/custom-analysis-task-configurator.js 2017-05-29 18:20:26 UTC (rev 217540)
+++ trunk/Websites/perf.webkit.org/public/v3/components/custom-analysis-task-configurator.js 2017-05-30 05:03:43 UTC (rev 217541)
@@ -12,6 +12,7 @@
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._repositoryGroupByConfiguration = {'Baseline': null, 'Comparison': null};
this._updateTriggerableLazily = new LazilyEvaluatedFunction(this._updateTriggerable.bind(this));
@@ -19,8 +20,6 @@
this._renderTriggerableTestsLazily = new LazilyEvaluatedFunction(this._renderTriggerableTests.bind(this));
this._renderTriggerablePlatformsLazily = new LazilyEvaluatedFunction(this._renderTriggerablePlatforms.bind(this));
this._renderRepositoryPanesLazily = new LazilyEvaluatedFunction(this._renderRepositoryPanes.bind(this));
-
- this._customRootUploaders = {};
}
tests() { return this._selectedTests; }
@@ -65,15 +64,17 @@
const baselineRepositoryGroup = triggerable.repositoryGroups().find((repositoryGroup) => repositoryGroup.accepts(baselineCommitSet));
if (baselineRepositoryGroup) {
this._repositoryGroupByConfiguration['Baseline'] = baselineRepositoryGroup;
- this._setUploadedFilesIfEmpty(this._customRootUploaders['Baseline'], baselineCommitSet);
+ this._setUploadedFilesToUploader(this._customRootUploaders['Baseline'], baselineCommitSet.customRoots());
this._specifiedRevisions['Baseline'] = this._revisionMapFromCommitSet(baselineCommitSet);
+ this._setPatchFiles('Baseline', baselineCommitSet);
}
const comparisonRepositoryGroup = triggerable.repositoryGroups().find((repositoryGroup) => repositoryGroup.accepts(baselineCommitSet));
if (comparisonRepositoryGroup) {
this._repositoryGroupByConfiguration['Comparison'] = comparisonRepositoryGroup;
- this._setUploadedFilesIfEmpty(this._customRootUploaders['Comparison'], comparisonCommitSet);
+ this._setUploadedFilesToUploader(this._customRootUploaders['Comparison'], comparisonCommitSet.customRoots());
this._specifiedRevisions['Comparison'] = this._revisionMapFromCommitSet(comparisonCommitSet);
+ this._setPatchFiles('Comparison', comparisonCommitSet);
}
this._showComparison = true;
@@ -80,14 +81,24 @@
this._updateCommitSetMap();
}
- _setUploadedFilesIfEmpty(uploader, commitSet)
+ _setUploadedFilesToUploader(uploader, files)
{
if (!uploader || uploader.hasFileToUpload() || uploader.uploadedFiles().length)
return;
- for (const uploadedFile of commitSet.customRoots())
+ uploader.clearUploads();
+ for (const uploadedFile of files)
uploader.addUploadedFile(uploadedFile);
}
+ _setPatchFiles(configurationName, commitSet)
+ {
+ for (const repository of commitSet.repositories()) {
+ const patch = commitSet.patchForRepository(repository);
+ if (patch)
+ this._setUploadedFilesToUploader(this._ensurePatchUploader(configurationName, repository), [patch]);
+ }
+ }
+
_revisionMapFromCommitSet(commitSet)
{
const revisionMap = new Map;
@@ -109,11 +120,7 @@
}
const baselineRootsUploader = createRootUploader();
- baselineRootsUploader.listenToAction('uploadedFile', (uploadedFile) => {
- comparisonRootsUploader.addUploadedFile(uploadedFile);
- this._updateCommitSetMap();
- });
-
+ baselineRootsUploader.listenToAction('uploadedFile', (uploadedFile) => this._updateCommitSetMap());
this._customRootUploaders['Baseline'] = baselineRootsUploader;
const comparisonRootsUploader = createRootUploader();
@@ -148,6 +155,19 @@
specifiedComparisonRevisions.set(key, specifiedBaselineRevisions.get(key));
this._specifiedRevisions['Comparison'] = specifiedComparisonRevisions;
+ for (const [repository, patchUploader] of this._patchUploaders['Baseline']) {
+ const files = patchUploader.uploadedFiles();
+ if (!files.length)
+ continue;
+ const comparisonPatchUploader = this._ensurePatchUploader('Comparison', repository);
+ for (const uploadedFile of files)
+ comparisonPatchUploader.addUploadedFile(uploadedFile);
+ }
+
+ const comparisonRootUploader = this._customRootUploaders['Comparison'];
+ for (const uploadedFile of this._customRootUploaders['Baseline'].uploadedFiles())
+ comparisonRootUploader.addUploadedFile(uploadedFile);
+
this.enqueueToRender();
}
Modified: trunk/Websites/perf.webkit.org/public/v3/components/instant-file-uploader.js (217540 => 217541)
--- trunk/Websites/perf.webkit.org/public/v3/components/instant-file-uploader.js 2017-05-29 18:20:26 UTC (rev 217540)
+++ trunk/Websites/perf.webkit.org/public/v3/components/instant-file-uploader.js 2017-05-30 05:03:43 UTC (rev 217541)
@@ -13,6 +13,12 @@
this._renderPreuploadFilesLazily = new LazilyEvaluatedFunction(this._renderPreuploadFiles.bind(this));
}
+ clearUploads()
+ {
+ this._uploadedFiles = [];
+ this._preuploadFiles = [];
+ this._uploadProgress = new WeakMap;
+ }
hasFileToUpload() { return !!this._preuploadFiles.length; }
uploadedFiles() { return this._uploadedFiles; }
@@ -33,9 +39,23 @@
didConstructShadowTree()
{
- this.content('file-adder')._onclick_ = () => {
+ const addButton = this.content('file-adder');
+ addButton._onclick_ = () => {
inputElement.click();
}
+ addButton.addEventListener('dragover', (event) => {
+ event.dataTransfer.dropEffect = 'copy';
+ event.preventDefault();
+ });
+ addButton.addEventListener('drop', (event) => {
+ event.preventDefault();
+ let files = event.dataTransfer.files;
+ if (!files.length)
+ return;
+ if (files.length > 1 && !this._allowMultipleFiles)
+ files = [files[0]];
+ this._uploadFiles(files);
+ });
const inputElement = document.createElement('input');
inputElement.type = 'file';
inputElement._onchange_ = () => this._didFileInputChange(inputElement);
@@ -125,13 +145,23 @@
{
if (!input.files.length)
return;
+ this._uploadFiles(input.files);
+ input.value = null;
+ this.enqueueToRender();
+ }
+
+ _uploadFiles(files)
+ {
const limit = UploadedFile.fileUploadSizeLimit;
- for (let file of input.files) {
+ for (let file of files) {
if (file.size > limit) {
alert(`The specified file "${file.name}" is too big (${this._fileSizeFormatter(file.size)}). It must be smaller than ${this._fileSizeFormatter(limit)}`);
- input.value = null;
return;
}
+ }
+
+ const uploadProgress = this._uploadProgress;
+ for (let file of files) {
UploadedFile.fetchUnloadedFileWithIdenticalHash(file).then((uploadedFile) => {
if (uploadedFile) {
this._didUploadFile(file, uploadedFile);
@@ -139,20 +169,20 @@
}
UploadedFile.uploadFile(file, (progress) => {
- this._uploadProgress.set(file, progress);
- this.enqueueToRender();
+ uploadProgress.set(file, progress);
+ if (this._uploadProgress == uploadProgress)
+ this.enqueueToRender();
}).then((uploadedFile) => {
- this._didUploadFile(file, uploadedFile);
+ if (this._uploadProgress == uploadProgress)
+ this._didUploadFile(file, uploadedFile);
}, (error) => {
- this._uploadProgress.set(file, {error: error === 0 ? 'UnknownError' : error});
- this.enqueueToRender();
+ uploadProgress.set(file, {error: error === 0 ? 'UnknownError' : error});
+ if (this._uploadedProgress == uploadProgress)
+ this.enqueueToRender();
});
});
}
- this._preuploadFiles = Array.from(input.files);
- input.value = null;
-
- this.enqueueToRender();
+ this._preuploadFiles = Array.from(files);
}
_removeUploadedFile(uploadedFile)
Modified: trunk/Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js (217540 => 217541)
--- trunk/Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js 2017-05-29 18:20:26 UTC (rev 217540)
+++ trunk/Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js 2017-05-30 05:03:43 UTC (rev 217541)
@@ -280,10 +280,10 @@
this.enqueueToRender();
}
- setAnalysisResults(analysisResults, metric)
+ setAnalysisResults(analysisResults)
{
this.part('revision-table').setAnalysisResults(analysisResults);
- this.part('results-viewer').setAnalysisResults(analysisResults, metric);
+ this.part('results-viewer').setAnalysisResults(analysisResults, null);
this.enqueueToRender();
}
@@ -369,15 +369,8 @@
font-size: 0.9rem;
}
- #new-container {
- display: flex;
- }
-
- #new-container test-group-revision-table {
- margin-left: 2rem;
- }
-
#test-group-list {
+ flex: none;
margin: 0;
padding: 0.2rem 0;
list-style: none;
@@ -617,12 +610,15 @@
_assignTestResultsIfPossible()
{
- if (!this._task || !this._metric || !this._testGroups || !this._analysisResults)
+ if (!this._task || !this._testGroups || !this._analysisResults)
return false;
- const view = this._analysisResults.viewForMetric(this._metric);
- this.part('group-pane').setAnalysisResults(this._analysisResults, this._metric);
- this.part('results-pane').setAnalysisResultsView(view);
+ this.part('group-pane').setAnalysisResults(this._analysisResults);
+ let metric = this._metric;
+ if (metric) {
+ const view = this._analysisResults.viewForMetric(metric);
+ this.part('results-pane').setAnalysisResultsView(view);
+ }
return true;
}
@@ -791,11 +787,11 @@
{
const newName = this._createRetryNameForTestGroup(testGroup.name());
const commitSetList = testGroup.requestedCommitSets();
-
- const commitSetMap = {};
- for (let commitSet of commitSetList)
- commitSetMap[testGroup.labelForCommitSet(commitSet)] = commitSet;
-
+ const platform = this._task.platform() || testGroup.platform();
+ return TestGroup.createWithCustomConfiguration(this._task, platform, testGroup.test(), newName, repetitionCount, commitSetList)
+ .then(this._didFetchTestGroups.bind(this), function (error) {
+ alert('Failed to create a new test group: ' + error);
+ });
return this._createTestGroupAfterVerifyingCommitSetList(newName, repetitionCount, commitSetMap);
}
@@ -825,7 +821,7 @@
for (let label in commitSetMap)
commitSets.push(commitSetMap[label]);
- TestGroup.createAndRefetchTestGroups(this._task, testGroupName, repetitionCount, commitSets)
+ return TestGroup.createAndRefetchTestGroups(this._task, testGroupName, repetitionCount, commitSets)
.then(this._didFetchTestGroups.bind(this), function (error) {
alert('Failed to create a new test group: ' + error);
});
