Changeset 230441 in webkit

Timestamp:

Apr 9, 2018 9:27:52 AM (6 years ago)

Author:

Dewei Zhu

Message:

Added 'CommitSet.diff' which will be shared between multiple independent incoming changes.
​https://bugs.webkit.org/show_bug.cgi?id=184368

Reviewed by Ryosuke Niwa.

'CommitSet.diff' will be used in multiple independent incoming changes.
It would be easier to make this a separate change to parallelize the changes depends on this API.

• public/v3/models/commit-set.js:

(CommitSet.prototype.createNameWithoutCollision): Moved from 'AnalysisTaskPage' and make it more generic.
(CommitSet.prototype.diff): Describe differences between 2 commit sets including commit, root and patch differences.

• public/v3/pages/analysis-task-page.js: Move 'AnalysisTaskPage._createRetryNameForTestGroup' to CommitSet in a more generic form.

(AnalysisTaskPage.prototype._retryCurrentTestGroup): Use 'CommitSet.withoutRootPatchOrOwnedCommit' instead.
(AnalysisTaskPage.prototype._createRetryNameForTestGroup): Moved to CommitSet in a more generic form.

• unit-tests/commit-set-tests.js: Added unit tests for 'CommitSet.diff'.

Location:

trunk/Websites/perf.webkit.org

Files:

• ChangeLog (modified) (1 diff)
• public/v3/models/commit-set.js (modified) (1 diff)
• public/v3/pages/analysis-task-page.js (modified) (2 diffs)
• unit-tests/commit-set-tests.js (modified) (4 diffs)

trunk/Websites/perf.webkit.org/ChangeLog

@@ +1 @@
+2018-04-06  Dewei Zhu  <dewei_zhu@apple.com>
+
+        Added 'CommitSet.diff' which will be shared between multiple independent incoming changes.
+        https://bugs.webkit.org/show_bug.cgi?id=184368
+
+        Reviewed by Ryosuke Niwa.
+
+        'CommitSet.diff' will be used in multiple independent incoming changes.
+        It would be easier to make this a separate change to parallelize the changes depends on this API.
+
+        * public/v3/models/commit-set.js:
+        (CommitSet.prototype.createNameWithoutCollision): Moved from 'AnalysisTaskPage' and make it more generic.
+        (CommitSet.prototype.diff): Describe differences between 2 commit sets including commit, root and patch differences.
+        * public/v3/pages/analysis-task-page.js: Move 'AnalysisTaskPage._createRetryNameForTestGroup' to CommitSet in a more generic form.
+        (AnalysisTaskPage.prototype._retryCurrentTestGroup): Use 'CommitSet.withoutRootPatchOrOwnedCommit' instead.
+        (AnalysisTaskPage.prototype._createRetryNameForTestGroup): Moved to CommitSet in a more generic form.
+        * unit-tests/commit-set-tests.js: Added unit tests for 'CommitSet.diff'.
+
 2018-04-05  Dewei Zhu  <dewei_zhu@apple.com>
 

trunk/Websites/perf.webkit.org/public/v3/models/commit-set.js

@@ -146 +146 @@
         return false;
     }
+
+    static createNameWithoutCollision(name, existingNameSet)
+    {
+        console.assert(existingNameSet instanceof Set);
+        if (!existingNameSet.has(name))
+            return name;
+        const nameWithNumberMatch = name.match(/(.+?)\s*\(\s*(\d+)\s*\)\s*$/);
+        let number = 1;
+        if (nameWithNumberMatch) {
+            name = nameWithNumberMatch[1];
+            number = parseInt(nameWithNumberMatch[2]);
+        }
+
+        let newName;
+        do {
+            number++;
+            newName = `${name} (${number})`;
+        } while (existingNameSet.has(newName));
+
+        return newName;
+    }
+
+    static diff(firstCommitSet, secondCommitSet)
+    {
+        console.assert(!firstCommitSet.equals(secondCommitSet));
+        const allRepositories = new Set([...firstCommitSet.repositories(), ...secondCommitSet.repositories()]);
+        const sortedRepositories = Repository.sortByNamePreferringOnesWithURL([...allRepositories]);
+        const nameParts = [];
+        const missingCommit = {label: () => 'none'};
+        const missingPatch = {filename: () => 'none'};
+        const makeNameGenerator = () => {
+            const existingNameSet = new Set;
+            return (name) => {
+                const newName = CommitSet.createNameWithoutCollision(name, existingNameSet);
+                existingNameSet.add(newName);
+                return newName;
+            }
+        };
+
+        for (const repository of sortedRepositories) {
+            const firstCommit = firstCommitSet.commitForRepository(repository) || missingCommit;
+            const secondCommit = secondCommitSet.commitForRepository(repository) || missingCommit;
+            const firstPatch = firstCommitSet.patchForRepository(repository) || missingPatch;
+            const secondPatch = secondCommitSet.patchForRepository(repository) || missingPatch;
+            const nameGenerator = makeNameGenerator();
+
+            if (firstCommit == secondCommit && firstPatch == secondPatch)
+                continue;
+
+            if (firstCommit != secondCommit && firstPatch == secondPatch)
+                nameParts.push(`${repository.name()}: ${secondCommit.diff(firstCommit).label}`);
+
+            // FIXME: It would be nice if we can abbreviate the name when it's too long.
+            const nameForFirstPatch = nameGenerator(firstPatch.filename());
+            const nameForSecondPath = nameGenerator(secondPatch.filename());
+
+            if (firstCommit == secondCommit && firstPatch != secondPatch)
+                nameParts.push(`${repository.name()}: ${nameForFirstPatch} - ${nameForSecondPath}`);
+
+            if (firstCommit != secondCommit && firstPatch != secondPatch)
+                nameParts.push(`${repository.name()}: ${firstCommit.label()} with ${nameForFirstPatch} - ${secondCommit.label()} with ${nameForSecondPath}`);
+        }
+
+        if (firstCommitSet.allRootFiles().length || secondCommitSet.allRootFiles().length) {
+            const firstRootFileSet = new Set(firstCommitSet.allRootFiles());
+            const secondRootFileSet = new Set(secondCommitSet.allRootFiles());
+            const uniqueInFirstCommitSet = firstCommitSet.allRootFiles().filter((rootFile) => !secondRootFileSet.has(rootFile));
+            const uniqueInSecondCommitSet = secondCommitSet.allRootFiles().filter((rootFile) => !firstRootFileSet.has(rootFile));
+            const nameGenerator = makeNameGenerator();
+            const firstDescription = uniqueInFirstCommitSet.map((rootFile) => nameGenerator(rootFile.filename())).join(', ');
+            const secondDescription = uniqueInSecondCommitSet.map((rootFile) => nameGenerator(rootFile.filename())).join(', ');
+            nameParts.push(`Roots: ${firstDescription || 'none'} - ${secondDescription || 'none'}`);
+        }
+
+        return nameParts.join(' ');
+    }
 }
 

