Attachment 370325 Details for Bug 197973 – Patch

[patch] Patch

bug-197973-20190521105318.patch (text/plain), 13.08 KB, created by Zhifei Fang on 2019-05-21 10:53:19 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Zhifei Fang

Created: 2019-05-21 10:53:19 PDT

Size: 13.08 KB

patch

obsolete

>Index: Websites/perf.webkit.org/ChangeLog
>===================================================================
>--- Websites/perf.webkit.org/ChangeLog	(revision 245569)
>+++ Websites/perf.webkit.org/ChangeLog	(working copy)
>@@ -1,3 +1,19 @@
>+2019-05-21  Zhifei FANG  <zhifei_fang@apple.com>
>+
>+        Fix 'analysis-task-configurator-pane' doesn't update when switch from one analysis task to another.
>+        https://bugs.webkit.org/show_bug.cgi?id=197973
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * browser-tests/analysis-task-page-tests.js: Added a unittest for analysis task page.
>+        (async.importScripts):
>+        (async):
>+        * browser-tests/index.html:
>+        * public/v3/components/custom-analysis-task-configurator.js:
>+        (CustomAnalysisTaskConfigurator.prototype.setCommitSets): We will need to find the comparisonRepositoryGroup based on the comparison commit set, for now it doesn't make any difference since baseline and comparison use the same repositoryGroup, however this could be a potential issue.
>+        * public/v3/pages/analysis-task-page.js:
>+        (AnalysisTaskConfiguratorPane.prototype.setTestGroups): When we first go to task A, we will set the form's commit sets, thenwe redirect to task B, we check if we already set the form's commit set, we already have did so, then we don't update the test groups, this cause the bug
>+
> 2019-05-14  Dewei Zhu  <dewei_zhu@apple.com>
> 
>         Pruning old file logic should not stop after removing 10 files if there are more files to be removed.
>Index: Websites/perf.webkit.org/browser-tests/analysis-task-page-tests.js
>===================================================================
>--- Websites/perf.webkit.org/browser-tests/analysis-task-page-tests.js	(nonexistent)
>+++ Websites/perf.webkit.org/browser-tests/analysis-task-page-tests.js	(working copy)
>@@ -0,0 +1,252 @@
>+describe('AnalysisTaskPage', () => {
>+
>+    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', 'models/metric.js', 'models/triggerable.js', 'models/test.js', 'models/platform.js', 'models/test-group.js', 'models/test.js', 'models/triggerable.js',
>+        'models/build-request.js', 'components/test-group-form.js', 'components/chart-pane-base.js', 'components/custom-analysis-task-configurator.js', 'components/instant-file-uploader.js',
>+        'lazily-evaluated-function.js', 'components/custom-configuration-test-group-form.js', 'pages/page.js', 'pages/page-with-heading.js', 'pages/analysis-task-page.js'];
>+
>+    const testGroupData1 = {
>+        author: "author1",
>+        buildRequests: ["147071", "147072"],
>+        commitSets: ["210948", "210949"],
>+        createdAt: 1557868890126,
>+        hidden: false,
>+        id: "1",
>+        initialRepetitionCount: "6",
>+        mayNeedMoreRequests: false,
>+        name: "Fake TestGroup 1",
>+        needsNotification: true,
>+        notificationSentAt: 0,
>+        platform: "1",
>+        task: "6490",
>+    };
>+
>+    const testGroupData2 = {
>+      author: "author2",
>+      buildRequests: ["147073", "147074"],
>+      commitSets: ["210949", "210950"],
>+      createdAt: 1557670817494,
>+      hidden: false,
>+      id: "2",
>+      initialRepetitionCount: "5",
>+      mayNeedMoreRequests: true,
>+      name: "Fake TestGroup 2",
>+      needsNotification: true,
>+      notificationSentAt: 0,
>+      platform: "2",
>+      task: "6083",
>+    }
>+
>+    const metricData = {
>+        aggregator: null,
>+        name: "Time",
>+        test: 1
>+    }
>+
>+    const platformData1 = {
>+        metrics: [],
>+        lastModified: [],
>+        name: "Fake platform 1"
>+    }
>+
>+    const platformData2 = {
>+        metrics: [],
>+        lastModified: [],
>+        name: "Fake platform 2"
>+    }
>+
>+    const repositoryData = {
>+        blameUrl: null,
>+        hasReportedCommits: true,
>+        name: "OS X",
>+        owner: null,
>+        url: null
>+    };
>+
>+    const commits = [
>+        {id: "1", repository: "1", commitOwner: null, revision: "240605", time: 1548716856285},
>+        {id: "2", repository: "1", commitOwner: null, revision: "240606", time: 1548684740000},
>+        {id: "3", repository: "1", commitOwner: null, revision: "240607", time: 1549637196578},
>+    ];
>+
>+
>+    const commitSets1 = [{
>+        customRoots: [],
>+        id: "210948",
>+        revisionItems: [
>+            {commit: "1", patch: null, rootFile: null, commitOwner: null, requiresBuild: false}
>+        ]
>+    },{
>+        customRoots: [],
>+        id: "210949",
>+        revisionItems: [
>+            {commit: "2", patch: null, rootFile: null, commitOwner: null, requiresBuild: false}
>+        ]
>+    }];
>+
>+    const commitSets2 = [{
>+        customRoots: [],
>+        id: "210949",
>+        revisionItems: [
>+            {commit: "2", patch: null, rootFile: null, commitOwner: null, requiresBuild: false}
>+        ]
>+    },{
>+        customRoots: [],
>+        id: "210950",
>+        revisionItems: [
>+            {commit: "3", patch: null, rootFile: null, commitOwner: null, requiresBuild: false}
>+        ]
>+    }];
>+
>+    const buildRequests1 = [{
>+        build: "1",
>+        commitSet: "210948",
>+        createdAt: 1548814524000,
>+        id: "147071",
>+        order: "-2",
>+        platform: "1",
>+        repositoryGroup: "9",
>+        status: "completed",
>+        task: "6083",
>+        test: "1",
>+        testGroup: "14554",
>+        triggerable: "1",
>+        url: "fake/url"
>+    },{
>+        build: "2",
>+        commitSet: "210949",
>+        createdAt: 1548814624000,
>+        id: "147072",
>+        order: "-2",
>+        platform: "1",
>+        repositoryGroup: "9",
>+        status: "completed",
>+        task: "6083",
>+        test: "1",
>+        testGroup: "14554",
>+        triggerable: "1",
>+        url: "fake/url"
>+    }];
>+
>+    const buildRequests2 = [{
>+        build: "3",
>+        commitSet: "210949",
>+        createdAt: 1548814524000,
>+        id: "147073",
>+        order: "-2",
>+        platform: "1",
>+        repositoryGroup: "9",
>+        status: "completed",
>+        task: "6083",
>+        test: "1",
>+        testGroup: "14554",
>+        triggerable: "2",
>+        url: "fake/url"
>+    },{
>+        build: "4",
>+        commitSet: "210950",
>+        createdAt: 1548814624000,
>+        id: "147074",
>+        order: "-2",
>+        platform: "1",
>+        repositoryGroup: "9",
>+        status: "completed",
>+        task: "6083",
>+        test: "1",
>+        testGroup: "14554",
>+        triggerable: "2",
>+        url: "fake/url"
>+      }];
>+
>+    const testData = {
>+        name: "fake test",
>+        parentId: null,
>+        url: null,
>+    };
>+
>+    const triggerableRepositoryGroupData = {
>+        acceptsCustomRoots: false,
>+        description: null,
>+        hidden: true,
>+        id: "1",
>+        name: "fake-svn",
>+        repositories: [],
>+    }
>+
>+    const triggerableData = {
>+        name: "fake triggerable",
>+        configurations: [],
>+        repositoryGroups:[]
>+    };
>+
>+    async function importScripts(context)
>+    {
>+        return await context.importScripts(scripts, 'AnalysisTaskConfiguratorPane', 'TestGroup', 'Platform', 'BuildRequest', 'Test', 'Repository', 'Metric', 'Triggerable', 'TriggerableRepositoryGroup', 'CommitLog');
>+    }
>+
>+    it("should change task configurator content when switch between differnt task", async () => {
>+        const context = new BrowsingContext;
>+        await importScripts(context);
>+        const taskConfiguratorPane = new context.symbols.AnalysisTaskConfiguratorPane;
>+        context.document.body.appendChild(taskConfiguratorPane.element());
>+        const repository = context.symbols.Repository.ensureSingleton("1", repositoryData);
>+        triggerableRepositoryGroupData.repositories= [{
>+            acceptsPatch: true,
>+            repository: repository
>+        }];
>+        const triggerableRepositoryGroup = context.symbols.TriggerableRepositoryGroup.ensureSingleton("1", triggerableRepositoryGroupData);
>+        const test = context.symbols.Test.ensureSingleton("1", testData);
>+        metricData.test = test;
>+        const metric = context.symbols.Metric.ensureSingleton("1", metricData);
>+        platformData1.metrics = [metric];
>+        const platform1 = context.symbols.Platform.ensureSingleton("1", platformData1);
>+        platformData2.metrics = [metric];
>+        const platform2 = context.symbols.Platform.ensureSingleton("2", platformData2);
>+        testGroupData1.platform = platform1;
>+        testGroupData2.platform = platform2;
>+        triggerableData.repositoryGroups = [triggerableRepositoryGroup];
>+        triggerableData.configurations = [{
>+            test: test,
>+            platform: platform1,
>+        }];
>+
>+        const triggerable1 = context.symbols.Triggerable.ensureSingleton("1", triggerableData);
>+        triggerableData.configurations = [{
>+            test: test,
>+            platform: platform2,
>+        }];
>+        const triggerable2 = context.symbols.Triggerable.ensureSingleton("2", triggerableData);
>+        const testGroup1 = context.symbols.TestGroup.ensureSingleton("1", testGroupData1);
>+        const testGroup2 = context.symbols.TestGroup.ensureSingleton("2", testGroupData2);
>+        const testGroups = [testGroup1, testGroup2];
>+        const configurator = taskConfiguratorPane.part('form').part('configurator');
>+        context.symbols.CommitLog._constructFromRawData({commits: commits});
>+        context.symbols.BuildRequest.constructBuildRequestsFromData({
>+            buildRequests: buildRequests1,
>+            commitSets: commitSets1,
>+            commits: commits,
>+            uploadedFiles: [],
>+        }).forEach((buildRequest) => {
>+            testGroup1.addBuildRequest(buildRequest);
>+        });
>+        configurator.render();
>+        taskConfiguratorPane.setTestGroups(testGroups, testGroup1);
>+        expect(configurator.platform()).to.be(testGroup1.platform());
>+        expect(configurator.tests()[0]._id).to.be(testGroup1.test()._id);
>+        expect(configurator.commitSets()[0].revisionForRepository(repository)).to.be("240605");
>+        expect(configurator.commitSets()[1].revisionForRepository(repository)).to.be("240606");
>+        context.symbols.BuildRequest.constructBuildRequestsFromData({
>+            buildRequests: buildRequests2,
>+            commitSets: commitSets2,
>+            commits: commits,
>+            uploadedFiles: [],
>+        }).forEach((buildRequest) => {
>+            testGroup2.addBuildRequest(buildRequest);
>+        });
>+        taskConfiguratorPane.setTestGroups(testGroups, testGroup2);
>+        expect(configurator.platform()).to.be(testGroup2.platform());
>+        expect(configurator.tests()[0]._id).to.be(testGroup2.test()._id);
>+        expect(configurator.commitSets()[0].revisionForRepository(repository)).to.be("240606");
>+        expect(configurator.commitSets()[1].revisionForRepository(repository)).to.be("240607");
>+    })
>+});
>Index: Websites/perf.webkit.org/browser-tests/index.html
>===================================================================
>--- Websites/perf.webkit.org/browser-tests/index.html	(revision 245539)
>+++ Websites/perf.webkit.org/browser-tests/index.html	(working copy)
>@@ -31,6 +31,7 @@ mocha.setup('bdd');
> <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 src="analysis-task-page-tests.js"></script>
> <script>
> 
> afterEach(() => {
>Index: Websites/perf.webkit.org/public/v3/components/custom-analysis-task-configurator.js
>===================================================================
>--- Websites/perf.webkit.org/public/v3/components/custom-analysis-task-configurator.js	(revision 245539)
>+++ Websites/perf.webkit.org/public/v3/components/custom-analysis-task-configurator.js	(working copy)
>@@ -92,7 +92,7 @@ class CustomAnalysisTaskConfigurator ext
>             this._setPatchFiles('Baseline', baselineCommitSet);
>         }
> 
>-        const comparisonRepositoryGroup = triggerable.repositoryGroups().find((repositoryGroup) => repositoryGroup.accepts(baselineCommitSet));
>+        const comparisonRepositoryGroup = triggerable.repositoryGroups().find((repositoryGroup) => repositoryGroup.accepts(comparisonCommitSet));
>         if (comparisonRepositoryGroup) {
>             this._repositoryGroupByConfiguration['Comparison'] = comparisonRepositoryGroup;
>             this._setUploadedFilesToUploader(this._customRootUploaders['Comparison'], comparisonCommitSet.customRoots());
>Index: Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js
>===================================================================
>--- Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js	(revision 245539)
>+++ Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js	(working copy)
>@@ -220,7 +220,7 @@ class AnalysisTaskConfiguratorPane exten
>     {
>         this._currentGroup = currentGroup;
>         const form = this.part('form');
>-        if (!form.hasCommitSets() && currentGroup)
>+        if (currentGroup)
>             form.setConfigurations(currentGroup.test(), currentGroup.platform(), currentGroup.repetitionCount(), currentGroup.requestedCommitSets());
>         this.enqueueToRender();
>     }

Actions: View | Formatted Diff | Diff

Attachments on bug 197973: 370095 | 370325 | 370328 | 370378 | 371537 | 371568 | 372586