Fix a bug in range bisector that start commit may be counted twice.
authordewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Jul 2018 03:55:21 +0000 (03:55 +0000)
committerdewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Jul 2018 03:55:21 +0000 (03:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187205

Reviewed by Darin Adler.

Range bisector counted start commit twice if start commit is the same as end commit.

* public/v3/commit-set-range-bisector.js:
(CommitSetRangeBisector.async.commitSetClosestToMiddleOfAllCommits):
* unit-tests/commit-set-range-bisector-tests.js: Added a unit test for this change.

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/v3/commit-set-range-bisector.js
Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js

index aa4dde1..c4e2112 100644 (file)
@@ -1,3 +1,16 @@
+2018-06-29  Dewei Zhu  <dewei_zhu@apple.com>
+
+        Fix a bug in range bisector that start commit may be counted twice.
+        https://bugs.webkit.org/show_bug.cgi?id=187205
+
+        Reviewed by Darin Adler.
+
+        Range bisector counted start commit twice if start commit is the same as end commit.
+
+        * public/v3/commit-set-range-bisector.js:
+        (CommitSetRangeBisector.async.commitSetClosestToMiddleOfAllCommits):
+        * unit-tests/commit-set-range-bisector-tests.js: Added a unit test for this change.
+
 2018-06-28  Dewei Zhu  <dewei_zhu@apple.com>
 
         Fix a bug ComponentBase that wrong content template may be used.
index 5840d68..651958f 100644 (file)
@@ -29,17 +29,17 @@ class CommitSetRangeBisector {
             }
 
             const [startCommit, endCommit] = CommitLog.orderTwoCommits(firstCommit, secondCommit);
-            const commits = startCommit === endCommit ? [startCommit] : await CommitLog.fetchBetweenRevisions(repository, startCommit.revision(), endCommit.revision());
+            const commitsExcludingStartCommit = startCommit === endCommit ? [] : await CommitLog.fetchBetweenRevisions(repository, startCommit.revision(), endCommit.revision());
 
             if (startCommit.hasCommitTime()) {
-                allCommitsWithCommitTime.push(startCommit, ...commits);
+                allCommitsWithCommitTime.push(startCommit, ...commitsExcludingStartCommit);
                 commitRangeByRepository.set(repository, (commit) =>
                     commit.hasCommitTime() && startCommit.time() <= commit.time() && commit.time() <= endCommit.time());
                 repositoriesWithCommitTime.add(repository);
             } else {
                 const indexByCommit = new Map;
                 indexByCommit.set(startCommit, 0);
-                commits.forEach((commit, index) => indexByCommit.set(commit, index + 1));
+                commitsExcludingStartCommit.forEach((commit, index) => indexByCommit.set(commit, index + 1));
                 indexForAllTimelessCommitsWithOrderByRepository.set(repository, indexByCommit);
                 commitRangeByRepository.set(repository, (commit) =>
                     commit.hasCommitOrder() && startCommit.order() <= commit.order() && commit.order() <= endCommit.order());
index 1e016d6..71cb13f 100644 (file)
@@ -315,6 +315,36 @@ describe('CommitSetRangeBisector', () => {
         ];
     }
 
+    function commitSetsWithTime()
+    {
+        return [
+            CommitSet.ensureSingleton(28, {
+                revisionItems: [
+                    { commit: makeCommit(1, MockModels.webkit, 'webkit-commit-1', 10), requiresBuild: false },
+                    { commit: makeCommit(201, MockModels.osx, 'osx-commit-201', 8), requiresBuild: false }
+                ],
+                customRoots: []}),
+            CommitSet.ensureSingleton(29, {
+                revisionItems: [
+                    { commit: makeCommit(1, MockModels.webkit, 'webkit-commit-1', 10), requiresBuild: false },
+                    { commit: makeCommit(203, MockModels.osx, 'osx-commit-203', 11), requiresBuild: false }
+                ],
+                customRoots: []}),
+            CommitSet.ensureSingleton(30, {
+                revisionItems: [
+                    { commit: makeCommit(1, MockModels.webkit, 'webkit-commit-1', 10), requiresBuild: false },
+                    { commit: makeCommit(204, MockModels.osx, 'osx-commit-204', 12), requiresBuild: false }
+                ],
+                customRoots: []}),
+            CommitSet.ensureSingleton(31, {
+                revisionItems: [
+                    { commit: makeCommit(1, MockModels.webkit, 'webkit-commit-1', 10), requiresBuild: false },
+                    { commit: makeCommit(205, MockModels.osx, 'osx-commit-205', 13), requiresBuild: false }
+                ],
+                customRoots: []}),
+        ];
+    }
+
     function createRoot()
     {
         return UploadedFile.ensureSingleton(456, {'createdAt': new Date('2017-05-01T21:03:27Z'), 'filename': 'root.dat', 'extension': '.dat', 'author': 'some user',
@@ -987,6 +1017,51 @@ describe('CommitSetRangeBisector', () => {
 
             assert.equal(await promise, allCommitSets[3]);
         });
+
+        it('should not double count a commit if start and end commits are the same', async () => {
+            const allCommitSets = commitSetsWithTime();
+            const startCommitSet = allCommitSets[0];
+            const endCommitSet = allCommitSets[allCommitSets.length - 1];
+            const promise = CommitSetRangeBisector.commitSetClosestToMiddleOfAllCommits([startCommitSet, endCommitSet], allCommitSets);
+            assert.equal(requests.length, 1);
+            const osxFetchRequest = requests[0];
+            assert.equal(osxFetchRequest.url, '/api/commits/9/?precedingRevision=osx-commit-201&lastRevision=osx-commit-205');
+            const osxId = MockModels.osx.id();
+            osxFetchRequest.resolve({
+                'commits': [
+                    {
+                        repository: osxId,
+                        id: 202,
+                        revision: 'osx-commit-202',
+                        ownsCommits: false,
+                        time: 9
+                    },
+                    {
+                        repository: osxId,
+                        id: 203,
+                        revision: 'osx-commit-203',
+                        ownsCommits: false,
+                        time: 11
+                    },
+                    {
+                        repository: osxId,
+                        id: 204,
+                        revision: 'osx-commit-204',
+                        ownsCommits: false,
+                        time: 12
+                    },
+                    {
+                        repository: osxId,
+                        id: 205,
+                        revision: 'osx-commit-205',
+                        ownsCommits: false,
+                        time: 13
+                    }
+                ]
+            });
+
+            assert.equal(await promise, allCommitSets[1]);
+        });
     });
 
     describe('_orderCommitSetsByTimeAndOrderThenDeduplicate', () => {