Attachment 339488 Details for Bug 185269 – Patch

[patch] Patch

bug-185269-20180503170343.patch (text/plain), 10.12 KB, created by dewei_zhu on 2018-05-03 17:03:43 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: dewei_zhu

Created: 2018-05-03 17:03:43 PDT

Size: 10.12 KB

patch

obsolete

>Subversion Revision: 231324
>diff --git a/Websites/perf.webkit.org/ChangeLog b/Websites/perf.webkit.org/ChangeLog
>index f88ea154efbbfd987ecede2a51374bef43bcf1da..6de1166a97975bcc8057488f40475b3bbc522c9a 100644
>--- a/Websites/perf.webkit.org/ChangeLog
>+++ b/Websites/perf.webkit.org/ChangeLog
>@@ -1,3 +1,20 @@
>+2018-05-03  Dewei Zhu  <dewei_zhu@apple.com>
>+
>+        Range bisector should check the commits for repositories without change in specified range.
>+        https://bugs.webkit.org/show_bug.cgi?id=185269
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        For repositories without a change in the specified range, we still need to use them to filter commit
>+        sets. Before this change, code does not apply filtering by those repositories against commit set. As
>+        a result, commit sets with different commits for those repositories may be chosen as bisecting commit set.
>+
>+        * public/v3/commit-set-range-bisector.js: Updated the logic to verify range for repositories without
>+        change in range.
>+        (CommitSetRangeBisector.async.commitSetClosestToMiddleOfAllCommits):
>+        * unit-tests/commit-set-range-bisector-tests.js: Added a unit test to guard against this change.
>+        (async):
>+
> 2018-05-02  Dewei Zhu  <dewei_zhu@apple.com>
> 
>         Create analysis task should sync analysis task status after creation.
>diff --git a/Websites/perf.webkit.org/public/v3/commit-set-range-bisector.js b/Websites/perf.webkit.org/public/v3/commit-set-range-bisector.js
>index e47847415f980a7604824bc238567e5c384f5e61..da297c4d6b330e9d3c2128d21914c1a04766436f 100644
>--- a/Websites/perf.webkit.org/public/v3/commit-set-range-bisector.js
>+++ b/Websites/perf.webkit.org/public/v3/commit-set-range-bisector.js
>@@ -16,18 +16,24 @@ class CommitSetRangeBisector {
>         const commitRangeByRepository = new Map;
>         const indexForAllTimelessCommitsWithOrderByRepository = new Map;
>         const allCommitsWithCommitTime = [];
>-        const topLevelRepositoriesWithCommitChange = firstCommitSet.topLevelRepositories()
>+        const topLevelRepositoriesWithOrderedCommits = firstCommitSet.topLevelRepositories()
>             .filter((repository) => {
>                 const firstCommit = firstCommitSet.commitForRepository(repository);
>                 const secondCommit = secondCommitSet.commitForRepository(repository);
>-                return firstCommit !== secondCommit && CommitLog.hasOrdering(firstCommit, secondCommit);
>+                return CommitLog.hasOrdering(firstCommit, secondCommit);
>             });
> 
>-        await Promise.all(topLevelRepositoriesWithCommitChange.map(async (repository) => {
>+        await Promise.all(topLevelRepositoriesWithOrderedCommits.map(async (repository) => {
>             const firstCommit = firstCommitSet.commitForRepository(repository);
>             const secondCommit = secondCommitSet.commitForRepository(repository);
>             const [startCommit, endCommit] = CommitLog.orderTwoCommits(firstCommit, secondCommit);
>-            const commits = await CommitLog.fetchBetweenRevisions(repository, startCommit.revision(), endCommit.revision());
>+
>+            let commits = null;
>+            if (startCommit !== endCommit)
>+                commits = await CommitLog.fetchBetweenRevisions(repository, startCommit.revision(), endCommit.revision());
>+            else
>+                commits = [startCommit];
>+
>             if (startCommit.hasCommitTime()) {
>                 allCommitsWithCommitTime.push(startCommit, ...commits);
>                 commitRangeByRepository.set(repository, (commit) =>
>diff --git a/Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js b/Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js
>index b5ae2020e50e4617c491d05df6aa022093cf41d8..bdae30efcff197b16c3da0df065684e3c935929e 100644
>--- a/Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js
>+++ b/Websites/perf.webkit.org/unit-tests/commit-set-range-bisector-tests.js
>@@ -140,6 +140,54 @@ describe('CommitSetRangeBisector', () => {
>         ];
>     }
> 
>+    function commitSetsWitCommitRollback()
>+    {
>+        return [
>+            CommitSet.ensureSingleton(11, {
>+                revisionItems: [
>+                    { commit: makeCommit(1, MockModels.webkit, 'webkit-commit-1', 10), requiresBuild: false },
>+                    { commit: makeCommit(11, MockModels.osx, 'osx-commit-1', 1), requiresBuild: false },
>+                    { commit: makeCommit(201, MockModels.ownerRepository, 'owner-commit-1', 0, 1), requiresBuild: false }
>+                ],
>+                customRoots: []}),
>+            CommitSet.ensureSingleton(12, {
>+                revisionItems: [
>+                    { commit: makeCommit(1, MockModels.webkit, 'webkit-commit-1', 10), requiresBuild: false },
>+                    { commit: makeCommit(12, MockModels.osx, 'osx-commit-2', 21), requiresBuild: false },
>+                    { commit: makeCommit(202, MockModels.ownerRepository, 'owner-commit-2', 0, 2), requiresBuild: false }
>+                ],
>+                customRoots: []}),
>+            CommitSet.ensureSingleton(13, {
>+                revisionItems: [
>+                    { commit: makeCommit(2, MockModels.webkit, 'webkit-commit-2', 20), requiresBuild: false },
>+                    { commit: makeCommit(12, MockModels.osx, 'osx-commit-2', 21), requiresBuild: false },
>+                    { commit: makeCommit(202, MockModels.ownerRepository, 'owner-commit-2', 0, 2), requiresBuild: false }
>+                ],
>+                customRoots: []}),
>+            CommitSet.ensureSingleton(14, {
>+                revisionItems: [
>+                    { commit: makeCommit(3, MockModels.webkit, 'webkit-commit-3', 30), requiresBuild: false },
>+                    { commit: makeCommit(13, MockModels.osx, 'osx-commit-3', 31), requiresBuild: false },
>+                    { commit: makeCommit(202, MockModels.ownerRepository, 'owner-commit-2', 0, 2), requiresBuild: false }
>+                ],
>+                customRoots: []}),
>+            CommitSet.ensureSingleton(15, {
>+                revisionItems: [
>+                    { commit: makeCommit(3, MockModels.webkit, 'webkit-commit-3', 30), requiresBuild: false },
>+                    { commit: makeCommit(13, MockModels.osx, 'osx-commit-3', 31), requiresBuild: false },
>+                    { commit: makeCommit(203, MockModels.ownerRepository, 'owner-commit-3', 0, 3), requiresBuild: false }
>+                ],
>+                customRoots: []}),
>+            CommitSet.ensureSingleton(17, {
>+                revisionItems: [
>+                    { commit: makeCommit(6, MockModels.webkit, 'webkit-commit-6', 60), requiresBuild: false },
>+                    { commit: makeCommit(13, MockModels.osx, 'osx-commit-3', 31), requiresBuild: false },
>+                    { commit: makeCommit(201, MockModels.ownerRepository, 'owner-commit-1', 0, 1), requiresBuild: false }
>+                ],
>+                customRoots: []}),
>+        ];
>+    }
>+
>     function commitSetsWithSomeHaveOwnedCommits()
>     {
>         return [
>@@ -615,6 +663,79 @@ describe('CommitSetRangeBisector', () => {
>             assert.equal(await promise, allCommitSets[3]);
>         });
> 
>+        it('should still check the repository revision even the repository has no change in the range', async () => {
>+            const allCommitSets = commitSetsWitCommitRollback();
>+            const startCommitSet = allCommitSets[0];
>+            const endCommitSet = allCommitSets[allCommitSets.length - 1];
>+            const promise = CommitSetRangeBisector.commitSetClosestToMiddleOfAllCommits([startCommitSet, endCommitSet], allCommitSets);
>+            const webkitId = MockModels.webkit.id();
>+            const osxId = MockModels.osx.id();
>+
>+            assert.equal(requests.length, 2);
>+            assert.equal(requests[0].url, '/api/commits/9/?precedingRevision=osx-commit-1&lastRevision=osx-commit-3');
>+            assert.equal(requests[1].url, '/api/commits/11/?precedingRevision=webkit-commit-1&lastRevision=webkit-commit-6');
>+
>+            requests[0].resolve({
>+                'commits': [
>+                    {
>+                        repository: osxId,
>+                        id: 12,
>+                        revision: 'osx-commit-2',
>+                        ownsCommits: false,
>+                        time: 21
>+                    },
>+                    {
>+                        repository: osxId,
>+                        id: 13,
>+                        revision: 'osx-commit-3',
>+                        ownsCommits: false,
>+                        time: 31
>+                    }
>+                ]
>+            });
>+
>+            requests[1].resolve({
>+                'commits': [
>+                    {
>+                        repository: webkitId,
>+                        id: 2,
>+                        revision: 'webkit-commit-2',
>+                        ownsCommits: false,
>+                        time: 20
>+                    },
>+                    {
>+                        repository: webkitId,
>+                        id: 3,
>+                        revision: 'webkit-commit-3',
>+                        ownsCommits: false,
>+                        time: 30
>+                    },
>+                    {
>+                        repository: webkitId,
>+                        id: 4,
>+                        revision: 'webkit-commit-4',
>+                        ownsCommits: false,
>+                        time: 40
>+                    },
>+                    {
>+                        repository: webkitId,
>+                        id: 5,
>+                        revision: 'webkit-commit-5',
>+                        ownsCommits: false,
>+                        time: 50
>+                    },
>+                    {
>+                        repository: webkitId,
>+                        id: 6,
>+                        revision: 'webkit-commit-6',
>+                        ownsCommits: false,
>+                        time: 60
>+                    },
>+                ]
>+            });
>+            assert.equal(await promise, null);
>+        });
>+
>         it('should filter out commit set with owned commit', async () => {
>             const allCommitSets = commitSetsWithSomeHaveOwnedCommits();
>             const startCommitSet = allCommitSets[0];

Flags:

rniwa: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185269: 339472 | 339488