Customizable test group form should not reset manually edited commit value sometimes.
authordewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Nov 2018 05:58:15 +0000 (05:58 +0000)
committerdewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Nov 2018 05:58:15 +0000 (05:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190863

Reviewed by Ryosuke Niwa.

After changing the radio button and manually editing the commit value, commit value should not be reset
while changing the name of the test group.
Add the logic to prompt warning when manually typed commit does not exist.

* browser-tests/customizable-test-group-form-tests.js: Added a unit test for this bug.
* browser-tests/index.html:
* public/v3/components/customizable-test-group-form.js: Should always update commit set as long as
the repository of that row does not have a owner repository.
(CustomizableTestGroupForm.prototype._constructTableRowForCommitsWithoutOwner):
(CustomizableTestGroupForm.prototype._constructTableRowForCommitsWithOwner):
(CustomizableTestGroupForm.prototype._constructRevisionRadioButtons):

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/browser-tests/customizable-test-group-form-tests.js [new file with mode: 0644]
Websites/perf.webkit.org/browser-tests/index.html
Websites/perf.webkit.org/public/v3/components/customizable-test-group-form.js

index 51973fd..bc29217 100644 (file)
@@ -1,3 +1,22 @@
+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
+
+        Reviewed by Ryosuke Niwa.
+
+        After changing the radio button and manually editing the commit value, commit value should not be reset
+        while changing the name of the test group.
+        Add the logic to prompt warning when manually typed commit does not exist.
+
+        * browser-tests/customizable-test-group-form-tests.js: Added a unit test for this bug.
+        * browser-tests/index.html:
+        * public/v3/components/customizable-test-group-form.js: Should always update commit set as long as
+        the repository of that row does not have a owner repository.
+        (CustomizableTestGroupForm.prototype._constructTableRowForCommitsWithoutOwner):
+        (CustomizableTestGroupForm.prototype._constructTableRowForCommitsWithOwner):
+        (CustomizableTestGroupForm.prototype._constructRevisionRadioButtons):
+
 2018-10-16  Dewei Zhu  <dewei_zhu@apple.com>
 
         Unreviewed, rolling out r236996.
diff --git a/Websites/perf.webkit.org/browser-tests/customizable-test-group-form-tests.js b/Websites/perf.webkit.org/browser-tests/customizable-test-group-form-tests.js
new file mode 100644 (file)
index 0000000..3c5ca5c
--- /dev/null
@@ -0,0 +1,92 @@
+
+describe('CustomizableTestGroupFormTests', () => {
+    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', 'components/test-group-form.js', 'components/customizable-test-group-form.js'];
+
+    async function createCustomizableTestGroupFormWithContext(context)
+    {
+        await context.importScripts(scripts, 'ComponentBase', 'DataModelObject', 'Repository', 'CommitLog', 'CommitSet', 'CustomizableTestGroupForm', 'MockRemoteAPI');
+        const customizableTestGroupForm = new context.symbols.CustomizableTestGroupForm;
+        context.document.body.appendChild(customizableTestGroupForm.element());
+        return customizableTestGroupForm;
+    }
+
+    const commitObjectA = {
+        "id": "185326",
+        "revision": "210948",
+        "repository": 1,
+        "previousCommit": null,
+        "ownsCommits": false,
+        "time": 1541494949681,
+        "authorName": "Zalan Bujtas",
+        "authorEmail": "zalan@apple.com",
+        "message": "a message",
+    };
+
+    const commitObjectB = {
+        "id": "185334",
+        "revision": "210949",
+        "repository": 1,
+        "previousCommit": null,
+        "ownsCommits": false,
+        "time": 1541494949682,
+        "authorName": "Chris Dumez",
+        "authorEmail": "cdumez@apple.com",
+        "message": "some message",
+    };
+
+    function cloneObject(object)
+    {
+        const clone = {};
+        for (const [key, value] of Object.entries(object))
+            clone[key] = value;
+        return clone;
+    }
+
+    it('Changing the value in revision editor should update corresponding commitSet as long as the repository of that row does not have owner', 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);
+
+        const radioButton = customizableTestGroupForm.content('custom-table').querySelector('input[type="radio"][name="A-1-radio"]:not(:checked)');
+        radioButton.click();
+        expect(radioButton.checked).to.be(true);
+
+        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('210949');
+        revisionEditor.value = '210948';
+        revisionEditor.dispatchEvent(new Event('change'));
+
+        customizableTestGroupForm.content('name').value = 'a/b test';
+        customizableTestGroupForm.content('name').dispatchEvent(new Event('input'));
+
+        await waitForComponentsToRender(context);
+
+        revisionEditors = customizableTestGroupForm.content('custom-table').querySelectorAll('input:not([type="radio"])');
+        revisionEditor = revisionEditors[0];
+        expect(revisionEditor.value).to.be('210948');
+    });
+});
index 217b964..354047d 100644 (file)
@@ -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="customizable-test-group-form-tests.js"></script>
 <script src="markup-page-tests.js"></script>
 <script src="test-group-result-page-tests.js"></script>
 <script>