trunk/Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js

@@ -786 +786 @@
     _retryCurrentTestGroup(testGroup, repetitionCount)
     {
-        const newName = this._createRetryNameForTestGroup(testGroup.name());
+        const existingNames = (this._testGroups || []).map((group) => group.name());
+        const newName = CommitSet.createNameWithoutCollision(testGroup.name(), new Set(existingNames));
         const commitSetList = testGroup.requestedCommitSets();
         const platform = this._task.platform() || testGroup.platform();
@@ -840 +841 @@
             alert('Failed to create a new test group: ' + error);
         });
-    }
-
-    _createRetryNameForTestGroup(name)
-    {
-        var nameWithNumberMatch = name.match(/(.+?)\s*\(\s*(\d+)\s*\)\s*$/);
-        var number = 1;
-        if (nameWithNumberMatch) {
-            name = nameWithNumberMatch[1];
-            number = parseInt(nameWithNumberMatch[2]);
-        }
-
-        var newName;
-        do {
-            number++;
-            newName = `${name} (${number})`;
-        } while (this._hasDuplicateTestGroupName(newName));
-
-        return newName;
     }
 

trunk/Websites/perf.webkit.org/unit-tests/commit-set-tests.js

@@ -28 +28 @@
     return UploadedFile.ensureSingleton(457, {'createdAt': new Date('2017-05-01T21:03:27Z'), 'filename': 'root.dat', 'extension': '.dat', 'author': 'some user',
         size: 16452111, sha256: '03eed7a8494ab8794c44b7d4308e55448fc56f4d6c175809ba968f78f656dbbb'});
+}
+
+function createSharedRoot()
+{
+    return UploadedFile.ensureSingleton(458, {'createdAt': new Date('2017-05-01T22:03:27Z'), 'filename': 'root.dat', 'extension': '.dat', 'author': 'some user',
+        size: 16452111, sha256: '03eed7a8494ab8794c44b7d4308e55448fc56f4aac175809ba968f78f656dbbb'});
 }
 
@@ -105 +111 @@
 }
 
