Custom test group form should use commit set map before customization as the behavior...
authordewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 8 Nov 2018 00:30:15 +0000 (00:30 +0000)
committerdewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 8 Nov 2018 00:30:15 +0000 (00:30 +0000)
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):

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/browser-tests/customizable-test-group-form-tests.js
Websites/perf.webkit.org/public/v3/components/customizable-test-group-form.js

index bc29217..1ad61fb 100644 (file)
@@ -1,5 +1,29 @@
 2018-11-06  Dewei Zhu  <dewei_zhu@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_zhu@apple.com>
+
         Customizable test group form should not reset manually edited commit value sometimes.
         https://bugs.webkit.org/show_bug.cgi?id=190863
 
index 3c5ca5c..04408c5 100644 (file)
@@ -89,4 +89,54 @@ describe('CustomizableTestGroupFormTests', () => {
         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');
+    });
 });
index f43ac93..a1cd2dc 100644 (file)
@@ -4,6 +4,7 @@ class CustomizableTestGroupForm extends TestGroupForm {
     {
         super('customizable-test-group-form');
         this._commitSetMap = null;
+        this._uncustomizedCommitSetMap = null;
         this._name = null;
         this._isCustomized = false;
         this._revisionEditorMap = null;
@@ -16,6 +17,7 @@ class CustomizableTestGroupForm extends TestGroupForm {
     setCommitSetMap(map)
     {
         this._commitSetMap = map;
+        this._uncustomizedCommitSetMap = map;
         this._isCustomized = false;
         this._fetchingCommitPromises = [];
         this._checkedLabelByPosition = new Map;
@@ -40,11 +42,11 @@ class CustomizableTestGroupForm extends TestGroupForm {
 
         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 @@ class CustomizableTestGroupForm extends TestGroupForm {
                         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 @@ class CustomizableTestGroupForm extends TestGroupForm {
         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 @@ 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)]);
+            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 @@ class CustomizableTestGroupForm extends TestGroupForm {
             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 @@ class CustomizableTestGroupForm extends TestGroupForm {
 
             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 @@ class CustomizableTestGroupForm extends TestGroupForm {
         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,14 +192,14 @@ class CustomizableTestGroupForm extends TestGroupForm {
         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())];
         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 @@ class CustomizableTestGroupForm extends TestGroupForm {
         return element('tr', cells);
     }
 
-    _constructRevisionRadioButtons(commitSetMap, repository, columnLabel, ownerRepository)
+    _constructRevisionRadioButtons(commitSetMap, repository, columnLabel, ownerRepository, uncustomizedCommitSetMap)
     {
         const element = ComponentBase.createElement;
 
@@ -264,16 +266,18 @@ class CustomizableTestGroupForm extends TestGroupForm {
         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])));
         }