index 20441c1..f43ac93 100644 (file)
@@ -174,7 +174,7 @@ class CustomizableTestGroupForm extends TestGroupForm {
         const cells = [element('th', {colspan: 2}, repository.label())];
 
         for (const label of commitSetMap.keys())
-            cells.push(this._constructRevisionRadioButtons(commitSetMap, repository, label, null, ownsRepositories));
+            cells.push(this._constructRevisionRadioButtons(commitSetMap, repository, label, null));
 
         if (ownsRepositories) {
             const plusButton = new PlusButton();
@@ -197,7 +197,7 @@ class CustomizableTestGroupForm extends TestGroupForm {
         const minusButton = new MinusButton();
 
         for (const label of commitSetMap.keys())
-            cells.push(this._constructRevisionRadioButtons(commitSetMap, repository, label, ownerRepository, false));
+            cells.push(this._constructRevisionRadioButtons(commitSetMap, repository, label, ownerRepository));
 
         minusButton.listenToAction('activate', () => {
             for (const commitSet of commitSetMap.values())
@@ -240,37 +240,41 @@ class CustomizableTestGroupForm extends TestGroupForm {
         return element('tr', cells);
     }
 
-    _constructRevisionRadioButtons(commitSetMap, repository, rowLabel, ownerRepository, ownsRepositories)
+    _constructRevisionRadioButtons(commitSetMap, repository, columnLabel, ownerRepository)
     {
         const element = ComponentBase.createElement;
-        const revisionEditor = element('input', {disabled: !!ownerRepository,
+
+        const commitForColumn = commitSetMap.get(columnLabel).commitForRepository(repository);
+        const revision = commitForColumn ? commitForColumn.revision() : '';
+        if (commitForColumn && commitForColumn.ownerCommit())
+            this._ownerRevisionMap.get(columnLabel).set(repository, commitForColumn.ownerCommit().revision());
+
+        const revisionEditor = element('input', {disabled: !!ownerRepository, value: revision,
             onchange: () => {
-                if (!ownsRepositories)
+                if (ownerRepository)
                     return;
-                commitSetMap.get(rowLabel).updateRevisionForOwnerRepository(repository, revisionEditor.value).catch(
-                    () => revisionEditor.value = '');
+
+                commitSetMap.get(columnLabel).updateRevisionForOwnerRepository(repository, revisionEditor.value).catch(
+                    () => {
+                        alert(`"${revisionEditor.value}" does not exist in "${repository.name()}".`);
+                        revisionEditor.value = revision;
+                    });
             }});
 
-        this._revisionEditorMap.get(rowLabel).set(repository, revisionEditor);
+        this._revisionEditorMap.get(columnLabel).set(repository, revisionEditor);
 
         const nodes = [];
         for (const labelToChoose of commitSetMap.keys()) {
             const commit = commitSetMap.get(labelToChoose).commitForRepository(repository);
-            const checkedLabel = this._checkedLabelByPosition.get(rowLabel).get(repository) || rowLabel;
+            const checkedLabel = this._checkedLabelByPosition.get(columnLabel).get(repository) || columnLabel;
             const checked =  labelToChoose == checkedLabel;
-            const radioButton = element('input', {type: 'radio', name: `${rowLabel}-${repository.id()}-radio`, checked,
+            const radioButton = element('input', {type: 'radio', name: `${columnLabel}-${repository.id()}-radio`, checked,
                 onchange: () => {
-                    this._checkedLabelByPosition.get(rowLabel).set(repository, labelToChoose);
+                    this._checkedLabelByPosition.get(columnLabel).set(repository, labelToChoose);
                     revisionEditor.value = commit ? commit.revision() : '';
                     if (commit && commit.ownerCommit())
-                        this._ownerRevisionMap.get(rowLabel).set(repository, commit.ownerCommit().revision());
+                        this._ownerRevisionMap.get(columnLabel).set(repository, commit.ownerCommit().revision());
                 }});
-
-            if (checked) {
-                revisionEditor.value = commit ? commit.revision() : '';
-                if (commit && commit.ownerCommit())
-                    this._ownerRevisionMap.get(rowLabel).set(repository, commit.ownerCommit().revision());
-            }
             nodes.push(element('td', element('label', [radioButton, labelToChoose])));
         }
         nodes.push(element('td', revisionEditor));