+function anotherWebKitCommit()
+{
+    return CommitLog.ensureSingleton(2018, {
+        repository: MockModels.webkit,
+        revision: 'webkit-commit-1',
+        ownsCommits: false,
+        time: 1456932773000
+    });
+}
+
+function commitWithSVNRevision()
+{
+    return CommitLog.ensureSingleton(2019, {
+        repository: MockModels.webkit,
+        revision: '12345',
+        ownsCommits: false,
+        time: 1456932773000
+    });
+}
+
+function anotherCommitWithSVNRevision()
+{
+    return CommitLog.ensureSingleton(2020, {
+        repository: MockModels.webkit,
+        revision: '45678',
+        ownsCommits: false,
+        time: 1456932773000
+    });
+}
+
+function commitWithGitRevision()
+{
+    return CommitLog.ensureSingleton(2021, {
+        repository: MockModels.webkitGit,
+        revision: '13a0590d34f26fda3953c42ff833132a1a6f6f5a',
+        ownsCommits: false,
+        time: 1456932773000
+    });
+}
+
+function anotherCommitWithGitRevision()
+{
+    return CommitLog.ensureSingleton(2022, {
+        repository: MockModels.webkitGit,
+        revision: '2f8dd3321d4f51c04f4e2019428ce9ffe97f1ef1',
+        ownsCommits: false,
+        time: 1456932773000
+    });
+}
+
 describe('CommitSet', () => {
     MockRemoteAPI.inject();
@@ -154 +210 @@
             revisionItems: [{ commit: webkitCommit(), requiresBuild: false }],
             customRoots: [createAnotherRoot()]
+        });
+    }
+
+    function commitSetWithTwoRoots()
+    {
+        return CommitSet.ensureSingleton(7, {
+            revisionItems: [{ commit: webkitCommit(), requiresBuild: false }],
+            customRoots: [createRoot(), createSharedRoot()]
+        });
+    }
+
+    function commitSetWithAnotherWebKitCommit()
+    {
+        return CommitSet.ensureSingleton(8, {
+            revisionItems: [{ commit: anotherWebKitCommit(), requiresBuild: false }],
+            customRoots: []
+        });
+    }
+
+    function commitSetWithAnotherCommitPatchAndRoot()
+    {
+        return CommitSet.ensureSingleton(9, {
+            revisionItems: [{ commit: anotherWebKitCommit(), requiresBuild: true, patch: createPatch()}],
+            customRoots: [createRoot(), createSharedRoot()]
+        });
+    }
+
+    function commitSetWithSVNCommit()
+    {
+        return CommitSet.ensureSingleton(10, {
+            revisionItems: [{ commit: commitWithSVNRevision(), requiresBuild: false }],
+            customRoots: []
+        });
+    }
+
+    function anotherCommitSetWithSVNCommit()
+    {
+        return CommitSet.ensureSingleton(11, {
+            revisionItems: [{ commit: anotherCommitWithSVNRevision(), requiresBuild: false }],
+            customRoots: []
+        });
+    }
+
+    function commitSetWithGitCommit()
+    {
+        return CommitSet.ensureSingleton(12, {
+            revisionItems: [{ commit: commitWithGitRevision(), requiresBuild: false }],
+            customRoots: []
+        });
+    }
+
+    function anotherCommitSetWithGitCommit()
+    {
+        return CommitSet.ensureSingleton(13, {
+            revisionItems: [{ commit: anotherCommitWithGitRevision(), requiresBuild: false }],
+            customRoots: []
+        });
+    }
+
+    function commitSetWithTwoCommits()
+    {
+        return CommitSet.ensureSingleton(14, {
+            revisionItems: [{ commit: commitWithGitRevision(), requiresBuild: false }, { commit: commitWithSVNRevision(), requiresBuild: false }],
+            customRoots: []
+        });
+    }
+
+    function anotherCommitSetWithTwoCommits()
+    {
+        return CommitSet.ensureSingleton(15, {
+            revisionItems: [{ commit: anotherCommitWithGitRevision(), requiresBuild: false }, { commit: anotherCommitWithSVNRevision(), requiresBuild: false }],
+            customRoots: []
         });
     }
@@ -199 +327 @@
             assert(oneMeasurementCommitSet().equals(oneCommitSet()));
         });
-    })
+    });
+
+    describe('diff',  () => {
+        it('should describe patch difference', () => {
+            assert.equal(CommitSet.diff(commitSetWithPatch(), commitSetWithAnotherPatch()), 'WebKit: patch.dat - patch.dat (2)');
+        });
+
+        it('should describe root difference', () => {
+            assert.equal(CommitSet.diff(commitSetWithRoot(), anotherCommitSetWithRoot()), 'Roots: root.dat - root.dat (2)');
+            assert.equal(CommitSet.diff(commitSetWithRoot(), commitSetWithTwoRoots()), 'Roots: none - root.dat');
+            assert.equal(CommitSet.diff(commitSetWithTwoRoots(), oneCommitSet()), 'Roots: root.dat, root.dat (2) - none');
+        });
+
+        it('should describe commit difference', () => {
+            assert.equal(CommitSet.diff(oneCommitSet(), commitSetWithAnotherWebKitCommit()), 'WebKit: webkit-commit-0 - webkit-commit-1');
+            assert.equal(CommitSet.diff(commitSetWithSVNCommit(), anotherCommitSetWithSVNCommit()), 'WebKit: r12345-r45678');
+            assert.equal(CommitSet.diff(commitSetWithGitCommit(), anotherCommitSetWithGitCommit()), 'WebKit-Git: 13a0590d..2f8dd332');
+            assert.equal(CommitSet.diff(commitSetWithTwoCommits(), anotherCommitSetWithTwoCommits()), 'WebKit: r12345-r45678 WebKit-Git: 13a0590d..2f8dd332');
+        });
+
+        it('should describe commit root and patch difference', () => {
+            assert.equal(CommitSet.diff(oneCommitSet(), commitSetWithAnotherCommitPatchAndRoot()), 'WebKit: webkit-commit-0 with none - webkit-commit-1 with patch.dat Roots: none - root.dat, root.dat (2)');
+        });
+    });
 });