Log Message
Custom test group form should use commit set map before customization as the behavior of radio buttons. https://bugs.webkit.org/show_bug.cgi?id=191347
Reviewed by Ryosuke Niwa. The radio button behavior should always set the same revision while editing the revision input. That means we should not use the intermediate commit set map but use the commit set map before "Customize" link is clicked. * browser-tests/customizable-test-group-form-tests.js: Added a unit test for this bug. * public/v3/components/customizable-test-group-form.js: Pass uncustomized commit set so that the radio button behavoir preserves. (CustomizableTestGroupForm): (CustomizableTestGroupForm.prototype.setCommitSetMap): (CustomizableTestGroupForm.prototype.didConstructShadowTree): (CustomizableTestGroupForm.prototype.render): (CustomizableTestGroupForm.prototype._renderCustomRevisionTable): (CustomizableTestGroupForm.prototype._constructTableBodyList): (CustomizableTestGroupForm.prototype._constructTableRowForCommitsWithoutOwner): (CustomizableTestGroupForm.prototype._constructTableRowForCommitsWithOwner): (CustomizableTestGroupForm.prototype._constructRevisionRadioButtons):
Modified Paths
Diff
Modified: trunk/Websites/perf.webkit.org/ChangeLog (237949 => 237950)
--- trunk/Websites/perf.webkit.org/ChangeLog 2018-11-08 00:15:59 UTC (rev 237949)
+++ trunk/Websites/perf.webkit.org/ChangeLog 2018-11-08 00:30:15 UTC (rev 237950)
@@ -1,5 +1,29 @@
2018-11-06 Dewei Zhu <dewei_...@apple.com>
+ Custom test group form should use commit set map before customization as the behavior of radio buttons.
+ https://bugs.webkit.org/show_bug.cgi?id=191347
+
+ Reviewed by Ryosuke Niwa.
+
+ The radio button behavior should always set the same revision while editing the revision input.
+ That means we should not use the intermediate commit set map but use the commit set map before
+ "Customize" link is clicked.
+
+ * browser-tests/customizable-test-group-form-tests.js: Added a unit test for this bug.
+ * public/v3/components/customizable-test-group-form.js: Pass uncustomized commit set so that the radio button
+ behavoir preserves.
+ (CustomizableTestGroupForm):
+ (CustomizableTestGroupForm.prototype.setCommitSetMap):
+ (CustomizableTestGroupForm.prototype.didConstructShadowTree):
+ (CustomizableTestGroupForm.prototype.render):
+ (CustomizableTestGroupForm.prototype._renderCustomRevisionTable):
+ (CustomizableTestGroupForm.prototype._constructTableBodyList):
+ (CustomizableTestGroupForm.prototype._constructTableRowForCommitsWithoutOwner):
+ (CustomizableTestGroupForm.prototype._constructTableRowForCommitsWithOwner):
+ (CustomizableTestGroupForm.prototype._constructRevisionRadioButtons):
+
+2018-11-06 Dewei Zhu <dewei_...@apple.com>
+
Customizable test group form should not reset manually edited commit value sometimes.
https://bugs.webkit.org/show_bug.cgi?id=190863
Modified: trunk/Websites/perf.webkit.org/browser-tests/customizable-test-group-form-tests.js (237949 => 237950)
--- trunk/Websites/perf.webkit.org/browser-tests/customizable-test-group-form-tests.js 2018-11-08 00:15:59 UTC (rev 237949)
+++ trunk/Websites/perf.webkit.org/browser-tests/customizable-test-group-form-tests.js 2018-11-08 00:30:15 UTC (rev 237950)
@@ -89,4 +89,54 @@
revisionEditor = revisionEditors[0];
expect(revisionEditor.value).to.be('210948');
});
+
+ it('should use the commit set map when customize button is clicked as the behavior of radio buttons', async () => {
+ const context = new BrowsingContext();
+ const customizableTestGroupForm = await createCustomizableTestGroupFormWithContext(context);
+ const repository = context.symbols.Repository.ensureSingleton(1, {name: 'WebKit'});
+
+ const commitA = cloneObject(commitObjectA);
+ const commitB = cloneObject(commitObjectB);
+ commitA.repository = repository;
+ commitB.repository = repository;
+ const webkitCommitA = context.symbols.CommitLog.ensureSingleton(185326, commitA);
+ const webkitCommitB = context.symbols.CommitLog.ensureSingleton(185334, commitB);
+ const commitSetA = context.symbols.CommitSet.ensureSingleton(1, {revisionItems: [{commit: webkitCommitA}]});
+ const commitSetB = context.symbols.CommitSet.ensureSingleton(2, {revisionItems: [{commit: webkitCommitB}]});
+
+ customizableTestGroupForm.setCommitSetMap({A: commitSetA, B: commitSetB});
+ customizableTestGroupForm.content('customize-link').click();
+
+ const requests = context.symbols.MockRemoteAPI.requests;
+ expect(requests.length).to.be(2);
+ expect(requests[0].url).to.be('/api/commits/1/210948');
+ expect(requests[1].url).to.be('/api/commits/1/210949');
+ requests[0].resolve({commits: [commitObjectA]});
+ requests[1].resolve({commits: [commitObjectB]});
+
+ await waitForComponentsToRender(context);
+
+ let revisionEditors = customizableTestGroupForm.content('custom-table').querySelectorAll('input:not([type="radio"])');
+ expect(revisionEditors.length).to.be(2);
+ let revisionEditor = revisionEditors[0];
+ expect(revisionEditor.value).to.be('210948');
+ revisionEditor.value = '210949';
+ revisionEditor.dispatchEvent(new Event('change'));
+
+ customizableTestGroupForm.content('name').value = 'a/b test';
+ customizableTestGroupForm.content('name').dispatchEvent(new Event('input'));
+
+ await waitForComponentsToRender(context);
+
+ let radioButton = customizableTestGroupForm.content('custom-table').querySelector('input[type="radio"][name="A-1-radio"]:not(:checked)');
+ radioButton.click();
+ expect(radioButton.checked).to.be(true);
+ radioButton = customizableTestGroupForm.content('custom-table').querySelector('input[type="radio"][name="A-1-radio"]:not(:checked)');
+ radioButton.click();
+ expect(radioButton.checked).to.be(true);
+
+ revisionEditors = customizableTestGroupForm.content('custom-table').querySelectorAll('input:not([type="radio"])');
+ revisionEditor = revisionEditors[0];
+ expect(revisionEditor.value).to.be('210948');
+ });
});
Modified: trunk/Websites/perf.webkit.org/public/v3/components/customizable-test-group-form.js (237949 => 237950)
--- trunk/Websites/perf.webkit.org/public/v3/components/customizable-test-group-form.js 2018-11-08 00:15:59 UTC (rev 237949)
+++ trunk/Websites/perf.webkit.org/public/v3/components/customizable-test-group-form.js 2018-11-08 00:30:15 UTC (rev 237950)
@@ -4,6 +4,7 @@
{
super('customizable-test-group-form');
this._commitSetMap = null;
+ this._uncustomizedCommitSetMap = null;
this._name = null;
this._isCustomized = false;
this._revisionEditorMap = null;
@@ -16,6 +17,7 @@
setCommitSetMap(map)
{
this._commitSetMap = map;
+ this._uncustomizedCommitSetMap = map;
this._isCustomized = false;
this._fetchingCommitPromises = [];
this._checkedLabelByPosition = new Map;
@@ -40,11 +42,11 @@
this.content('customize-link')._onclick_ = this.createEventHandler(() => {
if (!this._isCustomized) {
- const originalCommitSetMap = this._commitSetMap;
+ const uncustomizedCommitSetMap = this._commitSetMap;
const fetchingCommitPromises = [];
this._commitSetMap = new Map;
- for (const label in originalCommitSetMap) {
- const intermediateCommitSet = new IntermediateCommitSet(originalCommitSetMap[label]);
+ for (const label in uncustomizedCommitSetMap) {
+ const intermediateCommitSet = new IntermediateCommitSet(uncustomizedCommitSetMap[label]);
fetchingCommitPromises.push(intermediateCommitSet.fetchCommitLogs());
this._commitSetMap.set(label, intermediateCommitSet);
}
@@ -55,7 +57,7 @@
return;
this._isCustomized = true;
this._fetchingCommitPromises = [];
- for (const label in originalCommitSetMap)
+ for (const label in uncustomizedCommitSetMap)
this._checkedLabelByPosition.set(label, new Map);
this.enqueueToRender();
});
@@ -93,10 +95,10 @@
this.content('start-button').disabled = !(this._commitSetMap && this._name);
this.content('customize-link-container').style.display = !this._commitSetMap ? 'none' : null;
- this._renderCustomRevisionTable(this._commitSetMap, this._isCustomized);
+ this._renderCustomRevisionTable(this._commitSetMap, this._isCustomized, this._uncustomizedCommitSetMap);
}
- _renderCustomRevisionTable(commitSetMap, isCustomized)
+ _renderCustomRevisionTable(commitSetMap, isCustomized, uncustomizedCommitSetMap)
{
if (!commitSetMap || !isCustomized) {
this.renderReplace(this.content('custom-table'), []);
@@ -133,10 +135,10 @@
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)]);
+ this._constructTableBodyList(repositoryList, commitSetMap, ownedRepositoriesByRepository, this._hasIncompleteOwnedRepository, uncustomizedCommitSetMap)]);
}
- _constructTableBodyList(repositoryList, commitSetMap, ownedRepositoriesByRepository, hasIncompleteOwnedRepository)
+ _constructTableBodyList(repositoryList, commitSetMap, ownedRepositoriesByRepository, hasIncompleteOwnedRepository, uncustomizedCommitSetMap)
{
const element = ComponentBase.createElement;
const tableBodyList = [];
@@ -146,7 +148,7 @@
const hasIncompleteRow = hasIncompleteOwnedRepository.get(repository);
const ownedRepositories = ownedRepositoriesByRepository.get(repository);
- rows.push(this._constructTableRowForCommitsWithoutOwner(commitSetMap, repository, allCommitSetSpecifiesOwnerCommit, hasIncompleteOwnedRepository));
+ rows.push(this._constructTableRowForCommitsWithoutOwner(commitSetMap, repository, allCommitSetSpecifiesOwnerCommit, hasIncompleteOwnedRepository, uncustomizedCommitSetMap));
if ((!ownedRepositories || !ownedRepositories.size) && !hasIncompleteRow) {
tableBodyList.push(element('tbody', rows));
@@ -155,7 +157,7 @@
if (ownedRepositories) {
for (const ownedRepository of ownedRepositories)
- rows.push(this._constructTableRowForCommitsWithOwner(commitSetMap, ownedRepository, repository));
+ rows.push(this._constructTableRowForCommitsWithOwner(commitSetMap, ownedRepository, repository, uncustomizedCommitSetMap));
}
if (hasIncompleteRow) {
@@ -168,13 +170,13 @@
return tableBodyList;
}
- _constructTableRowForCommitsWithoutOwner(commitSetMap, repository, ownsRepositories, hasIncompleteOwnedRepository)
+ _constructTableRowForCommitsWithoutOwner(commitSetMap, repository, ownsRepositories, hasIncompleteOwnedRepository, uncustomizedCommitSetMap)
{
const element = ComponentBase.createElement;
const cells = [element('th', {colspan: 2}, repository.label())];
for (const label of commitSetMap.keys())
- cells.push(this._constructRevisionRadioButtons(commitSetMap, repository, label, null));
+ cells.push(this._constructRevisionRadioButtons(commitSetMap, repository, label, null, uncustomizedCommitSetMap));
if (ownsRepositories) {
const plusButton = new PlusButton();
@@ -190,7 +192,7 @@
return element('tr', cells);
}
- _constructTableRowForCommitsWithOwner(commitSetMap, repository, ownerRepository)
+ _constructTableRowForCommitsWithOwner(commitSetMap, repository, ownerRepository, uncustomizedCommitSetMap)
{
const element = ComponentBase.createElement;
const cells = [element('td', {class: 'owner-repository-label'}), element('th', repository.label())];
@@ -197,7 +199,7 @@
const minusButton = new MinusButton();
for (const label of commitSetMap.keys())
- cells.push(this._constructRevisionRadioButtons(commitSetMap, repository, label, ownerRepository));
+ cells.push(this._constructRevisionRadioButtons(commitSetMap, repository, label, ownerRepository, uncustomizedCommitSetMap));
minusButton.listenToAction('activate', () => {
for (const commitSet of commitSetMap.values())
@@ -240,7 +242,7 @@
return element('tr', cells);
}
- _constructRevisionRadioButtons(commitSetMap, repository, columnLabel, ownerRepository)
+ _constructRevisionRadioButtons(commitSetMap, repository, columnLabel, ownerRepository, uncustomizedCommitSetMap)
{
const element = ComponentBase.createElement;
@@ -264,16 +266,18 @@
this._revisionEditorMap.get(columnLabel).set(repository, revisionEditor);
const nodes = [];
- for (const labelToChoose of commitSetMap.keys()) {
- const commit = commitSetMap.get(labelToChoose).commitForRepository(repository);
+ for (const [labelToChoose, commitSet] of commitSetMap) {
+ const commit = commitSet.commitForRepository(repository);
const checkedLabel = this._checkedLabelByPosition.get(columnLabel).get(repository) || columnLabel;
- const checked = labelToChoose == checkedLabel;
+ const checked = labelToChoose == checkedLabel;
const radioButton = element('input', {type: 'radio', name: `${columnLabel}-${repository.id()}-radio`, checked,
onchange: () => {
+ const uncustomizedCommit = uncustomizedCommitSetMap[labelToChoose].commitForRepository(repository) || commit;
+ commitSetMap.get(columnLabel).setCommitForRepository(repository, uncustomizedCommit);
this._checkedLabelByPosition.get(columnLabel).set(repository, labelToChoose);
- revisionEditor.value = commit ? commit.revision() : '';
- if (commit && commit.ownerCommit())
- this._ownerRevisionMap.get(columnLabel).set(repository, commit.ownerCommit().revision());
+ revisionEditor.value = uncustomizedCommit ? uncustomizedCommit.revision() : '';
+ if (uncustomizedCommit && uncustomizedCommit.ownerCommit())
+ this._ownerRevisionMap.get(columnLabel).set(repository, uncustomizedCommit.ownerCommit().revision());
}});
nodes.push(element('td', element('label', [radioButton, labelToChoose])));
}
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes
