Attachment 370328 Details for Bug 197973 – Patch

[patch] Patch

bug-197973-20190521110446.patch (text/plain), 13.27 KB, created by Zhifei Fang on 2019-05-21 11:04:47 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Zhifei Fang

Created: 2019-05-21 11:04:47 PDT

Size: 13.27 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,26 @@
>+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!).
>+        
>+        Currently on analysis-task-page we check if his children
>+        custom-configuration-test-group-form already have been set test groups, if so
>+        we don't update the new test groups
>+        * browser-tests/analysis-task-page-tests.js: Added a unittest for analysis task page.
>+        * 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, then we 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
>+
> 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